2b6f969 fix: address review — DeployIntent always called, repo validation, docker login stdin, ARG docs · gordon

M docs/cli/push.md +3 -1


@@ -50,7 +50,9 @@ To push attachment images (databases, caches, etc.), use `gordon attachments pus
50	50		`v1.2.3-4-gabc1234` or `v1.2.3-dirty`). If no tag is found, `latest` is used.
51	51		- When `--build` is set, the command builds with `docker buildx build --load`
52	52		and injects `VERSION`, `GIT_TAG`, `GIT_SHA`, and `BUILD_TIME` into the build
53		-	environment plus any `--build-arg` values.
	53	+	environment plus any `--build-arg` values. To use these in your Dockerfile,
	54	+	declare them with `ARG` (e.g., `ARG VERSION`) then reference via `ENV` or
	55	+	in build steps.
54	56		- Use `-f/--file` to build from a Dockerfile outside the current directory root.
55	57		- The version tag and `latest` are both pushed (unless the version is `latest`).
56	58

   `v1.2.3-4-gabc1234` or `v1.2.3-dirty`). If no tag is found, `latest` is used.
 - When `--build` is set, the command builds with `docker buildx build --load`
   and injects `VERSION`, `GIT_TAG`, `GIT_SHA`, and `BUILD_TIME` into the build
-  environment plus any `--build-arg` values.
+  environment plus any `--build-arg` values. To use these in your Dockerfile,
+  declare them with `ARG` (e.g., `ARG VERSION`) then reference via `ENV` or
+  in build steps.
 - Use `-f/--file` to build from a Dockerfile outside the current directory root.
 - The version tag and `latest` are both pushed (unless the version is `latest`).

M docs/deployment/index.md +1 -1


@@ -50,7 +50,7 @@ Standard Docker workflow — no Gordon binary required on the runner.
50	50		- Registry endpoint is `gordon.example.com` (not a separate registry host)
51	51
52	52		```bash
53		-	docker login -u ci-bot -p "$GORDON_TOKEN" gordon.example.com
	53	+	echo "$GORDON_TOKEN" \| docker login -u ci-bot --password-stdin gordon.example.com
