Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions commands/list_tasks_masking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,14 @@ func TestListTasksDoesNotMaskStructuralDecorations(t *testing.T) {
- name: gate
block:
- name: unprobeable
dokku_git_auth:
host: github.com
dokku_registry_auth:
global: true
server: docker.io
username: u
password: p
`)

stdout, stderr, exit := runApply(t, path, "--secret_value=netrc", "--list-tasks", "--json")
stdout, stderr, exit := runApply(t, path, "--secret_value=registry", "--list-tasks", "--json")
if exit != 0 {
t.Fatalf("exit = %d, want 0; stdout=%s stderr=%s", exit, stdout, stderr)
}
Expand All @@ -373,7 +374,7 @@ func TestListTasksDoesNotMaskStructuralDecorations(t *testing.T) {
t.Errorf("masking must not touch %s; got:\n%s", want, stdout)
}
}
if !strings.Contains(stdout, `"probe_caveat":"*** state has no read command`) {
if !strings.Contains(stdout, `"probe_caveat":"*** login state has no read command`) {
t.Errorf("expected the prose caveat to be masked; got:\n%s", stdout)
}

Expand Down
5 changes: 3 additions & 2 deletions commands/list_tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ func TestApplyListTasksMarksProbeSupport(t *testing.T) {
path := writeTasksFile(t, `---
- tasks:
- name: unprobeable
dokku_git_auth:
host: github.com
dokku_registry_auth:
global: true
server: docker.io
username: deploy-bot
password: examplepassword
- name: partially probed
Expand Down
2 changes: 1 addition & 1 deletion docs/command-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ running anything:
$ docket apply --list-tasks
==> Play: api
[0] dokku apps:create api [tags=core]
[1] dokku git:auth github.com (never converges)
[1] dokku registry:login docker.io (never converges)
[2] dokku git:from-image api (partial probe)
```

Expand Down
2 changes: 1 addition & 1 deletion docs/tasks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ A task marked `(never converges)` cannot read its own state, so it plans as drif
- [dokku_docker_options](dokku_docker_options.md) - Manages docker-options for a given dokku application
- [dokku_domains](dokku_domains.md) - Manages the domains for a given dokku application or globally
- [dokku_domains_toggle](dokku_domains_toggle.md) - Enables or disables the domains plugin for a given dokku application
- [dokku_git_auth](dokku_git_auth.md) - Manages netrc credentials for a git host (never converges)
- [dokku_git_auth](dokku_git_auth.md) - Manages netrc credentials for a git host
- [dokku_git_from_archive](dokku_git_from_archive.md) - Deploys a git repository from an archive URL
- [dokku_git_from_image](dokku_git_from_image.md) - Deploys a git repository from a docker image
- [dokku_git_property](dokku_git_property.md) - Manages the git configuration for a given dokku application
Expand Down
4 changes: 2 additions & 2 deletions docs/tasks/dokku_git_auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Not supported - netrc credentials are write-only and cannot be read back.

## Probe support

Not supported - netrc state has no read command, so the task plans as drift on every run.
Supported.

## Identity

Expand All @@ -22,7 +22,7 @@ Keyed by `host`.
| --- | --- | --- | --- | --- | --- |
| `host` | string | yes | | | Git server hostname (e.g. github.com) |
| `username` | string | no | | | Netrc username. Required when state is present. |
| `password` | string | no | | | Netrc password. Required when state is present. (sensitive) |
| `password` | string | no | | | Netrc password. Required when state is present. Must not contain a newline. (sensitive) |
| `state` | string | no | present | present, absent | Desired state of the netrc entry |

## Examples
Expand Down
86 changes: 73 additions & 13 deletions tasks/git_auth_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,23 @@ package tasks
import (
"context"
"fmt"
"strings"

"github.com/dokku/docket/subprocess"
)

// GitAuthTask manages netrc credentials for a git host via dokku git:auth.
//
// Idempotency is intentionally skipped here because dokku has no public way
// to query the current netrc state (the file lives at $DOKKU_ROOT/.netrc with
// mode 0600). Tracking upstream support in dokku/dokku#8504; once a
// no-change exit code is available, this task should switch to using it
// instead of always reporting Changed=true.
// The netrc file itself is unreadable (mode 0600 under $DOKKU_ROOT) and there
// is no report that dumps it, but idempotency does not need one: dokku's
// git:auth-status compares the stored entry against credentials it is handed
// and answers with its exit code, which is the only question Plan() asks. See
// gitAuthMatches for the two shapes that answers.
//
// The password never reaches argv on either path. Both git:auth and
// git:auth-status read it from stdin when the username is supplied and the
// password argument is omitted, so it stays out of the plan output, the trace
// log, and the process table on the dokku host.
type GitAuthTask struct {
// Host is the git server hostname (e.g. github.com)
Host string `required:"true" identity:"key" yaml:"host" description:"Git server hostname (e.g. github.com)"`
Expand All @@ -22,7 +28,7 @@ type GitAuthTask struct {
Username string `required:"false" yaml:"username,omitempty" description:"Netrc username. Required when state is present."`

// Password is the netrc password. Required when state is present.
Password string `required:"false" sensitive:"true" yaml:"password,omitempty" description:"Netrc password. Required when state is present."`
Password string `required:"false" sensitive:"true" yaml:"password,omitempty" description:"Netrc password. Required when state is present. Must not contain a newline."`

// State is the desired state of the netrc entry
State State `required:"false" yaml:"state,omitempty" default:"present" options:"present,absent" description:"Desired state of the netrc entry"`
Expand All @@ -48,13 +54,18 @@ func (t GitAuthTask) Doc() string {
}

// ExportSupport reports how docket export handles this task.
//
// Probing and exporting ask different questions of the same command:
// git:auth-status confirms credentials the recipe already holds, but it cannot
// enumerate the hosts with an entry and never reveals a stored username, so
// there is still nothing for an exporter to reconstruct.
func (t GitAuthTask) ExportSupport() ExportSupport {
return ExportSupport{Status: ExportUnsupported, Caveat: "netrc credentials are write-only and cannot be read back"}
}

// ProbeSupport reports whether Plan() can read this task's current state.
func (t GitAuthTask) ProbeSupport() ProbeSupport {
return ProbeSupport{Status: ProbeUnsupported, Caveat: "netrc state has no read command, so the task plans as drift on every run"}
return ProbeSupport{Status: ProbeSupported}
}

// Examples returns the examples for the git auth task
Expand Down Expand Up @@ -91,41 +102,62 @@ func (t GitAuthTask) Validate() error {
if t.State == StatePresent && (t.Username == "" || t.Password == "") {
return fmt.Errorf("'username' and 'password' are required when state is 'present'")
}
// dokku reads the password with `read -r`, which stops at the first
// newline, and a .netrc entry is a single line either way. Rejecting it
// here is the difference between a clear error and a task that silently
// writes a truncated password and then never converges.
if strings.ContainsAny(t.Password, "\r\n") {
return fmt.Errorf("'password' must not contain a newline")
}
return nil
}

// Plan reports the drift the GitAuthTask would produce. dokku has no public
// way to query netrc state, so the plan reports drift unconditionally.
// Plan reports the drift the GitAuthTask would produce.
func (t GitAuthTask) Plan(ctx context.Context) PlanResult {
if err := t.Validate(); err != nil {
return planErr(err)
}
return DispatchPlan(t.State, map[State]func() PlanResult{
StatePresent: func() PlanResult {
matches, err := gitAuthMatches(ctx, t.Host, t.Username, t.Password)
if err != nil {
return PlanResult{Status: PlanStatusError, Error: err}
}
if matches {
return PlanResult{InSync: true, Status: PlanStatusOK}
}
inputs := []subprocess.ExecCommandInput{{
Command: "dokku",
Args: []string{"--quiet", "git:auth", t.Host, t.Username, t.Password},
Args: []string{"--quiet", "git:auth", t.Host, t.Username},
Stdin: strings.NewReader(t.Password),
}}
return PlanResult{
InSync: false,
Status: PlanStatusModify,
Reason: "netrc state not probed",
Mutations: []string{"git:auth " + t.Host + " " + t.Username + " " + t.Password},
Reason: "netrc entry does not match",
Mutations: []string{"git:auth " + t.Host + " as " + t.Username},
Commands: resolveCommands(ctx, inputs),
apply: func(ctx context.Context) TaskOutputState {
return runExecInputs(ctx, TaskOutputState{State: StateAbsent}, StatePresent, inputs)
},
}
},
StateAbsent: func() PlanResult {
cleared, err := gitAuthMatches(ctx, t.Host, "", "")
if err != nil {
return PlanResult{Status: PlanStatusError, Error: err}
}
if cleared {
return PlanResult{InSync: true, Status: PlanStatusOK}
}
inputs := []subprocess.ExecCommandInput{{
Command: "dokku",
Args: []string{"--quiet", "git:auth", t.Host},
}}
return PlanResult{
InSync: false,
Status: PlanStatusDestroy,
Reason: "netrc state not probed",
Reason: "netrc entry present",
Mutations: []string{"git:auth " + t.Host + " (clear)"},
Commands: resolveCommands(ctx, inputs),
apply: func(ctx context.Context) TaskOutputState {
Expand All @@ -136,6 +168,34 @@ func (t GitAuthTask) Plan(ctx context.Context) PlanResult {
})
}

// gitAuthMatches reports whether the netrc entry for host already matches the
// requested state. git:auth-status is a comparator rather than a dump: it
// prints nothing and exits 0 when the stored entry equals what it was handed.
// Handed no username it answers the absent-state question instead - exit 0
// when the host has no entry at all.
//
// The password goes over stdin, where dokku's fn-git-auth-read-password picks
// it up, so it never reaches the argv of the dokku process on the server.
//
// Returns (false, err) when the probe could not run - a transport failure, a
// missing dokku binary, or a cancellation; (true, nil) when the server is
// already in the requested state; (false, nil) otherwise. A "no" is one answer
// and not two: git:auth-status exits non-zero both for a host with no entry
// and for a host whose entry differs, so the present-state plan reports a
// modify rather than telling a create apart from a replacement. Tracking
// distinct exit codes upstream in dokku/dokku#8995.
func gitAuthMatches(ctx context.Context, host, username, password string) (bool, error) {
input := subprocess.ExecCommandInput{
Command: "dokku",
Args: []string{"--quiet", "git:auth-status", host},
}
if username != "" {
input.Args = append(input.Args, username)
input.Stdin = strings.NewReader(password)
}
return subprocess.Probe(ctx, input)
}

// init registers the GitAuthTask with the task registry
func init() {
RegisterTask(&GitAuthTask{})
Expand Down
33 changes: 33 additions & 0 deletions tasks/git_auth_task_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,30 @@ func TestIntegrationGitAuth(t *testing.T) {
t.Errorf("expected state 'present', got '%s'", result.State)
}

// re-applying the same credentials is a no-op. This is the end-to-end
// proof that git:auth-status compared what git:auth wrote, password
// included - both of which travelled on stdin.
result = setTask.Execute(testCtx())
if result.Error != nil {
t.Fatalf("failed to re-apply git auth: %v", result.Error)
}
if result.Changed {
t.Errorf("expected Changed=false when the netrc entry already matches")
}

// a rotated password is drift even though the host and username are
// unchanged, so the probe has to be comparing the secret and not just
// the entry's existence.
rotateTask := setTask
rotateTask.Password = "rotated-token"
result = rotateTask.Execute(testCtx())
if result.Error != nil {
t.Fatalf("failed to rotate git auth: %v", result.Error)
}
if !result.Changed {
t.Errorf("expected Changed=true when the password changed")
}

// remove credentials
unsetTask := GitAuthTask{Host: host, State: StateAbsent}
result = unsetTask.Execute(testCtx())
Expand All @@ -46,4 +70,13 @@ func TestIntegrationGitAuth(t *testing.T) {
if result.State != StateAbsent {
t.Errorf("expected state 'absent', got '%s'", result.State)
}

// removing an entry that is already gone is a no-op
result = unsetTask.Execute(testCtx())
if result.Error != nil {
t.Fatalf("failed to re-unset git auth: %v", result.Error)
}
if result.Changed {
t.Errorf("expected Changed=false when the host has no netrc entry")
}
}
Loading