fix: make route creation config-only and idempotent
Remove registry manifest validation from route POST/PUT admin handlers. Route creation is config management — image validation belongs at deploy time. AddRoute now returns nil without saving when domain+image already match. Push error message updated to suggest 'gordon bootstrap' for first deploys. Closes bootstrapping deadlock described in #98.
- commit
- 9609e08e019951c51e006b2eea3081d5bfd63484
- author
- brice · · 2026-03-13 20:33 UTC
- parent
- 6ccaf03
- signature
- unsigned
7 files · +237 -91
M internal/adapters/in/cli/push.go +4 -1
| @@ -206,8 +206,11 @@ func resolveFromImage(ctx context.Context, cp ControlPlane, imageArg, dockerfile | |||
| 206 | 206 | } | |
| 207 | 207 | ||
| 208 | 208 | if len(routes) == 0 { | |
| 209 | + | // bootstrap command is registered in bootstrap.go (see issue #98) | |
| 209 | 210 | return "", "", "", fmt.Errorf( | |
| 210 | - | "no route configured for image %q\n\nUse 'gordon routes add' to create a route first", | |
| 211 | + | "no route configured for image %q\n\nFor a first deploy, use 'gordon bootstrap <domain> %s'\nOr configure the route directly with 'gordon routes add <domain> %s'", | |
| 212 | + | imageArg, | |
| 213 | + | imageArg, | |
| 211 | 214 | imageArg, | |
| 212 | 215 | ) | |
| 213 | 216 | } | |
| @@ -206,8 +206,11 @@ func resolveFromImage(ctx context.Context, cp ControlPlane, imageArg, dockerfile | |||||
| 206 | } | 206 | } | ||
| 207 | 207 | ||||
| 208 | if len(routes) == 0 { | 208 | if len(routes) == 0 { | ||
| 209 | + | // bootstrap command is registered in bootstrap.go (see issue #98) | |||
| 209 | return "", "", "", fmt.Errorf( | 210 | return "", "", "", fmt.Errorf( | ||
| 210 | - | "no route configured for image %q\n\nUse 'gordon routes add' to create a route first", | 211 | + | "no route configured for image %q\n\nFor a first deploy, use 'gordon bootstrap <domain> %s'\nOr configure the route directly with 'gordon routes add <domain> %s'", |
| 212 | + | imageArg, | |||
| 213 | + | imageArg, | |||
| 211 | imageArg, | 214 | imageArg, | ||
| 212 | ) | 215 | ) | ||
| 213 | } | 216 | } | ||
M internal/adapters/in/cli/push_test.go +146 -0
| @@ -9,9 +9,141 @@ import ( | |||
| 9 | 9 | "strings" | |
| 10 | 10 | "testing" | |
| 11 | 11 | ||
| 12 | + | "github.com/bnema/gordon/internal/adapters/dto" | |
| 13 | + | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |
| 14 | + | "github.com/bnema/gordon/internal/domain" | |
| 12 | 15 | "github.com/stretchr/testify/assert" | |
| 13 | 16 | ) | |
| 14 | 17 | ||
| 18 | + | type resolveFromImageTestControlPlane struct { | |
| 19 | + | findRoutesByImage func(context.Context, string) ([]domain.Route, error) | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | var _ ControlPlane = (*resolveFromImageTestControlPlane)(nil) | |
| 23 | + | ||
| 24 | + | func (c *resolveFromImageTestControlPlane) ListRoutesWithDetails(context.Context) ([]remote.RouteInfo, error) { | |
| 25 | + | panic("unexpected call") | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func (c *resolveFromImageTestControlPlane) GetHealth(context.Context) (map[string]*remote.RouteHealth, error) { | |
| 29 | + | panic("unexpected call") | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | func (c *resolveFromImageTestControlPlane) GetRoute(context.Context, string) (*domain.Route, error) { | |
| 33 | + | panic("unexpected call") | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | func (c *resolveFromImageTestControlPlane) FindRoutesByImage(ctx context.Context, imageName string) ([]domain.Route, error) { | |
| 37 | + | if c.findRoutesByImage != nil { | |
| 38 | + | return c.findRoutesByImage(ctx, imageName) | |
| 39 | + | } | |
| 40 | + | return nil, nil | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | func (c *resolveFromImageTestControlPlane) AddRoute(context.Context, domain.Route) error { | |
| 44 | + | panic("unexpected call") | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func (c *resolveFromImageTestControlPlane) UpdateRoute(context.Context, domain.Route) error { | |
| 48 | + | panic("unexpected call") | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | func (c *resolveFromImageTestControlPlane) RemoveRoute(context.Context, string) error { | |
| 52 | + | panic("unexpected call") | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | func (c *resolveFromImageTestControlPlane) ListSecretsWithAttachments(context.Context, string) (*remote.SecretsListResult, error) { | |
| 56 | + | panic("unexpected call") | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | func (c *resolveFromImageTestControlPlane) SetSecrets(context.Context, string, map[string]string) error { | |
| 60 | + | panic("unexpected call") | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | func (c *resolveFromImageTestControlPlane) DeleteSecret(context.Context, string, string) error { | |
| 64 | + | panic("unexpected call") | |
| 65 | + | } | |
| 66 | + | ||
| 67 | + | func (c *resolveFromImageTestControlPlane) SetAttachmentSecrets(context.Context, string, string, map[string]string) error { | |
| 68 | + | panic("unexpected call") | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | func (c *resolveFromImageTestControlPlane) DeleteAttachmentSecret(context.Context, string, string, string) error { | |
| 72 | + | panic("unexpected call") | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func (c *resolveFromImageTestControlPlane) GetAllAttachmentsConfig(context.Context) (map[string][]string, error) { | |
| 76 | + | panic("unexpected call") | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | func (c *resolveFromImageTestControlPlane) GetAttachmentsConfig(context.Context, string) ([]string, error) { | |
| 80 | + | panic("unexpected call") | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | func (c *resolveFromImageTestControlPlane) AddAttachment(context.Context, string, string) error { | |
| 84 | + | panic("unexpected call") | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | func (c *resolveFromImageTestControlPlane) RemoveAttachment(context.Context, string, string) error { | |
| 88 | + | panic("unexpected call") | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | func (c *resolveFromImageTestControlPlane) GetStatus(context.Context) (*remote.Status, error) { | |
| 92 | + | panic("unexpected call") | |
| 93 | + | } | |
| 94 | + | ||
| 95 | + | func (c *resolveFromImageTestControlPlane) Reload(context.Context) error { | |
| 96 | + | panic("unexpected call") | |
| 97 | + | } | |
| 98 | + | ||
| 99 | + | func (c *resolveFromImageTestControlPlane) DeployIntent(context.Context, string) error { | |
| 100 | + | panic("unexpected call") | |
| 101 | + | } | |
| 102 | + | ||
| 103 | + | func (c *resolveFromImageTestControlPlane) Deploy(context.Context, string) (*remote.DeployResult, error) { | |
| 104 | + | panic("unexpected call") | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | func (c *resolveFromImageTestControlPlane) Restart(context.Context, string, bool) (*remote.RestartResult, error) { | |
| 108 | + | panic("unexpected call") | |
| 109 | + | } | |
| 110 | + | ||
| 111 | + | func (c *resolveFromImageTestControlPlane) ListTags(context.Context, string) ([]string, error) { | |
| 112 | + | panic("unexpected call") | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | func (c *resolveFromImageTestControlPlane) ListBackups(context.Context, string) ([]dto.BackupJob, error) { | |
| 116 | + | panic("unexpected call") | |
| 117 | + | } | |
| 118 | + | ||
| 119 | + | func (c *resolveFromImageTestControlPlane) BackupStatus(context.Context) ([]dto.BackupJob, error) { | |
| 120 | + | panic("unexpected call") | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | func (c *resolveFromImageTestControlPlane) RunBackup(context.Context, string, string) (*dto.BackupRunResponse, error) { | |
| 124 | + | panic("unexpected call") | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | func (c *resolveFromImageTestControlPlane) DetectDatabases(context.Context, string) ([]dto.DatabaseInfo, error) { | |
| 128 | + | panic("unexpected call") | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | func (c *resolveFromImageTestControlPlane) GetProcessLogs(context.Context, int) ([]string, error) { | |
| 132 | + | panic("unexpected call") | |
| 133 | + | } | |
| 134 | + | ||
| 135 | + | func (c *resolveFromImageTestControlPlane) GetContainerLogs(context.Context, string, int) ([]string, error) { | |
| 136 | + | panic("unexpected call") | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | func (c *resolveFromImageTestControlPlane) StreamProcessLogs(context.Context, int) (<-chan string, error) { | |
| 140 | + | panic("unexpected call") | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | func (c *resolveFromImageTestControlPlane) StreamContainerLogs(context.Context, string, int) (<-chan string, error) { | |
| 144 | + | panic("unexpected call") | |
| 145 | + | } | |
| 146 | + | ||
| 15 | 147 | func TestValidateBuildArg(t *testing.T) { | |
| 16 | 148 | tests := []struct { | |
| 17 | 149 | name string | |
| @@ -421,3 +553,17 @@ func TestParseLabelPair(t *testing.T) { | |||
| 421 | 553 | }) | |
| 422 | 554 | } | |
| 423 | 555 | } | |
| 556 | + | ||
| 557 | + | func TestResolveFromImage_NoRouteSuggestsBootstrap(t *testing.T) { | |
| 558 | + | cp := &resolveFromImageTestControlPlane{ | |
| 559 | + | findRoutesByImage: func(context.Context, string) ([]domain.Route, error) { | |
| 560 | + | return nil, nil | |
| 561 | + | }, | |
| 562 | + | } | |
| 563 | + | ||
| 564 | + | _, _, _, err := resolveFromImage(context.Background(), cp, "myapp", "Dockerfile") | |
| 565 | + | ||
| 566 | + | assert.Error(t, err) | |
| 567 | + | assert.Contains(t, err.Error(), `no route configured for image "myapp"`) | |
| 568 | + | assert.Contains(t, err.Error(), "gordon bootstrap") | |
| 569 | + | } | |
| @@ -9,9 +9,141 @@ import ( | |||||
| 9 | "strings" | 9 | "strings" | ||
| 10 | "testing" | 10 | "testing" | ||
| 11 | 11 | ||||
| 12 | + | "github.com/bnema/gordon/internal/adapters/dto" | |||
| 13 | + | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |||
| 14 | + | "github.com/bnema/gordon/internal/domain" | |||
| 12 | "github.com/stretchr/testify/assert" | 15 | "github.com/stretchr/testify/assert" | ||
| 13 | ) | 16 | ) | ||
| 14 | 17 | ||||
| 18 | + | type resolveFromImageTestControlPlane struct { | |||
| 19 | + | findRoutesByImage func(context.Context, string) ([]domain.Route, error) | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | var _ ControlPlane = (*resolveFromImageTestControlPlane)(nil) | |||
| 23 | + | ||||
| 24 | + | func (c *resolveFromImageTestControlPlane) ListRoutesWithDetails(context.Context) ([]remote.RouteInfo, error) { | |||
| 25 | + | panic("unexpected call") | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func (c *resolveFromImageTestControlPlane) GetHealth(context.Context) (map[string]*remote.RouteHealth, error) { | |||
| 29 | + | panic("unexpected call") | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | func (c *resolveFromImageTestControlPlane) GetRoute(context.Context, string) (*domain.Route, error) { | |||
| 33 | + | panic("unexpected call") | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | func (c *resolveFromImageTestControlPlane) FindRoutesByImage(ctx context.Context, imageName string) ([]domain.Route, error) { | |||
| 37 | + | if c.findRoutesByImage != nil { | |||
| 38 | + | return c.findRoutesByImage(ctx, imageName) | |||
| 39 | + | } | |||
| 40 | + | return nil, nil | |||
| 41 | + | } | |||
| 42 | + | ||||
| 43 | + | func (c *resolveFromImageTestControlPlane) AddRoute(context.Context, domain.Route) error { | |||
| 44 | + | panic("unexpected call") | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | func (c *resolveFromImageTestControlPlane) UpdateRoute(context.Context, domain.Route) error { | |||
| 48 | + | panic("unexpected call") | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | func (c *resolveFromImageTestControlPlane) RemoveRoute(context.Context, string) error { | |||
| 52 | + | panic("unexpected call") | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | func (c *resolveFromImageTestControlPlane) ListSecretsWithAttachments(context.Context, string) (*remote.SecretsListResult, error) { | |||
| 56 | + | panic("unexpected call") | |||
| 57 | + | } | |||
| 58 | + | ||||
| 59 | + | func (c *resolveFromImageTestControlPlane) SetSecrets(context.Context, string, map[string]string) error { | |||
| 60 | + | panic("unexpected call") | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | func (c *resolveFromImageTestControlPlane) DeleteSecret(context.Context, string, string) error { | |||
| 64 | + | panic("unexpected call") | |||
| 65 | + | } | |||
| 66 | + | ||||
| 67 | + | func (c *resolveFromImageTestControlPlane) SetAttachmentSecrets(context.Context, string, string, map[string]string) error { | |||
| 68 | + | panic("unexpected call") | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | func (c *resolveFromImageTestControlPlane) DeleteAttachmentSecret(context.Context, string, string, string) error { | |||
| 72 | + | panic("unexpected call") | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func (c *resolveFromImageTestControlPlane) GetAllAttachmentsConfig(context.Context) (map[string][]string, error) { | |||
| 76 | + | panic("unexpected call") | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | func (c *resolveFromImageTestControlPlane) GetAttachmentsConfig(context.Context, string) ([]string, error) { | |||
| 80 | + | panic("unexpected call") | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | func (c *resolveFromImageTestControlPlane) AddAttachment(context.Context, string, string) error { | |||
| 84 | + | panic("unexpected call") | |||
| 85 | + | } | |||
| 86 | + | ||||
| 87 | + | func (c *resolveFromImageTestControlPlane) RemoveAttachment(context.Context, string, string) error { | |||
| 88 | + | panic("unexpected call") | |||
| 89 | + | } | |||
| 90 | + | ||||
| 91 | + | func (c *resolveFromImageTestControlPlane) GetStatus(context.Context) (*remote.Status, error) { | |||
| 92 | + | panic("unexpected call") | |||
| 93 | + | } | |||
| 94 | + | ||||
| 95 | + | func (c *resolveFromImageTestControlPlane) Reload(context.Context) error { | |||
| 96 | + | panic("unexpected call") | |||
| 97 | + | } | |||
| 98 | + | ||||
| 99 | + | func (c *resolveFromImageTestControlPlane) DeployIntent(context.Context, string) error { | |||
| 100 | + | panic("unexpected call") | |||
| 101 | + | } | |||
| 102 | + | ||||
| 103 | + | func (c *resolveFromImageTestControlPlane) Deploy(context.Context, string) (*remote.DeployResult, error) { | |||
| 104 | + | panic("unexpected call") | |||
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | func (c *resolveFromImageTestControlPlane) Restart(context.Context, string, bool) (*remote.RestartResult, error) { | |||
| 108 | + | panic("unexpected call") | |||
| 109 | + | } | |||
| 110 | + | ||||
| 111 | + | func (c *resolveFromImageTestControlPlane) ListTags(context.Context, string) ([]string, error) { | |||
| 112 | + | panic("unexpected call") | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | func (c *resolveFromImageTestControlPlane) ListBackups(context.Context, string) ([]dto.BackupJob, error) { | |||
| 116 | + | panic("unexpected call") | |||
| 117 | + | } | |||
| 118 | + | ||||
| 119 | + | func (c *resolveFromImageTestControlPlane) BackupStatus(context.Context) ([]dto.BackupJob, error) { | |||
| 120 | + | panic("unexpected call") | |||
| 121 | + | } | |||
| 122 | + | ||||
| 123 | + | func (c *resolveFromImageTestControlPlane) RunBackup(context.Context, string, string) (*dto.BackupRunResponse, error) { | |||
| 124 | + | panic("unexpected call") | |||
| 125 | + | } | |||
| 126 | + | ||||
| 127 | + | func (c *resolveFromImageTestControlPlane) DetectDatabases(context.Context, string) ([]dto.DatabaseInfo, error) { | |||
| 128 | + | panic("unexpected call") | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | func (c *resolveFromImageTestControlPlane) GetProcessLogs(context.Context, int) ([]string, error) { | |||
| 132 | + | panic("unexpected call") | |||
| 133 | + | } | |||
| 134 | + | ||||
| 135 | + | func (c *resolveFromImageTestControlPlane) GetContainerLogs(context.Context, string, int) ([]string, error) { | |||
| 136 | + | panic("unexpected call") | |||
| 137 | + | } | |||
| 138 | + | ||||
| 139 | + | func (c *resolveFromImageTestControlPlane) StreamProcessLogs(context.Context, int) (<-chan string, error) { | |||
| 140 | + | panic("unexpected call") | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | func (c *resolveFromImageTestControlPlane) StreamContainerLogs(context.Context, string, int) (<-chan string, error) { | |||
| 144 | + | panic("unexpected call") | |||
| 145 | + | } | |||
| 146 | + | ||||
| 15 | func TestValidateBuildArg(t *testing.T) { | 147 | func TestValidateBuildArg(t *testing.T) { | ||
| 16 | tests := []struct { | 148 | tests := []struct { | ||
| 17 | name string | 149 | name string | ||
| @@ -421,3 +553,17 @@ func TestParseLabelPair(t *testing.T) { | |||||
| 421 | }) | 553 | }) | ||
| 422 | } | 554 | } | ||
| 423 | } | 555 | } | ||
| 556 | + | ||||
| 557 | + | func TestResolveFromImage_NoRouteSuggestsBootstrap(t *testing.T) { | |||
| 558 | + | cp := &resolveFromImageTestControlPlane{ | |||
| 559 | + | findRoutesByImage: func(context.Context, string) ([]domain.Route, error) { | |||
| 560 | + | return nil, nil | |||
| 561 | + | }, | |||
| 562 | + | } | |||
| 563 | + | ||||
| 564 | + | _, _, _, err := resolveFromImage(context.Background(), cp, "myapp", "Dockerfile") | |||
| 565 | + | ||||
| 566 | + | assert.Error(t, err) | |||
| 567 | + | assert.Contains(t, err.Error(), `no route configured for image "myapp"`) | |||
| 568 | + | assert.Contains(t, err.Error(), "gordon bootstrap") | |||
| 569 | + | } | |||
M internal/adapters/in/cli/routes.go +2 -2
| @@ -250,7 +250,7 @@ func newRoutesAddCmd() *cobra.Command { | |||
| 250 | 250 | ||
| 251 | 251 | cmd := &cobra.Command{ | |
| 252 | 252 | Use: "add <domain> <image>", | |
| 253 | - | Short: "Add a new route", | |
| 253 | + | Short: "Create or update a route", | |
| 254 | 254 | Long: `Add a new route mapping a domain to a container image. | |
| 255 | 255 | ||
| 256 | 256 | Examples: | |
| @@ -299,7 +299,7 @@ Examples: | |||
| 299 | 299 | } | |
| 300 | 300 | } | |
| 301 | 301 | ||
| 302 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Route added: %s -> %s", routeDomain, image))) | |
| 302 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Route configured: %s -> %s", routeDomain, image))) | |
| 303 | 303 | return nil | |
| 304 | 304 | }, | |
| 305 | 305 | } | |
| @@ -250,7 +250,7 @@ func newRoutesAddCmd() *cobra.Command { | |||||
| 250 | 250 | ||||
| 251 | cmd := &cobra.Command{ | 251 | cmd := &cobra.Command{ | ||
| 252 | Use: "add <domain> <image>", | 252 | Use: "add <domain> <image>", | ||
| 253 | - | Short: "Add a new route", | 253 | + | Short: "Create or update a route", |
| 254 | Long: `Add a new route mapping a domain to a container image. | 254 | Long: `Add a new route mapping a domain to a container image. | ||
| 255 | 255 | ||||
| 256 | Examples: | 256 | Examples: | ||
| @@ -299,7 +299,7 @@ Examples: | |||||
| 299 | } | 299 | } | ||
| 300 | } | 300 | } | ||
| 301 | 301 | ||||
| 302 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Route added: %s -> %s", routeDomain, image))) | 302 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Route configured: %s -> %s", routeDomain, image))) |
| 303 | return nil | 303 | return nil | ||
| 304 | }, | 304 | }, | ||
| 305 | } | 305 | } | ||
M internal/adapters/in/http/admin/handler.go +0 -30
| @@ -385,21 +385,6 @@ func (h *Handler) handleRoutesPost(w http.ResponseWriter, r *http.Request) { | |||
| 385 | 385 | return | |
| 386 | 386 | } | |
| 387 | 387 | ||
| 388 | - | // Validate image exists in registry before adding route | |
| 389 | - | if h.registrySvc != nil && route.Image != "" { | |
| 390 | - | imageName, imageRef := validation.ParseImageReference(route.Image) | |
| 391 | - | if _, err := h.registrySvc.GetManifest(ctx, imageName, imageRef); err != nil { | |
| 392 | - | if errors.Is(err, domain.ErrManifestNotFound) { | |
| 393 | - | log.Error().Err(err).Str("image", route.Image).Msg("image not found in registry") | |
| 394 | - | h.sendError(w, http.StatusBadRequest, fmt.Sprintf("image '%s' not found in Gordon's registry. Please push it first using: gordon push %s", route.Image, route.Image)) | |
| 395 | - | } else { | |
| 396 | - | log.Error().Err(err).Str("image", route.Image).Msg("failed to check image in registry") | |
| 397 | - | h.sendError(w, http.StatusServiceUnavailable, "failed to verify image in registry") | |
| 398 | - | } | |
| 399 | - | return | |
| 400 | - | } | |
| 401 | - | } | |
| 402 | - | ||
| 403 | 388 | if err := h.configSvc.AddRoute(ctx, route); err != nil { | |
| 404 | 389 | log.Error().Err(err).Str("domain", route.Domain).Msg("failed to add route") | |
| 405 | 390 | switch { | |
| @@ -442,21 +427,6 @@ func (h *Handler) handleRoutesPut(w http.ResponseWriter, r *http.Request, routeD | |||
| 442 | 427 | ||
| 443 | 428 | route.Domain = routeDomain | |
| 444 | 429 | ||
| 445 | - | // Validate image exists in registry before updating route | |
| 446 | - | if h.registrySvc != nil && route.Image != "" { | |
| 447 | - | imageName, imageRef := validation.ParseImageReference(route.Image) | |
| 448 | - | if _, err := h.registrySvc.GetManifest(ctx, imageName, imageRef); err != nil { | |
| 449 | - | if errors.Is(err, domain.ErrManifestNotFound) { | |
| 450 | - | log.Error().Err(err).Str("image", route.Image).Msg("image not found in registry") | |
| 451 | - | h.sendError(w, http.StatusBadRequest, fmt.Sprintf("image '%s' not found in Gordon's registry. Please push it first using: gordon push %s", route.Image, route.Image)) | |
| 452 | - | } else { | |
| 453 | - | log.Error().Err(err).Str("image", route.Image).Msg("failed to check image in registry") | |
| 454 | - | h.sendError(w, http.StatusServiceUnavailable, "failed to verify image in registry") | |
| 455 | - | } | |
| 456 | - | return | |
| 457 | - | } | |
| 458 | - | } | |
| 459 | - | ||
| 460 | 430 | if err := h.configSvc.UpdateRoute(ctx, route); err != nil { | |
| 461 | 431 | log.Error().Err(err).Str("domain", routeDomain).Msg("failed to update route") | |
| 462 | 432 | switch { | |
| @@ -385,21 +385,6 @@ func (h *Handler) handleRoutesPost(w http.ResponseWriter, r *http.Request) { | |||||
| 385 | return | 385 | return | ||
| 386 | } | 386 | } | ||
| 387 | 387 | ||||
| 388 | - | // Validate image exists in registry before adding route | |||
| 389 | - | if h.registrySvc != nil && route.Image != "" { | |||
| 390 | - | imageName, imageRef := validation.ParseImageReference(route.Image) | |||
| 391 | - | if _, err := h.registrySvc.GetManifest(ctx, imageName, imageRef); err != nil { | |||
| 392 | - | if errors.Is(err, domain.ErrManifestNotFound) { | |||
| 393 | - | log.Error().Err(err).Str("image", route.Image).Msg("image not found in registry") | |||
| 394 | - | h.sendError(w, http.StatusBadRequest, fmt.Sprintf("image '%s' not found in Gordon's registry. Please push it first using: gordon push %s", route.Image, route.Image)) | |||
| 395 | - | } else { | |||
| 396 | - | log.Error().Err(err).Str("image", route.Image).Msg("failed to check image in registry") | |||
| 397 | - | h.sendError(w, http.StatusServiceUnavailable, "failed to verify image in registry") | |||
| 398 | - | } | |||
| 399 | - | return | |||
| 400 | - | } | |||
| 401 | - | } | |||
| 402 | - | ||||
| 403 | if err := h.configSvc.AddRoute(ctx, route); err != nil { | 388 | if err := h.configSvc.AddRoute(ctx, route); err != nil { | ||
| 404 | log.Error().Err(err).Str("domain", route.Domain).Msg("failed to add route") | 389 | log.Error().Err(err).Str("domain", route.Domain).Msg("failed to add route") | ||
| 405 | switch { | 390 | switch { | ||
| @@ -442,21 +427,6 @@ func (h *Handler) handleRoutesPut(w http.ResponseWriter, r *http.Request, routeD | |||||
| 442 | 427 | ||||
| 443 | route.Domain = routeDomain | 428 | route.Domain = routeDomain | ||
| 444 | 429 | ||||
| 445 | - | // Validate image exists in registry before updating route | |||
| 446 | - | if h.registrySvc != nil && route.Image != "" { | |||
| 447 | - | imageName, imageRef := validation.ParseImageReference(route.Image) | |||
| 448 | - | if _, err := h.registrySvc.GetManifest(ctx, imageName, imageRef); err != nil { | |||
| 449 | - | if errors.Is(err, domain.ErrManifestNotFound) { | |||
| 450 | - | log.Error().Err(err).Str("image", route.Image).Msg("image not found in registry") | |||
| 451 | - | h.sendError(w, http.StatusBadRequest, fmt.Sprintf("image '%s' not found in Gordon's registry. Please push it first using: gordon push %s", route.Image, route.Image)) | |||
| 452 | - | } else { | |||
| 453 | - | log.Error().Err(err).Str("image", route.Image).Msg("failed to check image in registry") | |||
| 454 | - | h.sendError(w, http.StatusServiceUnavailable, "failed to verify image in registry") | |||
| 455 | - | } | |||
| 456 | - | return | |||
| 457 | - | } | |||
| 458 | - | } | |||
| 459 | - | ||||
| 460 | if err := h.configSvc.UpdateRoute(ctx, route); err != nil { | 430 | if err := h.configSvc.UpdateRoute(ctx, route); err != nil { | ||
| 461 | log.Error().Err(err).Str("domain", routeDomain).Msg("failed to update route") | 431 | log.Error().Err(err).Str("domain", routeDomain).Msg("failed to update route") | ||
| 462 | switch { | 432 | switch { | ||
M internal/adapters/in/http/admin/handler_test.go +46 -58
| @@ -158,7 +158,6 @@ func TestHandler_RoutesPost_RequiresWriteScope(t *testing.T) { | |||
| 158 | 158 | t.Run(tt.name, func(t *testing.T) { | |
| 159 | 159 | if tt.wantStatus == http.StatusCreated { | |
| 160 | 160 | configSvc.EXPECT().AddRoute(mock.Anything, mock.AnythingOfType("domain.Route")).Return(nil).Maybe() | |
| 161 | - | registrySvc.EXPECT().GetManifest(mock.Anything, "myapp", "latest").Return(nil, nil).Maybe() | |
| 162 | 161 | } | |
| 163 | 162 | ||
| 164 | 163 | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(routeJSON)) | |
| @@ -204,7 +203,6 @@ func TestHandler_RoutesPut_RequiresWriteScope(t *testing.T) { | |||
| 204 | 203 | t.Run(tt.name, func(t *testing.T) { | |
| 205 | 204 | if tt.wantStatus == http.StatusOK { | |
| 206 | 205 | configSvc.EXPECT().UpdateRoute(mock.Anything, mock.AnythingOfType("domain.Route")).Return(nil).Maybe() | |
| 207 | - | registrySvc.EXPECT().GetManifest(mock.Anything, "myapp", "v2").Return(nil, nil).Maybe() | |
| 208 | 206 | } | |
| 209 | 207 | ||
| 210 | 208 | req := httptest.NewRequest("PUT", "/admin/routes/app.example.com", bytes.NewBufferString(routeJSON)) | |
| @@ -218,6 +216,52 @@ func TestHandler_RoutesPut_RequiresWriteScope(t *testing.T) { | |||
| 218 | 216 | } | |
| 219 | 217 | } | |
| 220 | 218 | ||
| 219 | + | func TestHandler_RoutesPost_AllowsRouteCreationWithoutRegistryManifest(t *testing.T) { | |
| 220 | + | configSvc := inmocks.NewMockConfigService(t) | |
| 221 | + | authSvc := inmocks.NewMockAuthService(t) | |
| 222 | + | containerSvc := inmocks.NewMockContainerService(t) | |
| 223 | + | secretSvc := inmocks.NewMockSecretService(t) | |
| 224 | + | registrySvc := inmocks.NewMockRegistryService(t) | |
| 225 | + | route := domain.Route{Domain: "app.example.com", Image: "myapp:latest"} | |
| 226 | + | ||
| 227 | + | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |
| 228 | + | ||
| 229 | + | configSvc.EXPECT().AddRoute(mock.Anything, route).Return(nil).Once() | |
| 230 | + | ||
| 231 | + | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(`{"domain":"app.example.com","image":"myapp:latest"}`)) | |
| 232 | + | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |
| 233 | + | rec := httptest.NewRecorder() | |
| 234 | + | ||
| 235 | + | handler.ServeHTTP(rec, req) | |
| 236 | + | ||
| 237 | + | assert.Equal(t, http.StatusCreated, rec.Code) | |
| 238 | + | assert.JSONEq(t, `{"domain":"app.example.com","image":"myapp:latest","https":false}`, rec.Body.String()) | |
| 239 | + | registrySvc.AssertNotCalled(t, "GetManifest", mock.Anything, mock.Anything, mock.Anything) | |
| 240 | + | } | |
| 241 | + | ||
| 242 | + | func TestHandler_RoutesPut_AllowsRouteUpdateWithoutRegistryManifest(t *testing.T) { | |
| 243 | + | configSvc := inmocks.NewMockConfigService(t) | |
| 244 | + | authSvc := inmocks.NewMockAuthService(t) | |
| 245 | + | containerSvc := inmocks.NewMockContainerService(t) | |
| 246 | + | secretSvc := inmocks.NewMockSecretService(t) | |
| 247 | + | registrySvc := inmocks.NewMockRegistryService(t) | |
| 248 | + | route := domain.Route{Domain: "app.example.com", Image: "myapp:v2"} | |
| 249 | + | ||
| 250 | + | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |
| 251 | + | ||
| 252 | + | configSvc.EXPECT().UpdateRoute(mock.Anything, route).Return(nil).Once() | |
| 253 | + | ||
| 254 | + | req := httptest.NewRequest("PUT", "/admin/routes/app.example.com", bytes.NewBufferString(`{"image":"myapp:v2"}`)) | |
| 255 | + | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |
| 256 | + | rec := httptest.NewRecorder() | |
| 257 | + | ||
| 258 | + | handler.ServeHTTP(rec, req) | |
| 259 | + | ||
| 260 | + | assert.Equal(t, http.StatusOK, rec.Code) | |
| 261 | + | assert.JSONEq(t, `{"domain":"app.example.com","image":"myapp:v2","https":false}`, rec.Body.String()) | |
| 262 | + | registrySvc.AssertNotCalled(t, "GetManifest", mock.Anything, mock.Anything, mock.Anything) | |
| 263 | + | } | |
| 264 | + | ||
| 221 | 265 | func TestHandler_RoutesDelete_RequiresWriteScope(t *testing.T) { | |
| 222 | 266 | configSvc := inmocks.NewMockConfigService(t) | |
| 223 | 267 | authSvc := inmocks.NewMockAuthService(t) | |
| @@ -749,61 +793,6 @@ func TestHandler_RoutesPost_MissingFields(t *testing.T) { | |||
| 749 | 793 | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |
| 750 | 794 | ||
| 751 | 795 | configSvc.EXPECT().AddRoute(mock.Anything, mock.Anything).Return(tt.mockErr) | |
| 752 | - | registrySvc.EXPECT().GetManifest(mock.Anything, mock.Anything, mock.Anything).Return(nil, nil).Maybe() | |
| 753 | - | ||
| 754 | - | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(tt.body)) | |
| 755 | - | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |
| 756 | - | rec := httptest.NewRecorder() | |
| 757 | - | ||
| 758 | - | handler.ServeHTTP(rec, req) | |
| 759 | - | ||
| 760 | - | assert.Equal(t, http.StatusBadRequest, rec.Code) | |
| 761 | - | }) | |
| 762 | - | } | |
| 763 | - | } | |
| 764 | - | ||
| 765 | - | func TestHandler_RoutesPost_ImageNotFound(t *testing.T) { | |
| 766 | - | configSvc := inmocks.NewMockConfigService(t) | |
| 767 | - | authSvc := inmocks.NewMockAuthService(t) | |
| 768 | - | containerSvc := inmocks.NewMockContainerService(t) | |
| 769 | - | secretSvc := inmocks.NewMockSecretService(t) | |
| 770 | - | registrySvc := inmocks.NewMockRegistryService(t) | |
| 771 | - | ||
| 772 | - | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |
| 773 | - | ||
| 774 | - | tests := []struct { | |
| 775 | - | name string | |
| 776 | - | image string | |
| 777 | - | body string | |
| 778 | - | manifestName string | |
| 779 | - | manifestRef string | |
| 780 | - | }{ | |
| 781 | - | { | |
| 782 | - | name: "tagged image not found", | |
| 783 | - | image: "myapp:latest", | |
| 784 | - | body: `{"domain": "app.example.com", "image": "myapp:latest"}`, | |
| 785 | - | manifestName: "myapp", | |
| 786 | - | manifestRef: "latest", | |
| 787 | - | }, | |
| 788 | - | { | |
| 789 | - | name: "digest image not found", | |
| 790 | - | image: "myapp@sha256:abc123", | |
| 791 | - | body: `{"domain": "app.example.com", "image": "myapp@sha256:abc123"}`, | |
| 792 | - | manifestName: "myapp", | |
| 793 | - | manifestRef: "sha256:abc123", | |
| 794 | - | }, | |
| 795 | - | { | |
| 796 | - | name: "implicit latest tag not found", | |
| 797 | - | image: "myapp", | |
| 798 | - | body: `{"domain": "app.example.com", "image": "myapp"}`, | |
| 799 | - | manifestName: "myapp", | |
| 800 | - | manifestRef: "latest", | |
| 801 | - | }, | |
| 802 | - | } | |
| 803 | - | ||
| 804 | - | for _, tt := range tests { | |
| 805 | - | t.Run(tt.name, func(t *testing.T) { | |
| 806 | - | registrySvc.EXPECT().GetManifest(mock.Anything, tt.manifestName, tt.manifestRef).Return(nil, domain.ErrManifestNotFound) | |
| 807 | 796 | ||
| 808 | 797 | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(tt.body)) | |
| 809 | 798 | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |
| @@ -812,7 +801,6 @@ func TestHandler_RoutesPost_ImageNotFound(t *testing.T) { | |||
| 812 | 801 | handler.ServeHTTP(rec, req) | |
| 813 | 802 | ||
| 814 | 803 | assert.Equal(t, http.StatusBadRequest, rec.Code) | |
| 815 | - | assert.Contains(t, rec.Body.String(), fmt.Sprintf("image '%s' not found", tt.image)) | |
| 816 | 804 | }) | |
| 817 | 805 | } | |
| 818 | 806 | } | |
| @@ -158,7 +158,6 @@ func TestHandler_RoutesPost_RequiresWriteScope(t *testing.T) { | |||||
| 158 | t.Run(tt.name, func(t *testing.T) { | 158 | t.Run(tt.name, func(t *testing.T) { | ||
| 159 | if tt.wantStatus == http.StatusCreated { | 159 | if tt.wantStatus == http.StatusCreated { | ||
| 160 | configSvc.EXPECT().AddRoute(mock.Anything, mock.AnythingOfType("domain.Route")).Return(nil).Maybe() | 160 | configSvc.EXPECT().AddRoute(mock.Anything, mock.AnythingOfType("domain.Route")).Return(nil).Maybe() | ||
| 161 | - | registrySvc.EXPECT().GetManifest(mock.Anything, "myapp", "latest").Return(nil, nil).Maybe() | |||
| 162 | } | 161 | } | ||
| 163 | 162 | ||||
| 164 | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(routeJSON)) | 163 | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(routeJSON)) | ||
| @@ -204,7 +203,6 @@ func TestHandler_RoutesPut_RequiresWriteScope(t *testing.T) { | |||||
| 204 | t.Run(tt.name, func(t *testing.T) { | 203 | t.Run(tt.name, func(t *testing.T) { | ||
| 205 | if tt.wantStatus == http.StatusOK { | 204 | if tt.wantStatus == http.StatusOK { | ||
| 206 | configSvc.EXPECT().UpdateRoute(mock.Anything, mock.AnythingOfType("domain.Route")).Return(nil).Maybe() | 205 | configSvc.EXPECT().UpdateRoute(mock.Anything, mock.AnythingOfType("domain.Route")).Return(nil).Maybe() | ||
| 207 | - | registrySvc.EXPECT().GetManifest(mock.Anything, "myapp", "v2").Return(nil, nil).Maybe() | |||
| 208 | } | 206 | } | ||
| 209 | 207 | ||||
| 210 | req := httptest.NewRequest("PUT", "/admin/routes/app.example.com", bytes.NewBufferString(routeJSON)) | 208 | req := httptest.NewRequest("PUT", "/admin/routes/app.example.com", bytes.NewBufferString(routeJSON)) | ||
| @@ -218,6 +216,52 @@ func TestHandler_RoutesPut_RequiresWriteScope(t *testing.T) { | |||||
| 218 | } | 216 | } | ||
| 219 | } | 217 | } | ||
| 220 | 218 | ||||
| 219 | + | func TestHandler_RoutesPost_AllowsRouteCreationWithoutRegistryManifest(t *testing.T) { | |||
| 220 | + | configSvc := inmocks.NewMockConfigService(t) | |||
| 221 | + | authSvc := inmocks.NewMockAuthService(t) | |||
| 222 | + | containerSvc := inmocks.NewMockContainerService(t) | |||
| 223 | + | secretSvc := inmocks.NewMockSecretService(t) | |||
| 224 | + | registrySvc := inmocks.NewMockRegistryService(t) | |||
| 225 | + | route := domain.Route{Domain: "app.example.com", Image: "myapp:latest"} | |||
| 226 | + | ||||
| 227 | + | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |||
| 228 | + | ||||
| 229 | + | configSvc.EXPECT().AddRoute(mock.Anything, route).Return(nil).Once() | |||
| 230 | + | ||||
| 231 | + | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(`{"domain":"app.example.com","image":"myapp:latest"}`)) | |||
| 232 | + | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |||
| 233 | + | rec := httptest.NewRecorder() | |||
| 234 | + | ||||
| 235 | + | handler.ServeHTTP(rec, req) | |||
| 236 | + | ||||
| 237 | + | assert.Equal(t, http.StatusCreated, rec.Code) | |||
| 238 | + | assert.JSONEq(t, `{"domain":"app.example.com","image":"myapp:latest","https":false}`, rec.Body.String()) | |||
| 239 | + | registrySvc.AssertNotCalled(t, "GetManifest", mock.Anything, mock.Anything, mock.Anything) | |||
| 240 | + | } | |||
| 241 | + | ||||
| 242 | + | func TestHandler_RoutesPut_AllowsRouteUpdateWithoutRegistryManifest(t *testing.T) { | |||
| 243 | + | configSvc := inmocks.NewMockConfigService(t) | |||
| 244 | + | authSvc := inmocks.NewMockAuthService(t) | |||
| 245 | + | containerSvc := inmocks.NewMockContainerService(t) | |||
| 246 | + | secretSvc := inmocks.NewMockSecretService(t) | |||
| 247 | + | registrySvc := inmocks.NewMockRegistryService(t) | |||
| 248 | + | route := domain.Route{Domain: "app.example.com", Image: "myapp:v2"} | |||
| 249 | + | ||||
| 250 | + | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |||
| 251 | + | ||||
| 252 | + | configSvc.EXPECT().UpdateRoute(mock.Anything, route).Return(nil).Once() | |||
| 253 | + | ||||
| 254 | + | req := httptest.NewRequest("PUT", "/admin/routes/app.example.com", bytes.NewBufferString(`{"image":"myapp:v2"}`)) | |||
| 255 | + | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |||
| 256 | + | rec := httptest.NewRecorder() | |||
| 257 | + | ||||
| 258 | + | handler.ServeHTTP(rec, req) | |||
| 259 | + | ||||
| 260 | + | assert.Equal(t, http.StatusOK, rec.Code) | |||
| 261 | + | assert.JSONEq(t, `{"domain":"app.example.com","image":"myapp:v2","https":false}`, rec.Body.String()) | |||
| 262 | + | registrySvc.AssertNotCalled(t, "GetManifest", mock.Anything, mock.Anything, mock.Anything) | |||
| 263 | + | } | |||
| 264 | + | ||||
| 221 | func TestHandler_RoutesDelete_RequiresWriteScope(t *testing.T) { | 265 | func TestHandler_RoutesDelete_RequiresWriteScope(t *testing.T) { | ||
| 222 | configSvc := inmocks.NewMockConfigService(t) | 266 | configSvc := inmocks.NewMockConfigService(t) | ||
| 223 | authSvc := inmocks.NewMockAuthService(t) | 267 | authSvc := inmocks.NewMockAuthService(t) | ||
| @@ -749,61 +793,6 @@ func TestHandler_RoutesPost_MissingFields(t *testing.T) { | |||||
| 749 | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | 793 | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | ||
| 750 | 794 | ||||
| 751 | configSvc.EXPECT().AddRoute(mock.Anything, mock.Anything).Return(tt.mockErr) | 795 | configSvc.EXPECT().AddRoute(mock.Anything, mock.Anything).Return(tt.mockErr) | ||
| 752 | - | registrySvc.EXPECT().GetManifest(mock.Anything, mock.Anything, mock.Anything).Return(nil, nil).Maybe() | |||
| 753 | - | ||||
| 754 | - | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(tt.body)) | |||
| 755 | - | req = req.WithContext(ctxWithScopes("admin:routes:write")) | |||
| 756 | - | rec := httptest.NewRecorder() | |||
| 757 | - | ||||
| 758 | - | handler.ServeHTTP(rec, req) | |||
| 759 | - | ||||
| 760 | - | assert.Equal(t, http.StatusBadRequest, rec.Code) | |||
| 761 | - | }) | |||
| 762 | - | } | |||
| 763 | - | } | |||
| 764 | - | ||||
| 765 | - | func TestHandler_RoutesPost_ImageNotFound(t *testing.T) { | |||
| 766 | - | configSvc := inmocks.NewMockConfigService(t) | |||
| 767 | - | authSvc := inmocks.NewMockAuthService(t) | |||
| 768 | - | containerSvc := inmocks.NewMockContainerService(t) | |||
| 769 | - | secretSvc := inmocks.NewMockSecretService(t) | |||
| 770 | - | registrySvc := inmocks.NewMockRegistryService(t) | |||
| 771 | - | ||||
| 772 | - | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, registrySvc, nil, testLogger(), nil) | |||
| 773 | - | ||||
| 774 | - | tests := []struct { | |||
| 775 | - | name string | |||
| 776 | - | image string | |||
| 777 | - | body string | |||
| 778 | - | manifestName string | |||
| 779 | - | manifestRef string | |||
| 780 | - | }{ | |||
| 781 | - | { | |||
| 782 | - | name: "tagged image not found", | |||
| 783 | - | image: "myapp:latest", | |||
| 784 | - | body: `{"domain": "app.example.com", "image": "myapp:latest"}`, | |||
| 785 | - | manifestName: "myapp", | |||
| 786 | - | manifestRef: "latest", | |||
| 787 | - | }, | |||
| 788 | - | { | |||
| 789 | - | name: "digest image not found", | |||
| 790 | - | image: "myapp@sha256:abc123", | |||
| 791 | - | body: `{"domain": "app.example.com", "image": "myapp@sha256:abc123"}`, | |||
| 792 | - | manifestName: "myapp", | |||
| 793 | - | manifestRef: "sha256:abc123", | |||
| 794 | - | }, | |||
| 795 | - | { | |||
| 796 | - | name: "implicit latest tag not found", | |||
| 797 | - | image: "myapp", | |||
| 798 | - | body: `{"domain": "app.example.com", "image": "myapp"}`, | |||
| 799 | - | manifestName: "myapp", | |||
| 800 | - | manifestRef: "latest", | |||
| 801 | - | }, | |||
| 802 | - | } | |||
| 803 | - | ||||
| 804 | - | for _, tt := range tests { | |||
| 805 | - | t.Run(tt.name, func(t *testing.T) { | |||
| 806 | - | registrySvc.EXPECT().GetManifest(mock.Anything, tt.manifestName, tt.manifestRef).Return(nil, domain.ErrManifestNotFound) | |||
| 807 | 796 | ||||
| 808 | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(tt.body)) | 797 | req := httptest.NewRequest("POST", "/admin/routes", bytes.NewBufferString(tt.body)) | ||
| 809 | req = req.WithContext(ctxWithScopes("admin:routes:write")) | 798 | req = req.WithContext(ctxWithScopes("admin:routes:write")) | ||
| @@ -812,7 +801,6 @@ func TestHandler_RoutesPost_ImageNotFound(t *testing.T) { | |||||
| 812 | handler.ServeHTTP(rec, req) | 801 | handler.ServeHTTP(rec, req) | ||
| 813 | 802 | ||||
| 814 | assert.Equal(t, http.StatusBadRequest, rec.Code) | 803 | assert.Equal(t, http.StatusBadRequest, rec.Code) | ||
| 815 | - | assert.Contains(t, rec.Body.String(), fmt.Sprintf("image '%s' not found", tt.image)) | |||
| 816 | }) | 804 | }) | ||
| 817 | } | 805 | } | ||
| 818 | } | 806 | } | ||
M internal/usecase/config/service.go +4 -0
| @@ -315,6 +315,10 @@ func (s *Service) AddRoute(ctx context.Context, route domain.Route) error { | |||
| 315 | 315 | if s.config.Routes == nil { | |
| 316 | 316 | s.config.Routes = make(map[string]string) | |
| 317 | 317 | } | |
| 318 | + | if existed && previousImage == route.Image { | |
| 319 | + | s.mu.Unlock() | |
| 320 | + | return nil | |
| 321 | + | } | |
| 318 | 322 | s.config.Routes[route.Domain] = route.Image | |
| 319 | 323 | s.mu.Unlock() | |
| 320 | 324 | ||
| @@ -315,6 +315,10 @@ func (s *Service) AddRoute(ctx context.Context, route domain.Route) error { | |||||
| 315 | if s.config.Routes == nil { | 315 | if s.config.Routes == nil { | ||
| 316 | s.config.Routes = make(map[string]string) | 316 | s.config.Routes = make(map[string]string) | ||
| 317 | } | 317 | } | ||
| 318 | + | if existed && previousImage == route.Image { | |||
| 319 | + | s.mu.Unlock() | |||
| 320 | + | return nil | |||
| 321 | + | } | |||
| 318 | s.config.Routes[route.Domain] = route.Image | 322 | s.config.Routes[route.Domain] = route.Image | ||
| 319 | s.mu.Unlock() | 323 | s.mu.Unlock() | ||
| 320 | 324 | ||||
M internal/usecase/config/service_test.go +35 -0
| @@ -260,6 +260,41 @@ func TestService_AddRoute(t *testing.T) { | |||
| 260 | 260 | assert.Equal(t, "newapp:latest", config.Routes["new.example.com"]) | |
| 261 | 261 | }) | |
| 262 | 262 | ||
| 263 | + | t.Run("idempotent same domain and image saves once", func(t *testing.T) { | |
| 264 | + | tmpDir := t.TempDir() | |
| 265 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |
| 266 | + | err := os.WriteFile(configFile, []byte("[routes]\n"), 0600) | |
| 267 | + | require.NoError(t, err) | |
| 268 | + | ||
| 269 | + | v := viper.New() | |
| 270 | + | v.SetConfigFile(configFile) | |
| 271 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 272 | + | svc := NewService(v, eventBus) | |
| 273 | + | ctx := testContext() | |
| 274 | + | ||
| 275 | + | _ = svc.Load(ctx) | |
| 276 | + | ||
| 277 | + | route := domain.Route{ | |
| 278 | + | Domain: "new.example.com", | |
| 279 | + | Image: "newapp:latest", | |
| 280 | + | } | |
| 281 | + | ||
| 282 | + | err = svc.AddRoute(ctx, route) | |
| 283 | + | require.NoError(t, err) | |
| 284 | + | ||
| 285 | + | before, err := os.Stat(configFile) | |
| 286 | + | require.NoError(t, err) | |
| 287 | + | ||
| 288 | + | err = svc.AddRoute(ctx, route) | |
| 289 | + | require.NoError(t, err) | |
| 290 | + | ||
| 291 | + | after, err := os.Stat(configFile) | |
| 292 | + | require.NoError(t, err) | |
| 293 | + | ||
| 294 | + | assert.Equal(t, before.ModTime(), after.ModTime()) | |
| 295 | + | assert.Equal(t, "newapp:latest", svc.GetConfig().Routes["new.example.com"]) | |
| 296 | + | }) | |
| 297 | + | ||
| 263 | 298 | t.Run("empty domain", func(t *testing.T) { | |
| 264 | 299 | v := viper.New() | |
| 265 | 300 | eventBus := mocks.NewMockEventPublisher(t) | |
| @@ -260,6 +260,41 @@ func TestService_AddRoute(t *testing.T) { | |||||
| 260 | assert.Equal(t, "newapp:latest", config.Routes["new.example.com"]) | 260 | assert.Equal(t, "newapp:latest", config.Routes["new.example.com"]) | ||
| 261 | }) | 261 | }) | ||
| 262 | 262 | ||||
| 263 | + | t.Run("idempotent same domain and image saves once", func(t *testing.T) { | |||
| 264 | + | tmpDir := t.TempDir() | |||
| 265 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |||
| 266 | + | err := os.WriteFile(configFile, []byte("[routes]\n"), 0600) | |||
| 267 | + | require.NoError(t, err) | |||
| 268 | + | ||||
| 269 | + | v := viper.New() | |||
| 270 | + | v.SetConfigFile(configFile) | |||
| 271 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 272 | + | svc := NewService(v, eventBus) | |||
| 273 | + | ctx := testContext() | |||
| 274 | + | ||||
| 275 | + | _ = svc.Load(ctx) | |||
| 276 | + | ||||
| 277 | + | route := domain.Route{ | |||
| 278 | + | Domain: "new.example.com", | |||
| 279 | + | Image: "newapp:latest", | |||
| 280 | + | } | |||
| 281 | + | ||||
| 282 | + | err = svc.AddRoute(ctx, route) | |||
| 283 | + | require.NoError(t, err) | |||
| 284 | + | ||||
| 285 | + | before, err := os.Stat(configFile) | |||
| 286 | + | require.NoError(t, err) | |||
| 287 | + | ||||
| 288 | + | err = svc.AddRoute(ctx, route) | |||
| 289 | + | require.NoError(t, err) | |||
| 290 | + | ||||
| 291 | + | after, err := os.Stat(configFile) | |||
| 292 | + | require.NoError(t, err) | |||
| 293 | + | ||||
| 294 | + | assert.Equal(t, before.ModTime(), after.ModTime()) | |||
| 295 | + | assert.Equal(t, "newapp:latest", svc.GetConfig().Routes["new.example.com"]) | |||
| 296 | + | }) | |||
| 297 | + | ||||
| 263 | t.Run("empty domain", func(t *testing.T) { | 298 | t.Run("empty domain", func(t *testing.T) { | ||
| 264 | v := viper.New() | 299 | v := viper.New() | ||
| 265 | eventBus := mocks.NewMockEventPublisher(t) | 300 | eventBus := mocks.NewMockEventPublisher(t) | ||