54	54		docker build -t gordon.example.com/myapp:v1.2.0 .
55	55		docker push gordon.example.com/myapp:v1.2.0
56	56		```


@@ -50,7 +50,7 @@ Standard Docker workflow — no Gordon binary required on the runner.
50		- Registry endpoint is `gordon.example.com` (not a separate registry host)	50		- Registry endpoint is `gordon.example.com` (not a separate registry host)
51			51
52		```bash	52		```bash
53	-	docker login -u ci-bot -p "$GORDON_TOKEN" gordon.example.com	53	+	echo "$GORDON_TOKEN" \| docker login -u ci-bot --password-stdin gordon.example.com
54		docker build -t gordon.example.com/myapp:v1.2.0 .	54		docker build -t gordon.example.com/myapp:v1.2.0 .
55		docker push gordon.example.com/myapp:v1.2.0	55		docker push gordon.example.com/myapp:v1.2.0
56		```	56		```

M internal/adapters/in/cli/auth.go +13 -5


@@ -441,7 +441,12 @@ The hash can be stored in your secrets backend and referenced in the config:
441	441		// runTokenGenerate generates a new authentication token.
442	442		// parseAndConvertScopes parses a comma-separated scope string and converts
443	443		// simple scopes (push, pull, ) to Docker v2 format (repository::action).
444		-	func parseAndConvertScopes(scopesStr, repo string) []string {
	444	+	func parseAndConvertScopes(scopesStr, repo string) ([]string, error) {
	445	+	repo = strings.TrimSpace(repo)
	446	+	if repo == "" {
	447	+	return nil, fmt.Errorf("repository name cannot be empty")
	448	+	}
	449	+
445	450		rawScopes := strings.Split(scopesStr, ",")
446	451		for i := range rawScopes {
447	452		rawScopes[i] = strings.TrimSpace(rawScopes[i])

@@ -457,10 +462,10 @@ func parseAndConvertScopes(scopesStr, repo string) []string {
457	462		}
458	463
459	464		if !hasSimpleScope {
460		-	return rawScopes
	465	+	return rawScopes, nil
461	466		}
462	467
463		-	// Convert simple scopes to v2 format: repository:*:push,pull
	468	+	// Convert simple scopes to v2 format: repository:<repo>:push,pull
464	469		var scopes []string
465	470		var actions []string
466	471		for _, s := range rawScopes {

@@ -474,7 +479,7 @@ func parseAndConvertScopes(scopesStr, repo string) []string {
474	479		if len(actions) > 0 {
475	480		scopes = append(scopes, "repository:"+repo+":"+strings.Join(actions, ","))
476	481		}
477		-	return scopes
	482	+	return scopes, nil
478	483		}
479	484
480	485		func runTokenGenerate(subject, scopesStr, expiryStr, configPath, repo string) error {

@@ -492,7 +497,10 @@ func runTokenGenerate(subject, scopesStr, expiryStr, configPath, repo string) er
492	497		}
493	498		}
494	499
495		-	scopes := parseAndConvertScopes(scopesStr, repo)
	500	+	scopes, err := parseAndConvertScopes(scopesStr, repo)
	501	+	if err != nil {
	502	+	return fmt.Errorf("invalid scopes: %w", err)
	503	+	}
496	504
497	505		// Create auth service
498	506		log := zerowrap.New(zerowrap.Config{Level: "warn"})

 // runTokenGenerate generates a new authentication token.
 // parseAndConvertScopes parses a comma-separated scope string and converts
 // simple scopes (push, pull, *) to Docker v2 format (repository:*:action).
-func parseAndConvertScopes(scopesStr, repo string) []string {
+func parseAndConvertScopes(scopesStr, repo string) ([]string, error) {
+	repo = strings.TrimSpace(repo)
+	if repo == "" {
+		return nil, fmt.Errorf("repository name cannot be empty")
+	}
 	rawScopes := strings.Split(scopesStr, ",")
 	for i := range rawScopes {
 		rawScopes[i] = strings.TrimSpace(rawScopes[i])
 	}
 	if !hasSimpleScope {
-		return rawScopes
+		return rawScopes, nil
 	}
-	// Convert simple scopes to v2 format: repository:*:push,pull
+	// Convert simple scopes to v2 format: repository:<repo>:push,pull
 	var scopes []string
 	var actions []string
 	for _, s := range rawScopes {
 	if len(actions) > 0 {
 		scopes = append(scopes, "repository:"+repo+":"+strings.Join(actions, ","))
 	}
-	return scopes
+	return scopes, nil
 }
 func runTokenGenerate(subject, scopesStr, expiryStr, configPath, repo string) error {
 		}
 	}
-	scopes := parseAndConvertScopes(scopesStr, repo)
+	scopes, err := parseAndConvertScopes(scopesStr, repo)
+	if err != nil {
+		return fmt.Errorf("invalid scopes: %w", err)
+	}
 	// Create auth service
 	log := zerowrap.New(zerowrap.Config{Level: "warn"})

M internal/adapters/in/cli/auth_scopes_test.go +21 -5


@@ -4,30 +4,46 @@ import (
4	4		"testing"
5	5
6	6		"github.com/stretchr/testify/assert"
	7	+	"github.com/stretchr/testify/require"
7	8		)
8	9
9	10		func TestParseAndConvertScopes_DefaultWildcard(t *testing.T) {
10		-	got := parseAndConvertScopes("push,pull", "*")
	11	+	got, err := parseAndConvertScopes("push,pull", "*")
	12	+	require.NoError(t, err)
11	13		assert.Equal(t, []string{"repository:*:push,pull"}, got)
12	14		}
13	15
14	16		func TestParseAndConvertScopes_SpecificRepo(t *testing.T) {
15		-	got := parseAndConvertScopes("push,pull", "myapp")
	17	+	got, err := parseAndConvertScopes("push,pull", "myapp")
	18	+	require.NoError(t, err)
16	19		assert.Equal(t, []string{"repository:myapp:push,pull"}, got)
17	20		}
18	21
19	22		func TestParseAndConvertScopes_SpecificRepoPushOnly(t *testing.T) {
20		-	got := parseAndConvertScopes("push", "myapp")
	23	+	got, err := parseAndConvertScopes("push", "myapp")
	24	+	require.NoError(t, err)
21	25		assert.Equal(t, []string{"repository:myapp:push"}, got)
22	26		}
23	27
24	28		func TestParseAndConvertScopes_MixedWithAdminScopes(t *testing.T) {
25		-	got := parseAndConvertScopes("push,pull,admin:routes:read", "myapp")
	29	+	got, err := parseAndConvertScopes("push,pull,admin:routes:read", "myapp")
	30	+	require.NoError(t, err)
26	31		assert.ElementsMatch(t, []string{"repository:myapp:push,pull", "admin:routes:read"}, got)
27	32		}
28	33
29	34		func TestParseAndConvertScopes_AlreadyV2Format_RepoIgnored(t *testing.T) {
30	35		// When scopes are already in v2 format, --repo is ignored (scopes returned as-is)
31		-	got := parseAndConvertScopes("repository:specific:push", "*")
	36	+	got, err := parseAndConvertScopes("repository:specific:push", "*")
	37	+	require.NoError(t, err)
32	38		assert.Equal(t, []string{"repository:specific:push"}, got)
33	39		}
	40	+
	41	+	func TestParseAndConvertScopes_EmptyRepo(t *testing.T) {
	42	+	_, err := parseAndConvertScopes("push,pull", "")
	43	+	assert.EqualError(t, err, "repository name cannot be empty")
	44	+	}
	45	+
	46	+	func TestParseAndConvertScopes_WhitespaceRepo(t *testing.T) {
	47	+	_, err := parseAndConvertScopes("push,pull", " ")
	48	+	assert.EqualError(t, err, "repository name cannot be empty")
	49	+	}

 	"testing"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 func TestParseAndConvertScopes_DefaultWildcard(t *testing.T) {
-	got := parseAndConvertScopes("push,pull", "*")
+	got, err := parseAndConvertScopes("push,pull", "*")
+	require.NoError(t, err)
 	assert.Equal(t, []string{"repository:*:push,pull"}, got)
 }
 func TestParseAndConvertScopes_SpecificRepo(t *testing.T) {
-	got := parseAndConvertScopes("push,pull", "myapp")
+	got, err := parseAndConvertScopes("push,pull", "myapp")
+	require.NoError(t, err)
 	assert.Equal(t, []string{"repository:myapp:push,pull"}, got)
 }
 func TestParseAndConvertScopes_SpecificRepoPushOnly(t *testing.T) {
-	got := parseAndConvertScopes("push", "myapp")
+	got, err := parseAndConvertScopes("push", "myapp")
+	require.NoError(t, err)
 	assert.Equal(t, []string{"repository:myapp:push"}, got)
 }
 func TestParseAndConvertScopes_MixedWithAdminScopes(t *testing.T) {
-	got := parseAndConvertScopes("push,pull,admin:routes:read", "myapp")
+	got, err := parseAndConvertScopes("push,pull,admin:routes:read", "myapp")
+	require.NoError(t, err)
 	assert.ElementsMatch(t, []string{"repository:myapp:push,pull", "admin:routes:read"}, got)
 }
 func TestParseAndConvertScopes_AlreadyV2Format_RepoIgnored(t *testing.T) {
 	// When scopes are already in v2 format, --repo is ignored (scopes returned as-is)
-	got := parseAndConvertScopes("repository:specific:push", "*")
+	got, err := parseAndConvertScopes("repository:specific:push", "*")
+	require.NoError(t, err)
 	assert.Equal(t, []string{"repository:specific:push"}, got)
 }
+func TestParseAndConvertScopes_EmptyRepo(t *testing.T) {
+	_, err := parseAndConvertScopes("push,pull", "")
+	assert.EqualError(t, err, "repository name cannot be empty")
+}
+func TestParseAndConvertScopes_WhitespaceRepo(t *testing.T) {
+	_, err := parseAndConvertScopes("push,pull", "  ")
+	assert.EqualError(t, err, "repository name cannot be empty")
+}

M internal/adapters/in/cli/push.go +3 -4


@@ -233,9 +233,8 @@ func runPush(ctx context.Context, req pushRequest) error {
233	233		// shouldSkipDeploy signals deploy intent and returns whether the explicit deploy
234	234		// should be skipped (either because --no-deploy was set, or the token lacks scope).
235	235		func shouldSkipDeploy(ctx context.Context, plane ControlPlane, imageName string, noDeploy bool) bool {
236		-	if noDeploy {
237		-	return true
238		-	}
	236	+	// Always call DeployIntent to suppress server-side auto-deploy, even with --no-deploy.
	237	+	// Without this, the server's event listener would still auto-deploy the pushed image.
239	238		if err := plane.DeployIntent(ctx, imageName); err != nil {
240	239		if isInsufficientScope(err) {
241	240		fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive")

@@ -244,7 +243,7 @@ func shouldSkipDeploy(ctx context.Context, plane ControlPlane, imageName string,
244	243		// Non-fatal: worst case we get a redundant deploy via event
245	244		fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err)
246	245		}
247		-	return false
	246	+	return noDeploy
248	247		}
249	248
250	249		// isInsufficientScope returns true if the error is an HTTP 403 Forbidden response,

 // shouldSkipDeploy signals deploy intent and returns whether the explicit deploy
 // should be skipped (either because --no-deploy was set, or the token lacks scope).
 func shouldSkipDeploy(ctx context.Context, plane ControlPlane, imageName string, noDeploy bool) bool {
-	if noDeploy {
+	// Always call DeployIntent to suppress server-side auto-deploy, even with --no-deploy.
-		return true
+	// Without this, the server's event listener would still auto-deploy the pushed image.
-	}
 	if err := plane.DeployIntent(ctx, imageName); err != nil {
 		if isInsufficientScope(err) {
 			fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive")
 		// Non-fatal: worst case we get a redundant deploy via event
 		fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err)
 	}
-	return false
+	return noDeploy
 }
 // isInsufficientScope returns true if the error is an HTTP 403 Forbidden response,