fix(secrets): address review feedback

- Return error instead of nil on invalid KEY=value format in CLI
- Fix docs to use 'gordon deploy' instead of 'gordon push'
- Add Docker 255-char limit protection to resolveContainerName
- Add tests for service name length (63 chars) and trailing hyphen validation

commit: bb9656b70719bcb7788ca708e7c93c92473df92c
author: brice · 8mo ago · 2026-02-06 12:45 UTC
parent: 95aa1f7
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +51 -4

M docs/cli/secrets.md +1 -1


@@ -212,7 +212,7 @@ gordon secrets set app.mydomain.com -a redis REDIS_PASSWORD=cache-secret
212	212		gordon secrets list app.mydomain.com
213	213
214	214		# Redeploy to pick up new secrets
215		-	gordon push app.mydomain.com
	215	+	gordon deploy app.mydomain.com
216	216		```
217	217
218	218		## Related


@@ -212,7 +212,7 @@ gordon secrets set app.mydomain.com -a redis REDIS_PASSWORD=cache-secret
212		gordon secrets list app.mydomain.com	212		gordon secrets list app.mydomain.com
213			213
214		# Redeploy to pick up new secrets	214		# Redeploy to pick up new secrets
215	-	gordon push app.mydomain.com	215	+	gordon deploy app.mydomain.com
216		```	216		```
217			217
218		## Related	218		## Related

M internal/adapters/in/cli/secrets.go +1 -2


@@ -242,8 +242,7 @@ Examples:
242	242		for _, pair := range pairs {
243	243		parts := strings.SplitN(pair, "=", 2)
244	244		if len(parts) != 2 {
245		-	fmt.Println(styles.RenderError(fmt.Sprintf("Invalid format: %s (expected KEY=value)", pair)))
246		-	return nil
	245	+	return fmt.Errorf("invalid format: %s (expected KEY=value)", pair)
247	246		}
248	247		secrets[parts[0]] = parts[1]
249	248		}

 			for _, pair := range pairs {
 				parts := strings.SplitN(pair, "=", 2)
 				if len(parts) != 2 {
-					fmt.Println(styles.RenderError(fmt.Sprintf("Invalid format: %s (expected KEY=value)", pair)))
+					return fmt.Errorf("invalid format: %s (expected KEY=value)", pair)
-					return nil
 				}
 				secrets[parts[0]] = parts[1]
 			}

M internal/usecase/secrets/service.go +13 -1


@@ -260,8 +260,20 @@ func validateServiceName(service string) error {
260	260		}
261	261
262	262		// resolveContainerName builds the container name from a domain and service.
	263	+	// Ensures the total name does not exceed Docker's 255-character limit.
263	264		func resolveContainerName(domainName, service string) string {
264		-	return domain.SanitizeDomainForContainer(domainName) + "-" + service
	265	+	sanitized := domain.SanitizeDomainForContainer(domainName)
	266	+	name := sanitized + "-" + service
	267	+
	268	+	// Docker container names are limited to 255 characters
	269	+	if len(name) > 255 {
	270	+	// Truncate the domain part to fit, keeping room for "-" and service name
	271	+	maxDomainLen := 255 - 1 - len(service)
	272	+	if maxDomainLen > 0 {
	273	+	name = sanitized[:maxDomainLen] + "-" + service
	274	+	}
	275	+	}
	276	+	return name
265	277		}
266	278
267	279		// ValidateDomain validates that a domain is safe to use for secret storage.


@@ -260,8 +260,20 @@ func validateServiceName(service string) error {
260		}	260		}
261			261
262		// resolveContainerName builds the container name from a domain and service.	262		// resolveContainerName builds the container name from a domain and service.
			263	+	// Ensures the total name does not exceed Docker's 255-character limit.
263		func resolveContainerName(domainName, service string) string {	264		func resolveContainerName(domainName, service string) string {
264	-	return domain.SanitizeDomainForContainer(domainName) + "-" + service	265	+	sanitized := domain.SanitizeDomainForContainer(domainName)
			266	+	name := sanitized + "-" + service
			267	+
			268	+	// Docker container names are limited to 255 characters
			269	+	if len(name) > 255 {
			270	+	// Truncate the domain part to fit, keeping room for "-" and service name
			271	+	maxDomainLen := 255 - 1 - len(service)
			272	+	if maxDomainLen > 0 {
			273	+	name = sanitized[:maxDomainLen] + "-" + service
			274	+	}
			275	+	}
			276	+	return name
265		}	277		}
266			278
267		// ValidateDomain validates that a domain is safe to use for secret storage.	279		// ValidateDomain validates that a domain is safe to use for secret storage.

M internal/usecase/secrets/service_test.go +36 -0


@@ -529,6 +529,42 @@ func TestService_DeleteAttachment_InvalidServiceName(t *testing.T) {
529	529		assert.ErrorIs(t, err, ErrInvalidServiceName)
530	530		}
531	531
	532	+	func TestService_SetAttachment_ServiceNameTooLong(t *testing.T) {
	533	+	store := outmocks.NewMockDomainSecretStore(t)
	534	+	svc := NewService(store, testLogger())
	535	+
	536	+	// Service name exceeds DNS label limit of 63 characters
	537	+	longService := strings.Repeat("a", 64)
	538	+	err := svc.SetAttachment(context.Background(), "app.example.com", longService, map[string]string{"K": "V"})
	539	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
	540	+	}
	541	+
	542	+	func TestService_SetAttachment_ServiceNameTrailingHyphen(t *testing.T) {
	543	+	store := outmocks.NewMockDomainSecretStore(t)
	544	+	svc := NewService(store, testLogger())
	545	+
	546	+	err := svc.SetAttachment(context.Background(), "app.example.com", "postgres-", map[string]string{"K": "V"})
	547	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
	548	+	}
	549	+
	550	+	func TestService_DeleteAttachment_ServiceNameTooLong(t *testing.T) {
	551	+	store := outmocks.NewMockDomainSecretStore(t)
	552	+	svc := NewService(store, testLogger())
	553	+
	554	+	// Service name exceeds DNS label limit of 63 characters
	555	+	longService := strings.Repeat("a", 64)
	556	+	err := svc.DeleteAttachment(context.Background(), "app.example.com", longService, "SOME_KEY")
	557	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
	558	+	}
	559	+
	560	+	func TestService_DeleteAttachment_ServiceNameTrailingHyphen(t *testing.T) {
	561	+	store := outmocks.NewMockDomainSecretStore(t)
	562	+	svc := NewService(store, testLogger())
	563	+
	564	+	err := svc.DeleteAttachment(context.Background(), "app.example.com", "redis-", "SOME_KEY")
	565	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
	566	+	}
	567	+
532	568		func TestService_AttachmentValidationHappensBeforeStore(t *testing.T) {
533	569		store := outmocks.NewMockDomainSecretStore(t)
534	570		svc := NewService(store, testLogger())


@@ -529,6 +529,42 @@ func TestService_DeleteAttachment_InvalidServiceName(t *testing.T) {
529	assert.ErrorIs(t, err, ErrInvalidServiceName)	529		assert.ErrorIs(t, err, ErrInvalidServiceName)
530	}	530		}
531		531
		532	+	func TestService_SetAttachment_ServiceNameTooLong(t *testing.T) {
		533	+	store := outmocks.NewMockDomainSecretStore(t)
		534	+	svc := NewService(store, testLogger())
		535	+
		536	+	// Service name exceeds DNS label limit of 63 characters
		537	+	longService := strings.Repeat("a", 64)
		538	+	err := svc.SetAttachment(context.Background(), "app.example.com", longService, map[string]string{"K": "V"})
		539	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
		540	+	}
		541	+
		542	+	func TestService_SetAttachment_ServiceNameTrailingHyphen(t *testing.T) {
		543	+	store := outmocks.NewMockDomainSecretStore(t)
		544	+	svc := NewService(store, testLogger())
		545	+
		546	+	err := svc.SetAttachment(context.Background(), "app.example.com", "postgres-", map[string]string{"K": "V"})
		547	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
		548	+	}
		549	+
		550	+	func TestService_DeleteAttachment_ServiceNameTooLong(t *testing.T) {
		551	+	store := outmocks.NewMockDomainSecretStore(t)
		552	+	svc := NewService(store, testLogger())
		553	+
		554	+	// Service name exceeds DNS label limit of 63 characters
		555	+	longService := strings.Repeat("a", 64)
		556	+	err := svc.DeleteAttachment(context.Background(), "app.example.com", longService, "SOME_KEY")
		557	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
		558	+	}
		559	+
		560	+	func TestService_DeleteAttachment_ServiceNameTrailingHyphen(t *testing.T) {
		561	+	store := outmocks.NewMockDomainSecretStore(t)
		562	+	svc := NewService(store, testLogger())
		563	+
		564	+	err := svc.DeleteAttachment(context.Background(), "app.example.com", "redis-", "SOME_KEY")
		565	+	assert.ErrorIs(t, err, ErrInvalidServiceName)
		566	+	}
		567	+
532	func TestService_AttachmentValidationHappensBeforeStore(t *testing.T) {	568		func TestService_AttachmentValidationHappensBeforeStore(t *testing.T) {
533	store := outmocks.NewMockDomainSecretStore(t)	569		store := outmocks.NewMockDomainSecretStore(t)
534	svc := NewService(store, testLogger())	570		svc := NewService(store, testLogger())