From 8a0a3293e59ef6394bdf7def60f5c32c74b45073 Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 29 Jul 2026 09:27:47 +0000 Subject: [PATCH 1/3] feat: simplify linked service manifests Amp-Thread-ID: https://ampcode.com/threads/T-019fac48-72fe-7706-9ce1-45eb90eadbef Co-authored-by: Arjun Komath --- cli/internal/cli/app.go | 225 +++++++++--------- cli/internal/cli/app_test.go | 127 +++++++--- cli/internal/cli/types.go | 51 +++- cli/internal/manifest/manifest.go | 33 +-- cli/internal/manifest/manifest_test.go | 4 +- docs/api/public-api.mdx | 25 +- .../services/[serviceId]/route.ts | 47 ---- .../services/[serviceId]/builds/route.ts | 0 .../[serviceId]/configuration/route.ts | 2 +- .../services/[serviceId]/deploy/route.ts | 0 .../services/[serviceId]/logs/route.ts | 0 .../services/[serviceId]/metrics/route.ts | 0 .../services/[serviceId]/revisions/route.ts | 0 .../rollouts/[rolloutId]/logs/route.ts | 0 .../[serviceId]/rollouts/[rolloutId]/route.ts | 0 .../services/[serviceId]/rollouts/route.ts | 0 web/app/api/v1/services/[serviceId]/route.ts | 1 + .../services/[serviceId]/status/route.ts | 0 web/lib/public-api-routes.ts | 103 ++++++-- web/lib/public-api.ts | 96 ++++---- web/lib/service-revision-spec.ts | 4 +- web/tests/public-api-configuration.test.ts | 6 +- web/tests/public-api-revisions-auth.test.ts | 26 +- web/tests/public-api-source.test.ts | 74 ++++-- web/tests/service-revision-spec.test.ts | 13 + 25 files changed, 492 insertions(+), 345 deletions(-) delete mode 100644 web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/route.ts rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/builds/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/configuration/route.ts (66%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/deploy/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/logs/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/metrics/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/revisions/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/rollouts/[rolloutId]/logs/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/rollouts/[rolloutId]/route.ts (100%) rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/rollouts/route.ts (100%) create mode 100644 web/app/api/v1/services/[serviceId]/route.ts rename web/app/api/v1/{projects/[projectId]/environments/[environmentId] => }/services/[serviceId]/status/route.ts (100%) diff --git a/cli/internal/cli/app.go b/cli/internal/cli/app.go index 9d09e9f2..fb406a56 100644 --- a/cli/internal/cli/app.go +++ b/cli/internal/cli/app.go @@ -286,10 +286,6 @@ func (a *App) initCommand() *cobra.Command { folderName = "my-service" } starter := fmt.Sprintf(`apiVersion: v1 -project: - slug: %s -environment: - name: production service: name: %s source: @@ -304,7 +300,8 @@ service: ports: - containerPort: 80 public: false -`, folderName, folderName) + resources: null +`, folderName) if err := os.WriteFile(manifestPath, []byte(starter), 0o644); err != nil { return err } @@ -321,28 +318,23 @@ service: func (a *App) linkCommand() *cobra.Command { var force bool - var projectID, environmentID, serviceID string + var serviceID string cmd := &cobra.Command{ Use: "link", Short: "Create techulus.yml from an existing service", Annotations: map[string]string{ - "agent_notes": "Requires an interactive terminal and does not support --agent or --json. Agents should usually pass --project, --environment, and --service to status/logs instead of linking.", + "agent_notes": "Requires an interactive terminal or --service and does not support --agent or --json. Agents can pass --service to service commands instead of linking.", }, RunE: func(cmd *cobra.Command, args []string) error { if a.isMachineOutput() { return errors.New("tc link does not support --agent or --json") } explicitIDs := 0 - for _, id := range []string{projectID, environmentID, serviceID} { - if strings.TrimSpace(id) != "" { - explicitIDs++ - } - } - if explicitIDs != 0 && explicitIDs != 3 { - return errors.New("provide --project, --environment, and --service together") + if strings.TrimSpace(serviceID) != "" { + explicitIDs = 1 } if explicitIDs == 0 && !a.IsInteractive() { - return errors.New("tc link requires an interactive terminal or all ID flags") + return errors.New("tc link requires an interactive terminal or --service") } config, err := a.requireConfig() if err != nil { @@ -353,13 +345,31 @@ func (a *App) linkCommand() *cobra.Command { return err } manifestPath := filepath.Join(cwd, "techulus.yml") - if _, err := os.Stat(manifestPath); err == nil && !force { - return errors.New("techulus.yml already exists. Run `tc link --force` to replace it") - } else if err != nil && !errors.Is(err, os.ErrNotExist) { + var existing *manifest.Loaded + if _, err := os.Stat(manifestPath); err == nil { + existing, err = manifest.Load(cwd) + if err != nil { + return err + } + } else if !errors.Is(err, os.ErrNotExist) { return err } client := a.client(config) + if explicitIDs == 1 { + var direct struct { + Service serviceItem `json:"service"` + Target targetContext `json:"target"` + Management *serviceManagement `json:"management"` + } + if err := client.RequestJSON(cmd.Context(), http.MethodGet, "/api/v1/services/"+url.PathEscape(serviceID), nil, nil, &direct); err != nil { + return err + } + if err := managementCompatibilityError(direct.Management); err != nil { + return err + } + return a.finishLink(manifestPath, existing, force, direct.Service, direct.Target) + } ps, err := fetchAllProjects(cmd.Context(), client) if err != nil { return err @@ -369,16 +379,7 @@ func (a *App) linkCommand() *cobra.Command { } reader := bufio.NewReader(a.In) var project projectItem - if projectID != "" { - for _, v := range ps.Projects { - if v.ID == projectID { - project = v - } - } - if project.ID == "" { - return errors.New("project ID not found") - } - } else { + { project, err = selectFromList(reader, a.Out, "Select a project:", ps.Projects, func(v projectItem) string { return v.Name }) if err != nil { return err @@ -390,15 +391,10 @@ func (a *App) linkCommand() *cobra.Command { return err } var environment environmentItem - if environmentID != "" { - for _, v := range es.Environments { - if v.ID == environmentID { - environment = v - } - } - if environment.ID == "" { - return errors.New("environment ID not found") - } + if len(es.Environments) == 0 { + return errors.New("selected project has no environments") + } else if len(es.Environments) == 1 { + environment = es.Environments[0] } else { environment, err = selectFromList(reader, a.Out, "Select an environment:", es.Environments, func(v environmentItem) string { return v.Name }) if err != nil { @@ -411,16 +407,7 @@ func (a *App) linkCommand() *cobra.Command { return err } var service serviceItem - if serviceID != "" { - for _, v := range ss.Services { - if v.ID == serviceID { - service = v - } - } - if service.ID == "" { - return errors.New("service ID not found") - } - } else { + { service, err = selectFromList(reader, a.Out, "Select a service:", ss.Services, func(v serviceItem) string { return v.Name }) if err != nil { return err @@ -446,26 +433,14 @@ func (a *App) linkCommand() *cobra.Command { StartCommand *string `json:"startCommand"` Resources *manifest.Resources `json:"resources"` } `json:"current"` - Management *struct { - Patchable bool `json:"patchable"` - Blockers []struct { - Code string `json:"code"` - Message string `json:"message"` - } `json:"blockers"` - } `json:"management"` - } - base := sp + "/" + url.PathEscape(service.ID) + Management *serviceManagement `json:"management"` + } + base := "/api/v1/services/" + url.PathEscape(service.ID) if err := client.RequestJSON(cmd.Context(), http.MethodGet, base+"/configuration", nil, nil, &cfg); err != nil { return err } - if cfg.Management == nil { - return errors.New("configuration response did not include service management compatibility") - } - if !cfg.Management.Patchable { - if len(cfg.Management.Blockers) > 0 && cfg.Management.Blockers[0].Message != "" { - return errors.New(cfg.Management.Blockers[0].Message) - } - return errors.New("this service cannot be managed with techulus.yml") + if err := managementCompatibilityError(cfg.Management); err != nil { + return err } if cfg.Current.Resources != nil && cfg.Current.Resources.CPUCores == nil && cfg.Current.Resources.MemoryMB == nil { cfg.Current.Resources = nil @@ -488,7 +463,13 @@ func (a *App) linkCommand() *cobra.Command { } } } - m := manifest.Manifest{APIVersion: "v1", Project: manifest.Project{ID: project.ID, Slug: project.Slug}, Environment: manifest.Environment{ID: environment.ID, Name: environment.Name}, Service: manifest.Service{ID: service.ID, Name: service.Name, Source: service.Source, Hostname: cfg.Current.Hostname, Ports: ports, Replicas: cfg.Current.Replicas, Placement: placement, HealthCheck: cfg.Current.HealthCheck, StartCommand: cfg.Current.StartCommand, Resources: cfg.Current.Resources}} + m := manifest.Manifest{APIVersion: "v1", Target: &manifest.Target{ServiceID: service.ID}, Service: manifest.Service{Name: service.Name, Source: service.Source, Hostname: cfg.Current.Hostname, Ports: ports, Replicas: cfg.Current.Replicas, Placement: placement, HealthCheck: cfg.Current.HealthCheck, StartCommand: cfg.Current.StartCommand, Resources: cfg.Current.Resources}} + if existing != nil { + if existing.Manifest.Linked() && existing.Manifest.Target.ServiceID != service.ID && !force { + return errors.New("manifest is linked to another service; use --force to rebind") + } + m.Service = existing.Manifest.Service + } if err := manifest.Save(manifestPath, m); err != nil { return err } @@ -500,8 +481,6 @@ func (a *App) linkCommand() *cobra.Command { }, } cmd.Flags().BoolVar(&force, "force", false, "Replace an existing techulus.yml") - cmd.Flags().StringVar(&projectID, "project", "", "Project ID") - cmd.Flags().StringVar(&environmentID, "environment", "", "Environment ID") cmd.Flags().StringVar(&serviceID, "service", "", "Service ID") return cmd } @@ -531,16 +510,13 @@ func (a *App) applyCommand() *cobra.Command { if placement == nil { return errors.New("service.placement is required") } - body := map[string]any{"source": sourcePatch(loaded.Manifest.Service.Source), "hostname": loaded.Manifest.Service.Hostname, "ports": loaded.Manifest.Service.Ports, "healthCheck": loaded.Manifest.Service.HealthCheck, "startCommand": loaded.Manifest.Service.StartCommand} + body := map[string]any{"name": loaded.Manifest.Service.Name, "source": sourcePatch(loaded.Manifest.Service.Source), "hostname": loaded.Manifest.Service.Hostname, "ports": loaded.Manifest.Service.Ports, "healthCheck": loaded.Manifest.Service.HealthCheck, "startCommand": loaded.Manifest.Service.StartCommand, "resources": loaded.Manifest.Service.Resources} if placement.Mode == "automatic" { body["placement"] = map[string]any{"mode": "automatic", "replicas": loaded.Manifest.Service.Replicas} } else { body["placement"] = map[string]any{"mode": "manual", "placements": placement.Servers} } - if loaded.Manifest.Service.Resources != nil { - body["resources"] = loaded.Manifest.Service.Resources - } - if err := client.RequestJSON(cmd.Context(), http.MethodPatch, serviceBase(loaded.Manifest)+"/configuration", nil, body, &result); err != nil { + if err := client.RequestJSON(cmd.Context(), http.MethodPut, serviceBase(loaded.Manifest)+"/configuration", nil, body, &result); err != nil { return err } if a.isMachineOutput() { @@ -552,6 +528,54 @@ func (a *App) applyCommand() *cobra.Command { } } +func managementCompatibilityError(management *serviceManagement) error { + if management == nil { + return errors.New("service response did not include management compatibility") + } + if management.Patchable { + return nil + } + if len(management.Blockers) > 0 && management.Blockers[0].Message != "" { + return errors.New(management.Blockers[0].Message) + } + return errors.New("this service cannot be managed with techulus.yml") +} + +func (a *App) finishLink(path string, existing *manifest.Loaded, force bool, service serviceItem, target targetContext) error { + serviceID := service.ID + if target.Service.ID != "" { + serviceID = target.Service.ID + } + if existing != nil { + if existing.Manifest.Linked() { + if existing.Manifest.Target.ServiceID == serviceID { + return a.printLinked(path, target) + } + if !force { + return errors.New("manifest is linked to another service; use --force to rebind") + } + } + existing.Manifest.Target = &manifest.Target{ServiceID: serviceID} + if err := manifest.Save(path, existing.Manifest); err != nil { + return err + } + return a.printLinked(path, target) + } + m := manifest.Manifest{APIVersion: "v1", Target: &manifest.Target{ServiceID: serviceID}, Service: manifest.Service{Name: service.Name, Source: service.Source, Hostname: service.Hostname, Ports: service.Ports, Replicas: service.Replicas, Placement: service.Placement, HealthCheck: service.HealthCheck, StartCommand: service.StartCommand, Resources: service.Resources}} + if err := manifest.Save(path, m); err != nil { + return err + } + return a.printLinked(path, target) +} + +func (a *App) printLinked(path string, target targetContext) error { + output.Section(a.Out, "Linked") + output.Field(a.Out, "Service", fmt.Sprintf("%s/%s/%s", target.Project.Slug, target.Environment.Name, target.Service.Name)) + output.Field(a.Out, "Manifest", path) + output.Next(a.Out, "tc status or tc apply") + return nil +} + func (a *App) deployCommand() *cobra.Command { return &cobra.Command{ Use: "deploy", @@ -611,7 +635,7 @@ func (a *App) statusCommand() *cobra.Command { Use: "status", Short: "Show service rollout and deployment status", Annotations: map[string]string{ - "agent_notes": "Without explicit target flags, tc reads techulus.yml from the current directory.\nFor agent use outside a linked directory, pass --project, --environment, and --service together.", + "agent_notes": "Without --service, tc reads the target from techulus.yml in the current directory.\nFor agent use outside a linked directory, pass --service.", }, RunE: func(cmd *cobra.Command, args []string) error { config, err := a.requireConfig() @@ -647,7 +671,7 @@ func (a *App) logsCommand() *cobra.Command { Use: "logs", Short: "Show service logs", Annotations: map[string]string{ - "agent_notes": "Without explicit target flags, tc reads techulus.yml from the current directory.\nFor agent use outside a linked directory, pass --project, --environment, and --service together.\nIn --agent or --json mode, logs are one-shot JSON output; --follow=true is not supported.", + "agent_notes": "Without --service, tc reads the target from techulus.yml in the current directory.\nFor agent use outside a linked directory, pass --service.\nIn --agent or --json mode, logs are one-shot JSON output; --follow=true is not supported.", }, RunE: func(cmd *cobra.Command, args []string) error { if tail < 1 || tail > 1000 { @@ -715,9 +739,7 @@ func (a *App) environmentsCommand() *cobra.Command { return e } if id == "" { - if l, x := a.ensureManifest(); x == nil { - id = l.Manifest.Project.ID - } + return errors.New("missing --project") } if id == "" { return errors.New("missing --project (or link this directory)") @@ -746,17 +768,7 @@ func (a *App) servicesCommand() *cobra.Command { return e } if p == "" || eid == "" { - if l, x := a.ensureManifest(); x == nil { - if p == "" { - p = l.Manifest.Project.ID - } - if eid == "" { - eid = l.Manifest.Environment.ID - } - } - } - if p == "" || eid == "" { - return errors.New("missing --project and --environment (or link this directory)") + return errors.New("missing --project and --environment") } path := "/api/v1/projects/" + url.PathEscape(p) + "/environments/" + url.PathEscape(eid) + "/services" out, e := fetchAllServices(cmd.Context(), a.client(cfg), path) @@ -833,8 +845,6 @@ func (a *App) rolloutCommand() *cobra.Command { c := &cobra.Command{Use: "rollout ", Short: "Show rollout detail", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return a.getRolloutResource(cmd, target, args[0], false, "", 100) }} - c.PersistentFlags().StringVar(&target.Project, "project", "", "Project ID") - c.PersistentFlags().StringVar(&target.Environment, "environment", "", "Environment ID") c.PersistentFlags().StringVar(&target.Service, "service", "", "Service ID") var q string var limit int @@ -1083,28 +1093,16 @@ func parseAgentArgs(cmd *cobra.Command) []agentArg { } type serviceTargetFlags struct { - Project string - Environment string - Service string + Service string } func addServiceTargetFlags(cmd *cobra.Command, target *serviceTargetFlags) { - cmd.Flags().StringVar(&target.Project, "project", "", "Project ID") - cmd.Flags().StringVar(&target.Environment, "environment", "", "Environment ID") cmd.Flags().StringVar(&target.Service, "service", "", "Service ID") } func (a *App) resolveServiceTarget(target serviceTargetFlags) (manifest.Manifest, error) { - project := strings.TrimSpace(target.Project) - environment := strings.TrimSpace(target.Environment) service := strings.TrimSpace(target.Service) - explicitCount := 0 - for _, value := range []string{project, environment, service} { - if value != "" { - explicitCount++ - } - } - if explicitCount == 0 { + if service == "" { loaded, err := a.ensureManifest() if err != nil { return manifest.Manifest{}, err @@ -1114,21 +1112,16 @@ func (a *App) resolveServiceTarget(target serviceTargetFlags) (manifest.Manifest } return loaded.Manifest, nil } - if explicitCount != 3 { - return manifest.Manifest{}, errors.New("provide --project, --environment, and --service together") - } return manifest.Manifest{ - APIVersion: "v1", - Project: manifest.Project{ID: project, Slug: project}, - Environment: manifest.Environment{ID: environment, Name: environment}, - Service: manifest.Service{ - ID: service, Name: service, - }, + APIVersion: "v1", Target: &manifest.Target{ServiceID: service}, Service: manifest.Service{Name: service}, }, nil } func serviceBase(value manifest.Manifest) string { - return "/api/v1/projects/" + url.PathEscape(value.Project.ID) + "/environments/" + url.PathEscape(value.Environment.ID) + "/services/" + url.PathEscape(value.Service.ID) + if value.Target == nil { + return "/api/v1/services/" + } + return "/api/v1/services/" + url.PathEscape(value.Target.ServiceID) } func sourcePatch(source manifest.Source) map[string]any { @@ -1362,7 +1355,7 @@ func (a *App) runLogs(ctx context.Context, config *auth.Config, value manifest.M if a.isMachineOutput() { return a.writeData(result, "Logs") } - fmt.Fprintf(a.Out, "%s/%s/%s\n", value.Project.Slug, value.Environment.Name, value.Service.Name) + fmt.Fprintf(a.Out, "%s/%s/%s\n", result.Target.Project.Slug, result.Target.Environment.Name, result.Target.Service.Name) if result.Provider == "disabled" { output.Section(a.Out, "Logs") output.Field(a.Out, "Status", "disabled") @@ -1470,7 +1463,7 @@ func printApplyResult(w io.Writer, result applyResponse) { } func printStatus(w io.Writer, value manifest.Manifest, status statusResponse) { - fmt.Fprintf(w, "%s/%s/%s\n", value.Project.Slug, value.Environment.Name, value.Service.Name) + fmt.Fprintf(w, "%s/%s/%s\n", status.Target.Project.Slug, status.Target.Environment.Name, status.Target.Service.Name) output.Section(w, "Service") output.Field(w, "ID", status.Service.ID) if status.Service.Source.Type == "image" { diff --git a/cli/internal/cli/app_test.go b/cli/internal/cli/app_test.go index b2201f0e..3a91a406 100644 --- a/cli/internal/cli/app_test.go +++ b/cli/internal/cli/app_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "os" @@ -19,10 +20,8 @@ import ( ) const imageManifest = `apiVersion: v1 -project: {id: p, slug: app} -environment: {id: e, name: prod} +target: {serviceId: s} service: - id: s name: web source: {type: image, image: nginx:1.27} replicas: 2 @@ -134,6 +133,71 @@ func TestInitRecommendsLinkInHumanAndJSON(t *testing.T) { } } +func TestDirectLinkPreservesDesiredConfigurationAndRequiresForceToRebind(t *testing.T) { + d := t.TempDir() + var requested string + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requested = r.URL.Path + id := strings.TrimPrefix(r.URL.Path, "/api/v1/services/") + fmt.Fprintf(w, `{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":%q,"name":"remote"}},"service":{"id":%q,"name":"remote","source":{"type":"image","image":"remote:latest"},"ports":[],"replicas":1,"placement":{"mode":"automatic"},"hostname":null,"healthCheck":null,"startCommand":null,"resources":null},"management":{"patchable":true,"blockers":[]}}`, id, id) + })) + defer s.Close() + writeConfig(t, s.URL) + app, out := testApp(t, d, s.Client()) + if err := execute(app, "init"); err != nil { + t.Fatal(err) + } + before, err := manifest.Load(d) + if err != nil { + t.Fatal(err) + } + out.Reset() + if err := execute(app, "link", "--service", "s"); err != nil { + t.Fatal(err) + } + linked, _ := manifest.Load(d) + if requested != "/api/v1/services/s" || linked.Manifest.Target.ServiceID != "s" || !reflect.DeepEqual(linked.Manifest.Service, before.Manifest.Service) { + t.Fatalf("requested=%q manifest=%#v", requested, linked.Manifest) + } + assertHumanOutput(t, out.String(), "Linked", "app/prod/remote", filepath.Join(d, "techulus.yml")) + if err := execute(app, "link", "--service", "s"); err != nil { + t.Fatalf("same-target link: %v", err) + } + if err := execute(app, "link", "--service", "other"); err == nil || !strings.Contains(err.Error(), "--force") { + t.Fatalf("different-target error = %v", err) + } + if err := execute(app, "link", "--service", "other", "--force"); err != nil { + t.Fatal(err) + } + rebound, _ := manifest.Load(d) + if rebound.Manifest.Target.ServiceID != "other" || !reflect.DeepEqual(rebound.Manifest.Service, before.Manifest.Service) { + t.Fatalf("rebound manifest=%#v", rebound.Manifest) + } +} + +func TestDirectLinkRejectsUnmanagedServiceWithoutChangingManifest(t *testing.T) { + d := t.TempDir() + writeManifest(t, d, imageManifest) + before, err := os.ReadFile(filepath.Join(d, "techulus.yml")) + if err != nil { + t.Fatal(err) + } + s := responseServer(t, `{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"other","name":"remote"}},"service":{"id":"other","name":"remote","source":{"type":"image","image":"remote:latest"},"ports":[],"replicas":1,"placement":{"mode":"automatic"},"hostname":null,"healthCheck":null,"startCommand":null,"resources":null},"management":{"patchable":false,"blockers":[{"code":"UNSUPPORTED_PORTS","message":"TCP services must be managed in the web UI"}]}}`) + writeConfig(t, s.URL) + app, _ := testApp(t, d, s.Client()) + err = execute(app, "link", "--service", "other", "--force") + if err == nil || !strings.Contains(err.Error(), "TCP services") { + t.Fatalf("error = %v", err) + } + after, readErr := os.ReadFile(filepath.Join(d, "techulus.yml")) + if readErr != nil { + t.Fatal(readErr) + } + if !bytes.Equal(after, before) { + t.Fatal("failed direct link changed the existing manifest") + } +} + func TestLinkByIDsFetchesConfigurationAndSupportsPublicGitHub(t *testing.T) { var paths []string s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -145,7 +209,7 @@ func TestLinkByIDsFetchesConfigurationAndSupportsPublicGitHub(t *testing.T) { w.Write([]byte(`{"environments":[{"id":"e","name":"prod"}]}`)) case "/api/v1/projects/p/environments/e/services": w.Write([]byte(`{"services":[{"id":"s","name":"web","source":{"type":"github","repository":"https://github.com/acme/public","branch":"main","rootDir":"cmd/api"}}]}`)) - case "/api/v1/projects/p/environments/e/services/s/configuration": + case "/api/v1/services/s/configuration": w.Write([]byte(`{"current":{"replicas":2,"placement":{"mode":"manual"},"placements":[{"serverId":"server-a","count":1},{"serverId":"server-b","count":1}],"hostname":null,"ports":[],"healthCheck":null,"startCommand":null},"management":{"patchable":true,"blockers":[]}}`)) default: t.Errorf("path=%s", r.URL.Path) @@ -154,10 +218,15 @@ func TestLinkByIDsFetchesConfigurationAndSupportsPublicGitHub(t *testing.T) { defer s.Close() writeConfig(t, s.URL) d := t.TempDir() - app, _ := testApp(t, d, s.Client()) - if err := execute(app, "link", "--project", "p", "--environment", "e", "--service", "s"); err != nil { + app, out := testApp(t, d, s.Client()) + app.IsInteractive = func() bool { return true } + app.In = strings.NewReader("1\n1\n") + if err := execute(app, "link"); err != nil { t.Fatal(err) } + if strings.Contains(out.String(), "Select an environment:") { + t.Fatalf("single environment unexpectedly prompted:\n%s", out.String()) + } loaded, err := manifest.Load(d) if err != nil { t.Fatal(err) @@ -168,7 +237,7 @@ func TestLinkByIDsFetchesConfigurationAndSupportsPublicGitHub(t *testing.T) { if loaded.Manifest.Service.Placement == nil || loaded.Manifest.Service.Placement.Mode != "manual" || len(loaded.Manifest.Service.Placement.Servers) != 2 || loaded.Manifest.Service.Placement.Servers[1].ServerID != "server-b" { t.Fatalf("placement=%#v", loaded.Manifest.Service.Placement) } - want := []string{"/api/v1/projects", "/api/v1/projects/p/environments", "/api/v1/projects/p/environments/e/services", "/api/v1/projects/p/environments/e/services/s/configuration"} + want := []string{"/api/v1/projects", "/api/v1/projects/p/environments", "/api/v1/projects/p/environments/e/services", "/api/v1/services/s/configuration"} if !reflect.DeepEqual(paths, want) { t.Fatalf("paths=%v", paths) } @@ -183,7 +252,7 @@ func TestLinkRejectsManualServiceWithoutPlacements(t *testing.T) { w.Write([]byte(`{"environments":[{"id":"e","name":"prod"}]}`)) case "/api/v1/projects/p/environments/e/services": w.Write([]byte(`{"services":[{"id":"s","name":"web","source":{"type":"image","image":"nginx"}}]}`)) - case "/api/v1/projects/p/environments/e/services/s/configuration": + case "/api/v1/services/s/configuration": w.Write([]byte(`{"current":{"replicas":0,"placement":{"mode":"manual"},"placements":[],"hostname":null,"ports":[],"healthCheck":null,"startCommand":null},"management":{"patchable":true,"blockers":[]}}`)) default: t.Errorf("path=%s", r.URL.Path) @@ -193,7 +262,9 @@ func TestLinkRejectsManualServiceWithoutPlacements(t *testing.T) { writeConfig(t, s.URL) d := t.TempDir() app, _ := testApp(t, d, s.Client()) - err := execute(app, "link", "--project", "p", "--environment", "e", "--service", "s") + app.IsInteractive = func() bool { return true } + app.In = strings.NewReader("1\n1\n") + err := execute(app, "link") if err == nil || !strings.Contains(err.Error(), "configure at least one server placement") { t.Fatalf("error = %v", err) } @@ -221,12 +292,12 @@ func TestApplyExactNestedPatchForSources(t *testing.T) { if err := execute(app, "apply"); err != nil { t.Fatal(err) } - if method != "PATCH" || path != "/api/v1/projects/p/environments/e/services/s/configuration" { + if method != "PUT" || path != "/api/v1/services/s/configuration" { t.Fatalf("%s %s", method, path) } source := body["source"].(map[string]any) placement := body["placement"].(map[string]any) - if source["type"] != tc.sourceType || placement["mode"] != "automatic" || placement["replicas"] != float64(2) || len(body) != 6 { + if source["type"] != tc.sourceType || placement["mode"] != "automatic" || placement["replicas"] != float64(2) || len(body) != 8 { t.Fatalf("body=%#v", body) } if tc.name == "github_clear_root" { @@ -419,7 +490,7 @@ func TestMissingIDsFailLocally(t *testing.T) { for _, command := range []string{"apply", "deploy", "status", "logs"} { t.Run(command, func(t *testing.T) { d := t.TempDir() - writeManifest(t, d, strings.ReplaceAll(imageManifest, "id: p, ", "")) + writeManifest(t, d, strings.ReplaceAll(imageManifest, "target: {serviceId: s}\n", "")) app, _ := testApp(t, d, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { t.Fatal("unexpected network request") return nil, nil @@ -490,14 +561,14 @@ func TestStatusAndResourceRoutesAndOutput(t *testing.T) { var gotPath, gotQuery string s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath, gotQuery = r.URL.Path, r.URL.RawQuery - w.Write([]byte(`{"service":{"id":"0400075c-69aa-46c2-bccc-fc172b8c6b28","name":"web","source":{"type":"image","image":"nginx"}},"latestBuild":null,"latestRollout":null,"deployments":[],"items":[]}`)) + w.Write([]byte(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"service":{"id":"0400075c-69aa-46c2-bccc-fc172b8c6b28","name":"web","source":{"type":"image","image":"nginx"}},"latestBuild":null,"latestRollout":null,"deployments":[],"items":[]}`)) })) defer s.Close() writeConfig(t, s.URL) for _, mode := range []string{"--agent", "--json"} { app, out := testApp(t, t.TempDir(), s.Client()) args := append([]string{mode}, tc.args...) - args = append(args, "--project", "p", "--environment", "e", "--service", "s") + args = append(args, "--service", "s") if err := execute(app, args...); err != nil { t.Fatal(err) } @@ -508,13 +579,13 @@ func TestStatusAndResourceRoutesAndOutput(t *testing.T) { } if tc.args[0] == "status" { app, out := testApp(t, t.TempDir(), s.Client()) - args := append(tc.args, "--project", "p", "--environment", "e", "--service", "s") + args := append(tc.args, "--service", "s") if err := execute(app, args...); err != nil { t.Fatal(err) } - assertHumanOutput(t, out.String(), "ID", "0400075c-69aa-46c2-bccc-fc172b8c6b28") + assertHumanOutput(t, out.String(), "app/prod/web", "ID", "0400075c-69aa-46c2-bccc-fc172b8c6b28") } - base := "/api/v1/projects/p/environments/e/services/s" + base := "/api/v1/services/s" if gotPath != base+tc.path || gotQuery != tc.query { t.Fatalf("got %s?%s", gotPath, gotQuery) } @@ -544,7 +615,7 @@ func TestBuildsHumanOutputIsFormatted(t *testing.T) { writeConfig(t, s.URL) app, out := testApp(t, t.TempDir(), s.Client()) - err := execute(app, "builds", "--project", "p", "--environment", "e", "--service", "s") + err := execute(app, "builds", "--service", "s") if err != nil { t.Fatal(err) } @@ -558,7 +629,7 @@ func TestConfigurationHumanOutputIsFormatted(t *testing.T) { writeConfig(t, s.URL) app, out := testApp(t, t.TempDir(), s.Client()) - err := execute(app, "config", "--project", "p", "--environment", "e", "--service", "s") + err := execute(app, "config", "--service", "s") if err != nil { t.Fatal(err) } @@ -570,7 +641,7 @@ func TestMetricsHumanOutputIsFormatted(t *testing.T) { writeConfig(t, s.URL) app, out := testApp(t, t.TempDir(), s.Client()) - err := execute(app, "metrics", "--project", "p", "--environment", "e", "--service", "s") + err := execute(app, "metrics", "--service", "s") if err != nil { t.Fatal(err) } @@ -585,7 +656,7 @@ func TestMetricsHumanOutputShowsDisabledProvider(t *testing.T) { writeConfig(t, s.URL) app, out := testApp(t, t.TempDir(), s.Client()) - err := execute(app, "metrics", "--project", "p", "--environment", "e", "--service", "s") + err := execute(app, "metrics", "--service", "s") if err != nil { t.Fatal(err) } @@ -597,7 +668,7 @@ func TestRevisionsHumanOutputIsFormatted(t *testing.T) { writeConfig(t, s.URL) app, out := testApp(t, t.TempDir(), s.Client()) - err := execute(app, "revisions", "--project", "p", "--environment", "e", "--service", "s") + err := execute(app, "revisions", "--service", "s") if err != nil { t.Fatal(err) } @@ -607,11 +678,11 @@ func TestRevisionsHumanOutputIsFormatted(t *testing.T) { func TestRolloutHumanOutputIsFormatted(t *testing.T) { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/api/v1/projects/p/environments/e/services/s/rollouts": + case "/api/v1/services/s/rollouts": w.Write([]byte(`{"rollouts":[{"id":"2c917d90-4bc1-4274-b3bf-34fed009fc12","status":"in_progress","currentStage":"health_check","createdAt":"2026-07-21T11:04:59.46Z","completedAt":null,"deployments":[{"serverName":"ubuntu-2","phase":"running","healthStatus":"healthy"}]}],"nextCursor":"next-page"}`)) - case "/api/v1/projects/p/environments/e/services/s/rollouts/r1": + case "/api/v1/services/s/rollouts/r1": w.Write([]byte(`{"rollout":{"id":"2c917d90-4bc1-4274-b3bf-34fed009fc12","status":"completed","currentStage":"completed","createdAt":"2026-07-21T11:04:59.46Z","completedAt":"2026-07-21T11:05:30Z","deployments":[]}}`)) - case "/api/v1/projects/p/environments/e/services/s/rollouts/r1/logs": + case "/api/v1/services/s/rollouts/r1/logs": w.Write([]byte(`{"provider":"enabled","logs":[{"message":"Rollout started","stage":"preparing","timestamp":"2026-07-21T11:04:59.46Z"},{"message":"Starting container","stage":"health_check","timestamp":"2026-07-21T11:05:00.529Z"}]}`)) default: t.Fatalf("unexpected path %s", r.URL.Path) @@ -629,7 +700,7 @@ func TestRolloutHumanOutputIsFormatted(t *testing.T) { {[]string{"rollout", "logs", "r1"}, []string{"Rollout logs (2)", "[preparing]", "Rollout started", "[health check]", "Starting container"}}, } { app, out := testApp(t, t.TempDir(), s.Client()) - args := append(tc.args, "--project", "p", "--environment", "e", "--service", "s") + args := append(tc.args, "--service", "s") if err := execute(app, args...); err != nil { t.Fatal(err) } @@ -674,7 +745,7 @@ func TestLogsDrainAvailablePagesWithoutSleeping(t *testing.T) { requests++ switch requests { case 1: - w.Write([]byte(`{"provider":"enabled","logs":[],"nextCursor":"page-1"}`)) + w.Write([]byte(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"provider":"enabled","logs":[],"nextCursor":"page-1"}`)) case 2: if got := r.URL.Query().Get("cursor"); got != "page-1" { t.Errorf("cursor=%q", got) @@ -702,7 +773,7 @@ func TestLogsDrainAvailablePagesWithoutSleeping(t *testing.T) { if requests != 3 || sleeps != 0 { t.Fatalf("requests=%d sleeps=%d", requests, sleeps) } - if !strings.Contains(out.String(), "same-time-a") || !strings.Contains(out.String(), "same-time-b") { + if !strings.Contains(out.String(), "app/prod/web") || !strings.Contains(out.String(), "same-time-a") || !strings.Contains(out.String(), "same-time-b") { t.Fatalf("missing equal-timestamp logs in output:\n%s", out.String()) } } diff --git a/cli/internal/cli/types.go b/cli/internal/cli/types.go index 59359330..4a2ffac7 100644 --- a/cli/internal/cli/types.go +++ b/cli/internal/cli/types.go @@ -41,10 +41,41 @@ type environmentItem struct { Name string `json:"name"` } type serviceItem struct { - ID string `json:"id"` - Name string `json:"name"` - Hostname *string `json:"hostname"` - Source manifest.Source `json:"source"` + ID string `json:"id"` + Name string `json:"name"` + Hostname *string `json:"hostname"` + Source manifest.Source `json:"source"` + Ports []manifest.Port `json:"ports"` + Replicas int `json:"replicas"` + Placement *manifest.Placement `json:"placement"` + HealthCheck *manifest.HealthCheck `json:"healthCheck"` + StartCommand *string `json:"startCommand"` + Resources *manifest.Resources `json:"resources"` +} +type targetProject struct { + ID string `json:"id"` + Slug string `json:"slug"` +} +type targetEnvironment struct { + ID string `json:"id"` + Name string `json:"name"` +} +type targetService struct { + ID string `json:"id"` + Name string `json:"name"` +} +type targetContext struct { + Project targetProject `json:"project"` + Environment targetEnvironment `json:"environment"` + Service targetService `json:"service"` +} +type managementBlocker struct { + Code string `json:"code"` + Message string `json:"message"` +} +type serviceManagement struct { + Patchable bool `json:"patchable"` + Blockers []managementBlocker `json:"blockers"` } type projectsResponse struct { Projects []projectItem `json:"projects"` @@ -69,6 +100,7 @@ type deployResponse struct { BuildID *string `json:"buildId"` } type statusResponse struct { + Target targetContext `json:"target"` Service struct { ID string `json:"id"` Name string `json:"name"` @@ -86,9 +118,10 @@ type serviceLog struct { } type logsResponse struct { - Provider string `json:"provider"` - Logs []serviceLog `json:"logs"` - NextCursor string `json:"nextCursor"` - HasMore bool `json:"hasMore"` - PollAfterMS int `json:"pollAfterMs"` + Target targetContext `json:"target"` + Provider string `json:"provider"` + Logs []serviceLog `json:"logs"` + NextCursor string `json:"nextCursor"` + HasMore bool `json:"hasMore"` + PollAfterMS int `json:"pollAfterMs"` } diff --git a/cli/internal/manifest/manifest.go b/cli/internal/manifest/manifest.go index d929bb54..2bbff0bd 100644 --- a/cli/internal/manifest/manifest.go +++ b/cli/internal/manifest/manifest.go @@ -16,21 +16,14 @@ import ( var windowsAbsolutePath = regexp.MustCompile(`^[A-Za-z]:[\\/]`) type Manifest struct { - APIVersion string `json:"apiVersion" yaml:"apiVersion"` - Project Project `json:"project" yaml:"project"` - Environment Environment `json:"environment" yaml:"environment"` - Service Service `json:"service" yaml:"service"` + APIVersion string `json:"apiVersion" yaml:"apiVersion"` + Target *Target `json:"target,omitempty" yaml:"target,omitempty"` + Service Service `json:"service" yaml:"service"` } -type Project struct { - ID string `json:"id,omitempty" yaml:"id,omitempty"` - Slug string `json:"slug" yaml:"slug"` -} -type Environment struct { - ID string `json:"id,omitempty" yaml:"id,omitempty"` - Name string `json:"name" yaml:"name"` +type Target struct { + ServiceID string `json:"serviceId,omitempty" yaml:"serviceId,omitempty"` } type Service struct { - ID string `json:"id,omitempty" yaml:"id,omitempty"` Name string `json:"name" yaml:"name"` Source Source `json:"source" yaml:"source"` Hostname *string `json:"hostname" yaml:"hostname"` @@ -115,11 +108,9 @@ func Save(path string, m Manifest) error { } func ApplyDefaults(m *Manifest) { m.APIVersion = strings.TrimSpace(m.APIVersion) - m.Project.ID = strings.TrimSpace(m.Project.ID) - m.Project.Slug = strings.TrimSpace(m.Project.Slug) - m.Environment.ID = strings.TrimSpace(m.Environment.ID) - m.Environment.Name = strings.TrimSpace(m.Environment.Name) - m.Service.ID = strings.TrimSpace(m.Service.ID) + if m.Target != nil { + m.Target.ServiceID = strings.TrimSpace(m.Target.ServiceID) + } m.Service.Name = strings.TrimSpace(m.Service.Name) s := &m.Service.Source s.Type = strings.ToLower(strings.TrimSpace(s.Type)) @@ -175,12 +166,6 @@ func Validate(m Manifest) error { if m.APIVersion != "v1" { return errors.New("apiVersion must be v1") } - if m.Project.Slug == "" { - return errors.New("project.slug is required") - } - if m.Environment.Name == "" { - return errors.New("environment.name is required") - } if m.Service.Name == "" { return errors.New("service.name is required") } @@ -315,7 +300,7 @@ func Validate(m Manifest) error { return nil } func (m Manifest) Linked() bool { - return m.Project.ID != "" && m.Environment.ID != "" && m.Service.ID != "" + return m.Target != nil && strings.TrimSpace(m.Target.ServiceID) != "" } func CanonicalGitHubRepository(value string) (string, error) { u, err := url.Parse(strings.TrimSpace(value)) diff --git a/cli/internal/manifest/manifest_test.go b/cli/internal/manifest/manifest_test.go index b18b5709..0261d28a 100644 --- a/cli/internal/manifest/manifest_test.go +++ b/cli/internal/manifest/manifest_test.go @@ -6,7 +6,7 @@ import ( ) func base() Manifest { - return Manifest{APIVersion: "v1", Project: Project{ID: "p", Slug: "app"}, Environment: Environment{ID: "e", Name: "prod"}, Service: Service{ID: "s", Name: "web", Source: Source{Type: "image", Image: "nginx"}, Replicas: 1, Placement: &Placement{Mode: "automatic"}}} + return Manifest{APIVersion: "v1", Target: &Target{ServiceID: "s"}, Service: Service{Name: "web", Source: Source{Type: "image", Image: "nginx"}, Replicas: 1, Placement: &Placement{Mode: "automatic"}}} } func TestDefaultsAndRoundTrip(t *testing.T) { m := base() @@ -65,8 +65,6 @@ func TestPlacementRoundTripAndValidation(t *testing.T) { func TestPlacementIsRequired(t *testing.T) { _, err := Parse([]byte(`apiVersion: v1 -project: {slug: app} -environment: {name: prod} service: name: web source: {type: image, image: nginx} diff --git a/docs/api/public-api.mdx b/docs/api/public-api.mdx index 2f2dd5cc..46d9b2a8 100644 --- a/docs/api/public-api.mdx +++ b/docs/api/public-api.mdx @@ -46,20 +46,20 @@ Content-Type: application/json } ``` -## Authorization and containment +## Authorization and service identity Roles are global for the current Techulus Cloud installation. There are no project-level permissions. - `reader`, `developer`, and `admin` can read resources. - `developer` and `admin` can change configuration and deploy services. -Service URLs always include the project and environment: +Service operations use the globally unique service ID: ```text -/api/v1/projects/{projectId}/environments/{environmentId}/services/{serviceId} +/api/v1/services/{serviceId} ``` -The API returns `404 NOT_FOUND` when an ID exists but does not belong to the parent IDs in the URL. +Nested project and environment paths remain available for browsing collections. Service responses include an authoritative `target` with the containing project, environment, and service labels. ## Errors @@ -82,7 +82,7 @@ Use `code` for automation. Do not match the human-readable `message`. | `GET` | `/api/v1/projects` | List projects | | `GET` | `/api/v1/projects/{projectId}/environments` | List environments in a project | | `GET` | `/api/v1/projects/{projectId}/environments/{environmentId}/services` | List services in an environment | -| `GET` | `/api/v1/projects/{projectId}/environments/{environmentId}/services/{serviceId}` | Return one contained service | +| `GET` | `/api/v1/services/{serviceId}` | Return the target and canonical service configuration | Collection responses use an opaque keyset cursor: @@ -105,11 +105,11 @@ Pass `nextCursor` as `?cursor=...`. `limit` defaults to 100 and accepts values f ## Service resources -The paths in this table are relative to the nested service URL. +The paths in this table are relative to `/api/v1/services/{serviceId}`. | Method | Path suffix | Description | | --- | --- | --- | -| `GET`, `PATCH` | `/configuration` | Read safe current/active configuration or atomically apply the managed subset | +| `GET`, `PUT` | `/configuration` | Read safe current/active configuration or atomically replace all managed configuration | | `GET` | `/status` | Read source, latest build and rollout, and persisted deployments | | `POST` | `/deploy` | Queue an image rollout or GitHub build | | `GET` | `/logs` | Search logs and optionally long poll | @@ -136,12 +136,13 @@ A GitHub service keeps mutable repository settings in `current.source`. Its `act Configuration and revision responses never include secret names, values, or ciphertext. -### Patch configuration +### Replace configuration -`PATCH /configuration` is atomic. Every field is optional. Omitted fields remain unchanged. An included `ports` array replaces the entire managed port set. +`PUT /configuration` is atomic and replaces the complete managed configuration. The request must contain exactly `name`, `source`, `hostname`, `ports`, `placement`, `healthCheck`, `startCommand`, and `resources`. Omitted or unknown fields are rejected. Use `null` to clear nullable fields, including `resources`. ```json { + "name": "web", "source": { "type": "github", "repository": "https://github.com/techulus/cloud", @@ -175,6 +176,8 @@ Configuration and revision responses never include secret names, values, or ciph } ``` +The `tc apply` command sends this complete replacement. A linked `techulus.yml` stores only `target.serviceId`; desired service configuration remains under `service`. Explicit CLI targeting uses `--service `. During interactive `tc link`, a project with exactly one environment selects it automatically; zero environments is an error and multiple environments prompt for a choice. + The API supports these source variants: ```json @@ -190,7 +193,7 @@ The API supports these source variants: } ``` -GitHub repository URLs are canonical HTTPS `github.com` URLs. `rootDir` must stay inside the repository. Use `rootDir: null` to clear an existing build root. Source conversion and GitHub repository switching are not supported. +GitHub repository URLs are canonical HTTPS `github.com` URLs. `rootDir` is required and must either stay inside the repository or be `null` to clear an existing build root. Source conversion and GitHub repository switching are not supported. Use automatic placement for stateless services: @@ -312,4 +315,4 @@ The CLI uses the same endpoints documented above: | `tc metrics` | Query service metrics | | `tc revisions` | List the redacted revision changelog | -`tc link` stores the selected project, environment, and service IDs in `techulus.yml`. Image and GitHub services use the same `tc link`, `tc apply`, `tc deploy`, and inspection commands. +`tc link` stores only the selected `target.serviceId` in `techulus.yml`. Image and GitHub services use the same `tc link`, `tc apply`, `tc deploy`, and inspection commands. diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/route.ts b/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/route.ts deleted file mode 100644 index f696aa3c..00000000 --- a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/route.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { requireApiKeyRole } from "@/lib/api-auth"; -import { - apiError, - findNestedService, - notFound, - resolvePersistedSource, -} from "@/lib/public-api"; -export async function GET( - request: Request, - { - params, - }: { - params: Promise<{ - projectId: string; - environmentId: string; - serviceId: string; - }>; - }, -) { - const auth = await requireApiKeyRole(request, [ - "admin", - "developer", - "reader", - ]); - if (!auth.ok) return auth.response; - try { - const p = await params; - const service = await findNestedService( - p.projectId, - p.environmentId, - p.serviceId, - ); - if (!service) return notFound(); - return Response.json({ - service: { - id: service.id, - name: service.name, - hostname: service.hostname, - source: await resolvePersistedSource(service), - createdAt: service.createdAt, - }, - }); - } catch (error) { - console.error("[public-api] read service failed", error); - return apiError("Internal server error", "INTERNAL_ERROR", 500); - } -} diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/builds/route.ts b/web/app/api/v1/services/[serviceId]/builds/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/builds/route.ts rename to web/app/api/v1/services/[serviceId]/builds/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/configuration/route.ts b/web/app/api/v1/services/[serviceId]/configuration/route.ts similarity index 66% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/configuration/route.ts rename to web/app/api/v1/services/[serviceId]/configuration/route.ts index b4e71e9b..4f5c8771 100644 --- a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/configuration/route.ts +++ b/web/app/api/v1/services/[serviceId]/configuration/route.ts @@ -1,4 +1,4 @@ export { getConfiguration as GET, - patchConfigurationRoute as PATCH, + putConfigurationRoute as PUT, } from "@/lib/public-api-routes"; diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/deploy/route.ts b/web/app/api/v1/services/[serviceId]/deploy/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/deploy/route.ts rename to web/app/api/v1/services/[serviceId]/deploy/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/logs/route.ts b/web/app/api/v1/services/[serviceId]/logs/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/logs/route.ts rename to web/app/api/v1/services/[serviceId]/logs/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/metrics/route.ts b/web/app/api/v1/services/[serviceId]/metrics/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/metrics/route.ts rename to web/app/api/v1/services/[serviceId]/metrics/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/revisions/route.ts b/web/app/api/v1/services/[serviceId]/revisions/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/revisions/route.ts rename to web/app/api/v1/services/[serviceId]/revisions/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/rollouts/[rolloutId]/logs/route.ts b/web/app/api/v1/services/[serviceId]/rollouts/[rolloutId]/logs/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/rollouts/[rolloutId]/logs/route.ts rename to web/app/api/v1/services/[serviceId]/rollouts/[rolloutId]/logs/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/rollouts/[rolloutId]/route.ts b/web/app/api/v1/services/[serviceId]/rollouts/[rolloutId]/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/rollouts/[rolloutId]/route.ts rename to web/app/api/v1/services/[serviceId]/rollouts/[rolloutId]/route.ts diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/rollouts/route.ts b/web/app/api/v1/services/[serviceId]/rollouts/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/rollouts/route.ts rename to web/app/api/v1/services/[serviceId]/rollouts/route.ts diff --git a/web/app/api/v1/services/[serviceId]/route.ts b/web/app/api/v1/services/[serviceId]/route.ts new file mode 100644 index 00000000..b21aff2d --- /dev/null +++ b/web/app/api/v1/services/[serviceId]/route.ts @@ -0,0 +1 @@ +export { getServiceDetails as GET } from "@/lib/public-api-routes"; diff --git a/web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/status/route.ts b/web/app/api/v1/services/[serviceId]/status/route.ts similarity index 100% rename from web/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/status/route.ts rename to web/app/api/v1/services/[serviceId]/status/route.ts diff --git a/web/lib/public-api-routes.ts b/web/lib/public-api-routes.ts index 6af35edf..8721a186 100644 --- a/web/lib/public-api-routes.ts +++ b/web/lib/public-api-routes.ts @@ -14,12 +14,12 @@ import { METRIC_RANGE_KEYS } from "@/lib/metric-ranges"; import { apiError, badRequest, - configurationPatchSchema, - findNestedService, + findServiceContext, isPublicApiDomainError, notFound, - patchConfiguration, publicApiDomainResponse, + replaceConfiguration, + replaceConfigurationSchema, resolvePersistedSource, safeConfiguration, } from "@/lib/public-api"; @@ -42,8 +42,6 @@ import { import { isMetricsEnabled, queryServiceMetrics } from "@/lib/victoria-metrics"; export type PublicServiceParams = { - projectId: string; - environmentId: string; serviceId: string; }; export type PublicServiceContext = { params: Promise }; @@ -54,12 +52,10 @@ async function readScope(request: Request, context: PublicServiceContext) { if (!auth.ok) return { response: auth.response }; try { const params = await context.params; - const service = await findNestedService( - params.projectId, - params.environmentId, - params.serviceId, - ); - return service ? { service, params } : { response: notFound() }; + const found = await findServiceContext(params.serviceId); + return found + ? { service: found.service, target: found, params } + : { response: notFound() }; } catch (error) { return { response: internalError(error, "resolve service scope") }; } @@ -70,12 +66,10 @@ async function writeScope(request: Request, context: PublicServiceContext) { if (!auth.ok) return { response: auth.response }; try { const params = await context.params; - const service = await findNestedService( - params.projectId, - params.environmentId, - params.serviceId, - ); - return service ? { service, params, auth } : { response: notFound() }; + const found = await findServiceContext(params.serviceId); + return found + ? { service: found.service, target: found, params, auth } + : { response: notFound() }; } catch (error) { return { response: internalError(error, "resolve service scope") }; } @@ -100,6 +94,47 @@ function internalError(error: unknown, operation: string) { return apiError("Internal server error", "INTERNAL_ERROR", 500); } +export async function getServiceDetails( + request: Request, + context: PublicServiceContext, +) { + const scope = await readScope(request, context); + if ("response" in scope) return scope.response; + try { + const configuration = await safeConfiguration(scope.service); + const current = configuration.current; + return Response.json({ + target: { + project: { id: scope.target.projectId, slug: scope.target.projectSlug }, + environment: { + id: scope.target.environmentId, + name: scope.target.environmentName, + }, + service: { id: scope.service.id, name: scope.service.name }, + }, + service: { + id: scope.service.id, + name: scope.service.name, + source: current.source, + hostname: current.hostname, + ports: current.ports, + replicas: current.replicas, + placement: + current.placement.mode === "manual" + ? { mode: "manual", servers: current.placements } + : current.placement, + healthCheck: current.healthCheck, + startCommand: current.startCommand, + resources: + current.resources.cpuCores == null ? null : current.resources, + }, + management: configuration.management, + }); + } catch (error) { + return internalError(error, "read service"); + } +} + export async function getConfiguration( request: Request, context: PublicServiceContext, @@ -113,13 +148,13 @@ export async function getConfiguration( } } -export async function patchConfigurationRoute( +export async function putConfigurationRoute( request: Request, context: PublicServiceContext, ) { const scope = await writeScope(request, context); if ("response" in scope) return scope.response; - const parsed = configurationPatchSchema.safeParse( + const parsed = replaceConfigurationSchema.safeParse( await request.json().catch(() => null), ); if (!parsed.success) { @@ -128,11 +163,13 @@ export async function patchConfigurationRoute( ); } try { - return Response.json(await patchConfiguration(scope.service, parsed.data)); + return Response.json( + await replaceConfiguration(scope.service, parsed.data), + ); } catch (error) { return isPublicApiDomainError(error) ? publicApiDomainResponse(error) - : internalError(error, "patch configuration"); + : internalError(error, "replace configuration"); } } @@ -211,6 +248,14 @@ export async function getStatus( resolvePersistedSource(scope.service), ]); return Response.json({ + target: { + project: { id: scope.target.projectId, slug: scope.target.projectSlug }, + environment: { + id: scope.target.environmentId, + name: scope.target.environmentName, + }, + service: { id: scope.service.id, name: scope.service.name }, + }, service: { id: scope.service.id, name: scope.service.name, @@ -435,6 +480,14 @@ export async function getServiceLogs( if ("response" in scope) return scope.response; if (!isLoggingEnabled()) { return Response.json({ + target: { + project: { id: scope.target.projectId, slug: scope.target.projectSlug }, + environment: { + id: scope.target.environmentId, + name: scope.target.environmentName, + }, + service: { id: scope.service.id, name: scope.service.name }, + }, provider: "disabled", logs: [], nextCursor: null, @@ -467,6 +520,14 @@ export async function getServiceLogs( }) : await query(); return Response.json({ + target: { + project: { id: scope.target.projectId, slug: scope.target.projectSlug }, + environment: { + id: scope.target.environmentId, + name: scope.target.environmentName, + }, + service: { id: scope.service.id, name: scope.service.name }, + }, provider: "enabled", logs: result.logs.map(publicServiceLog), nextCursor: nextServiceLogCursor(result.logs, options.rawCursor), diff --git a/web/lib/public-api.ts b/web/lib/public-api.ts index 0d722a85..99ca6141 100644 --- a/web/lib/public-api.ts +++ b/web/lib/public-api.ts @@ -15,6 +15,7 @@ import { serviceVolumes, } from "@/db/schema"; import { validateDockerImageInternal } from "@/lib/docker-image"; +import { nameSchema } from "@/lib/schemas"; import { getServiceTotalReplicas } from "@/lib/service-config"; import { parseServiceRevisionSpec } from "@/lib/service-revision-changes"; import { @@ -110,7 +111,7 @@ export const publicSourceSchema = z.discriminatedUnion("type", [ type: z.literal("github"), repository: githubRepositorySchema, branch: z.string().trim().min(1).max(255), - rootDir: rootDirSchema.nullable().optional(), + rootDir: rootDirSchema.nullable(), }), ]); @@ -120,7 +121,7 @@ export type PublicSource = type: "github"; repository: string | null; branch: string; - rootDir?: string; + rootDir: string | null; }; export type NestedService = typeof services.$inferSelect; type GitHubRepo = typeof githubRepos.$inferSelect; @@ -161,9 +162,7 @@ export function resolvePersistedSourceFromRows( repo?.defaultBranch?.trim() || service.githubBranch?.trim() || "main", - ...(service.githubRootDir?.trim() - ? { rootDir: service.githubRootDir.trim() } - : {}), + rootDir: service.githubRootDir?.trim() || null, }; } @@ -182,33 +181,27 @@ export async function resolvePersistedSource( return resolvePersistedSourceFromRows(service, repo); } -export async function findNestedService( - projectId: string, - environmentId: string, - serviceId: string, -) { - const row = await db - .select({ service: services }) - .from(projects) +export async function findServiceContext(serviceId: string) { + return db + .select({ + service: services, + projectId: projects.id, + projectSlug: projects.slug, + environmentId: environments.id, + environmentName: environments.name, + }) + .from(services) + .innerJoin(projects, eq(projects.id, services.projectId)) .innerJoin( environments, and( - eq(environments.id, environmentId), + eq(environments.id, services.environmentId), eq(environments.projectId, projects.id), ), ) - .innerJoin( - services, - and( - eq(services.id, serviceId), - eq(services.projectId, projects.id), - eq(services.environmentId, environments.id), - isNull(services.deletedAt), - ), - ) - .where(eq(projects.id, projectId)) - .limit(1); - return row[0]?.service ?? null; + .where(and(eq(services.id, serviceId), isNull(services.deletedAt))) + .limit(1) + .then((rows) => rows[0] ?? null); } export function apiError(message: string, code: string, status: number) { @@ -296,7 +289,7 @@ function sanitizeSpec(specification: unknown) { type: "github" as const, repository: spec.source.repository, branch: spec.source.branch, - ...(spec.source.rootDir ? { rootDir: spec.source.rootDir } : {}), + rootDir: spec.source.rootDir, } : { type: "image" as const, image: spec.source.image }, hostname: spec.hostname, @@ -459,8 +452,7 @@ export async function safeConfiguration(service: NestedService) { const comparableCurrent = { source: current.source, - hostname: - current.hostname?.trim() || getDefaultServiceHostname(service.name), + hostname: current.hostname?.trim() || getDefaultServiceHostname(service.id), stateful: current.stateful, placement: current.placement.mode === "automatic" @@ -592,13 +584,14 @@ export const placementSchema = z.discriminatedUnion("mode", [ }); }), ]); -export const configurationPatchSchema = z.strictObject({ - source: publicSourceSchema.optional(), - hostname: hostnameSchema.nullable().optional(), - ports: z.array(portSchema).max(100).optional(), - placement: placementSchema.optional(), - healthCheck: healthCheckSchema.nullable().optional(), - startCommand: z.string().trim().min(1).max(4096).nullable().optional(), +export const replaceConfigurationSchema = z.strictObject({ + name: nameSchema, + source: publicSourceSchema, + hostname: hostnameSchema.nullable(), + ports: z.array(portSchema).max(100), + placement: placementSchema, + healthCheck: healthCheckSchema.nullable(), + startCommand: z.string().trim().min(1).max(4096).nullable(), resources: z .strictObject({ cpuCores: z.number().min(0.1).max(64).nullable(), @@ -608,7 +601,7 @@ export const configurationPatchSchema = z.strictObject({ (value) => (value.cpuCores === null) === (value.memoryMb === null), "CPU and memory limits must both be set or both be null", ) - .optional(), + .nullable(), }); type PublicApiDomainError = Error & { code: string; status: number }; @@ -644,9 +637,9 @@ function healthCheckFromService(service: NestedService) { : null; } -export async function patchConfiguration( +export async function replaceConfiguration( service: NestedService, - input: z.infer, + input: z.infer, ) { if (input.source?.type === "image" && input.source.image !== service.image) { const validation = await validateDockerImageInternal(input.source.image); @@ -835,6 +828,7 @@ export async function patchConfiguration( changes.push(label); return true; }; + if (changed("name", persisted.name, input.name)) set.name = input.name; if ( input.hostname !== undefined && @@ -876,18 +870,20 @@ export async function patchConfiguration( ); } if ( - input.resources !== undefined && changed( "resources", - { - cpuCores: persisted.resourceCpuLimit, - memoryMb: persisted.resourceMemoryLimitMb, - }, + persisted.resourceCpuLimit == null && + persisted.resourceMemoryLimitMb == null + ? null + : { + cpuCores: persisted.resourceCpuLimit, + memoryMb: persisted.resourceMemoryLimitMb, + }, input.resources, ) ) { - set.resourceCpuLimit = input.resources.cpuCores; - set.resourceMemoryLimitMb = input.resources.memoryMb; + set.resourceCpuLimit = input.resources?.cpuCores ?? null; + set.resourceMemoryLimitMb = input.resources?.memoryMb ?? null; } if ( input.source?.type === "image" && @@ -954,11 +950,9 @@ export async function patchConfiguration( .where(eq(githubRepos.id, repo.id)); } } - if (input.source.rootDir !== undefined) { - const desiredRoot = input.source.rootDir; - if (changed("source.rootDir", persisted.githubRootDir, desiredRoot)) { - set.githubRootDir = desiredRoot; - } + const desiredRoot = input.source.rootDir; + if (changed("source.rootDir", persisted.githubRootDir, desiredRoot)) { + set.githubRootDir = desiredRoot; } } diff --git a/web/lib/service-revision-spec.ts b/web/lib/service-revision-spec.ts index c14f461e..af8de6cd 100644 --- a/web/lib/service-revision-spec.ts +++ b/web/lib/service-revision-spec.ts @@ -164,6 +164,7 @@ export type ServiceRevisionSpec = { export type ServiceRevisionDraft = { service: { + id: string; name: string; image: string; hostname: string | null; @@ -278,8 +279,7 @@ export function buildServiceRevisionSpec( schemaVersion: SERVICE_REVISION_SCHEMA_VERSION, image, source: overrides.source ?? { type: "image", image }, - hostname: - service.hostname?.trim() || getDefaultServiceHostname(service.name), + hostname: service.hostname?.trim() || getDefaultServiceHostname(service.id), stateful: service.stateful ?? false, serverless: { enabled: service.serverlessEnabled ?? false, diff --git a/web/tests/public-api-configuration.test.ts b/web/tests/public-api-configuration.test.ts index f17dce62..0850281c 100644 --- a/web/tests/public-api-configuration.test.ts +++ b/web/tests/public-api-configuration.test.ts @@ -29,7 +29,7 @@ describe("public API configuration state", () => { mocks.select.mockClear(); }); - it("does not report a derived default hostname as a pending change", async () => { + it("derives a stable default hostname from immutable service identity", async () => { mocks.rows.push( [], [], @@ -42,7 +42,7 @@ describe("public API configuration state", () => { schemaVersion: 2, image: "nginx:1.27", source: { type: "image", image: "nginx:1.27" }, - hostname: "hello-service", + hostname: "service-1", stateful: false, serverless: { enabled: false, @@ -85,7 +85,7 @@ describe("public API configuration state", () => { } as never); expect(configuration.current.hostname).toBeNull(); - expect(configuration.active?.hostname).toBe("hello-service"); + expect(configuration.active?.hostname).toBe("service-1"); expect(configuration.hasPendingChanges).toBe(false); expect(configuration.changes).toEqual([]); }); diff --git a/web/tests/public-api-revisions-auth.test.ts b/web/tests/public-api-revisions-auth.test.ts index b6a61597..8c139938 100644 --- a/web/tests/public-api-revisions-auth.test.ts +++ b/web/tests/public-api-revisions-auth.test.ts @@ -32,7 +32,7 @@ const mocks = vi.hoisted(() => ({ { status: 401 }, ), })), - findNestedService: vi.fn(async () => ({ id: "service-1" })), + findService: vi.fn(async () => ({ id: "service-1" })), queryServiceRevisionChangelog: vi.fn(async () => ({ revisions: [ { @@ -60,25 +60,33 @@ vi.mock("@/lib/api-auth", () => ({ vi.mock("@/lib/public-api", async (importOriginal) => ({ ...(await importOriginal()), - findNestedService: mocks.findNestedService, + findServiceContext: async (_serviceId: string) => { + const service = await mocks.findService(); + return service + ? { + service, + projectId: "project-1", + projectSlug: "project", + environmentId: "environment-1", + environmentName: "production", + } + : null; + }, })); vi.mock("@/lib/service-revision-changelog", () => ({ queryServiceRevisionChangelog: mocks.queryServiceRevisionChangelog, })); -import { GET } from "@/app/api/v1/projects/[projectId]/environments/[environmentId]/services/[serviceId]/revisions/route"; +import { GET } from "@/app/api/v1/services/[serviceId]/revisions/route"; it("lists public revisions with an API key without requiring a browser session", async () => { const response = await GET( - new Request( - "https://cloud.test/api/v1/projects/project-1/environments/environment-1/services/service-1/revisions", - { headers: { "x-api-key": "tcl_secret" } }, - ), + new Request("https://cloud.test/api/v1/services/service-1/revisions", { + headers: { "x-api-key": "tcl_secret" }, + }), { params: Promise.resolve({ - projectId: "project-1", - environmentId: "environment-1", serviceId: "service-1", }), }, diff --git a/web/tests/public-api-source.test.ts b/web/tests/public-api-source.test.ts index 030625a6..94e7363d 100644 --- a/web/tests/public-api-source.test.ts +++ b/web/tests/public-api-source.test.ts @@ -1,11 +1,23 @@ import { describe, expect, it } from "vitest"; import { canonicalGitHubRepository, - configurationPatchSchema, isSafeRepositoryRoot, publicSourceSchema, + replaceConfigurationSchema, } from "@/lib/public-api"; +const completeConfiguration = (overrides: Record = {}) => ({ + name: "web", + source: { type: "image", image: "nginx:1.27" }, + hostname: null, + ports: [], + placement: { mode: "automatic", replicas: 1 }, + healthCheck: null, + startCommand: null, + resources: null, + ...overrides, +}); + describe("public API GitHub sources", () => { it.each([ [ @@ -72,13 +84,16 @@ describe("public API GitHub sources", () => { it("requires a nonblank GitHub branch", () => { expect( - configurationPatchSchema.safeParse({ - source: { - type: "github", - repository: "https://github.com/owner/repository", - branch: " ", - }, - }).success, + replaceConfigurationSchema.safeParse( + completeConfiguration({ + source: { + type: "github", + repository: "https://github.com/owner/repository", + branch: " ", + rootDir: null, + }, + }), + ).success, ).toBe(false); }); @@ -92,31 +107,48 @@ describe("public API GitHub sources", () => { type: "github", repository: "https://github.com/owner/repository", branch: "main", + rootDir: null, image: "registry.example/app:latest", }, ])("rejects mixed source fields", (source) => { - expect(configurationPatchSchema.safeParse({ source }).success).toBe(false); + expect( + replaceConfigurationSchema.safeParse(completeConfiguration({ source })) + .success, + ).toBe(false); }); - it("distinguishes omitted rootDir from explicit null", () => { - const omitted = publicSourceSchema.parse({ + it("requires explicit null to clear rootDir", () => { + const omitted = publicSourceSchema.safeParse({ type: "github", repository: "https://github.com/owner/repository", branch: "main", }); const cleared = publicSourceSchema.parse({ - ...omitted, + type: "github", + repository: "https://github.com/owner/repository", + branch: "main", rootDir: null, }); - expect(omitted).not.toHaveProperty("rootDir"); + expect(omitted.success).toBe(false); expect(cleared).toHaveProperty("rootDir", null); }); }); describe("public API placement schema", () => { + it("requires every managed field and rejects unknown fields", () => { + expect(replaceConfigurationSchema.safeParse({ name: "web" }).success).toBe( + false, + ); + expect( + replaceConfigurationSchema.safeParse( + completeConfiguration({ unmanaged: true }), + ).success, + ).toBe(false); + }); + it("rejects the removed standalone replicas field", () => { - expect(configurationPatchSchema.safeParse({ replicas: 3 }).success).toBe( + expect(replaceConfigurationSchema.safeParse({ replicas: 3 }).success).toBe( false, ); }); @@ -125,9 +157,10 @@ describe("public API placement schema", () => { { mode: "automatic", replicas: 3 }, { mode: "manual", placements: [{ serverId: "server-1", count: 2 }] }, ])("accepts valid placement intent", (placement) => { - expect(configurationPatchSchema.safeParse({ placement }).success).toBe( - true, - ); + expect( + replaceConfigurationSchema.safeParse(completeConfiguration({ placement })) + .success, + ).toBe(true); }); it.each([ @@ -148,8 +181,9 @@ describe("public API placement schema", () => { ], }, ])("rejects invalid placement intent", (placement) => { - expect(configurationPatchSchema.safeParse({ placement }).success).toBe( - false, - ); + expect( + replaceConfigurationSchema.safeParse(completeConfiguration({ placement })) + .success, + ).toBe(false); }); }); diff --git a/web/tests/service-revision-spec.test.ts b/web/tests/service-revision-spec.test.ts index a64d9a63..ba313fad 100644 --- a/web/tests/service-revision-spec.test.ts +++ b/web/tests/service-revision-spec.test.ts @@ -9,6 +9,7 @@ function draft( ): ServiceRevisionDraft { return { service: { + id: "service-1", name: "API Service", image: "nginx:latest", hostname: "api.internal", @@ -68,6 +69,18 @@ function draft( } describe("service revision specification", () => { + it("keeps the default hostname stable when the service is renamed", () => { + const original = draft(); + original.service.hostname = null; + const renamed = draft(); + renamed.service.hostname = null; + renamed.service.name = "Renamed API Service"; + + expect(buildServiceRevisionSpec(renamed).hostname).toBe( + buildServiceRevisionSpec(original).hostname, + ); + }); + it("normalizes draft row ordering", () => { const first = buildServiceRevisionSpec(draft()); const reorderedDraft = draft(); From 4f1aaa1896751e7e348a4178ca2151d0307da69e Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 29 Jul 2026 10:58:42 +0000 Subject: [PATCH 2/3] feat: plan and confirm manifest apply Amp-Thread-ID: https://ampcode.com/threads/T-019fac48-72fe-7706-9ce1-45eb90eadbef Co-authored-by: Arjun Komath --- cli/internal/api/client.go | 7 + cli/internal/cli/app.go | 131 +++++-- cli/internal/cli/app_test.go | 323 +++++++++++++++++- cli/internal/cli/types.go | 12 +- cli/internal/manifest/manifest.go | 22 +- cli/internal/manifest/manifest_test.go | 22 +- cli/internal/output/output.go | 12 +- docs/api/public-api.mdx | 7 +- web/actions/projects.ts | 219 +++++++----- .../[serviceId]/configuration/plan/route.ts | 1 + .../inngest/functions/migration-workflow.ts | 43 +-- .../functions/service-deletion-workflow.ts | 128 ++++--- web/lib/public-api-routes.ts | 50 ++- web/lib/public-api.ts | 314 +++++++++++++---- web/lib/service-revision-spec.ts | 25 +- web/tests/public-api-configuration.test.ts | 6 +- web/tests/public-api-plan.test.ts | 211 ++++++++++++ web/tests/public-api-source.test.ts | 2 +- web/tests/service-revision-spec.test.ts | 19 +- 19 files changed, 1267 insertions(+), 287 deletions(-) create mode 100644 web/app/api/v1/services/[serviceId]/configuration/plan/route.ts create mode 100644 web/tests/public-api-plan.test.ts diff --git a/cli/internal/api/client.go b/cli/internal/api/client.go index 8cdaa68d..49fa336f 100644 --- a/cli/internal/api/client.go +++ b/cli/internal/api/client.go @@ -64,6 +64,10 @@ func NewClient(host, apiKey string) *Client { } func (c *Client) RequestJSON(ctx context.Context, method, path string, query url.Values, body any, out any) error { + return c.RequestJSONWithHeaders(ctx, method, path, query, nil, body, out) +} + +func (c *Client) RequestJSONWithHeaders(ctx context.Context, method, path string, query url.Values, customHeaders map[string]string, body any, out any) error { endpoint := c.Host + path if len(query) > 0 { endpoint += "?" + query.Encode() @@ -72,6 +76,9 @@ func (c *Client) RequestJSON(ctx context.Context, method, path string, query url if c.APIKey != "" { headers["x-api-key"] = c.APIKey } + for key, value := range customHeaders { + headers[key] = value + } return JSON(ctx, c.HTTPClient, method, endpoint, headers, body, out) } diff --git a/cli/internal/cli/app.go b/cli/internal/cli/app.go index fb406a56..452e5ad3 100644 --- a/cli/internal/cli/app.go +++ b/cli/internal/cli/app.go @@ -55,6 +55,14 @@ type handledError struct { err error } +type applyPlanError struct { + message string + plan applyResponse +} + +func (e applyPlanError) Error() string { return e.message } +func (e applyPlanError) PlanData() any { return e.plan } + func (e handledError) Error() string { return e.err.Error() } @@ -291,7 +299,7 @@ service: source: type: image image: nginx:1.27 - hostname: null + hostname: %s replicas: 1 placement: mode: automatic @@ -301,7 +309,7 @@ service: - containerPort: 80 public: false resources: null -`, folderName) +`, folderName, folderName) if err := os.WriteFile(manifestPath, []byte(starter), 0o644); err != nil { return err } @@ -317,7 +325,6 @@ service: } func (a *App) linkCommand() *cobra.Command { - var force bool var serviceID string cmd := &cobra.Command{ Use: "link", @@ -329,11 +336,8 @@ func (a *App) linkCommand() *cobra.Command { if a.isMachineOutput() { return errors.New("tc link does not support --agent or --json") } - explicitIDs := 0 - if strings.TrimSpace(serviceID) != "" { - explicitIDs = 1 - } - if explicitIDs == 0 && !a.IsInteractive() { + explicitID := strings.TrimSpace(serviceID) != "" + if !explicitID && !a.IsInteractive() { return errors.New("tc link requires an interactive terminal or --service") } config, err := a.requireConfig() @@ -356,7 +360,7 @@ func (a *App) linkCommand() *cobra.Command { } client := a.client(config) - if explicitIDs == 1 { + if explicitID { var direct struct { Service serviceItem `json:"service"` Target targetContext `json:"target"` @@ -368,7 +372,7 @@ func (a *App) linkCommand() *cobra.Command { if err := managementCompatibilityError(direct.Management); err != nil { return err } - return a.finishLink(manifestPath, existing, force, direct.Service, direct.Target) + return a.finishLink(manifestPath, existing, direct.Service, direct.Target) } ps, err := fetchAllProjects(cmd.Context(), client) if err != nil { @@ -465,10 +469,13 @@ func (a *App) linkCommand() *cobra.Command { } m := manifest.Manifest{APIVersion: "v1", Target: &manifest.Target{ServiceID: service.ID}, Service: manifest.Service{Name: service.Name, Source: service.Source, Hostname: cfg.Current.Hostname, Ports: ports, Replicas: cfg.Current.Replicas, Placement: placement, HealthCheck: cfg.Current.HealthCheck, StartCommand: cfg.Current.StartCommand, Resources: cfg.Current.Resources}} if existing != nil { - if existing.Manifest.Linked() && existing.Manifest.Target.ServiceID != service.ID && !force { - return errors.New("manifest is linked to another service; use --force to rebind") + if existing.Manifest.Linked() && existing.Manifest.Target.ServiceID != service.ID { + return fmt.Errorf("manifest is linked to service %s; remove target.serviceId before relinking", existing.Manifest.Target.ServiceID) } m.Service = existing.Manifest.Service + if m.Service.Hostname == nil { + m.Service.Hostname = cfg.Current.Hostname + } } if err := manifest.Save(manifestPath, m); err != nil { return err @@ -480,13 +487,13 @@ func (a *App) linkCommand() *cobra.Command { return nil }, } - cmd.Flags().BoolVar(&force, "force", false, "Replace an existing techulus.yml") cmd.Flags().StringVar(&serviceID, "service", "", "Service ID") return cmd } func (a *App) applyCommand() *cobra.Command { - return &cobra.Command{ + var yes bool + cmd := &cobra.Command{ Use: "apply", Short: "Apply techulus.yml to the linked service", Annotations: map[string]string{ @@ -501,7 +508,6 @@ func (a *App) applyCommand() *cobra.Command { if err != nil { return err } - var result applyResponse client := a.client(config) if !loaded.Manifest.Linked() { return errors.New("service is not linked: run `tc link`") @@ -516,16 +522,79 @@ func (a *App) applyCommand() *cobra.Command { } else { body["placement"] = map[string]any{"mode": "manual", "placements": placement.Servers} } - if err := client.RequestJSON(cmd.Context(), http.MethodPut, serviceBase(loaded.Manifest)+"/configuration", nil, body, &result); err != nil { + base := serviceBase(loaded.Manifest) + "/configuration" + var plan applyResponse + if err := client.RequestJSON(cmd.Context(), http.MethodPost, base+"/plan", nil, body, &plan); err != nil { return err } - if a.isMachineOutput() { - return a.writeData(result, "Apply") + confirmationReader := bufio.NewReader(a.In) + const maxStaleReplans = 3 + staleReplans := 0 + for { + if !a.isMachineOutput() { + printApplyResult(a.Out, plan) + } + if len(plan.Changes) == 0 { + if a.isMachineOutput() { + return a.writeData(plan, "Plan") + } + fmt.Fprintln(a.Out, "No changes. The service already matches techulus.yml.") + return nil + } + if staleReplans >= maxStaleReplans { + return applyPlanError{ + message: "service configuration keeps changing; review the latest plan and try again", + plan: plan, + } + } + if yes && staleReplans > 0 { + return applyPlanError{ + message: "configuration changed; review the new plan and run tc apply --yes again", + plan: plan, + } + } + if !yes { + if !a.IsInteractive() { + return applyPlanError{message: "confirmation required", plan: plan} + } + confirmed, confirmErr := a.confirmApply(confirmationReader) + if confirmErr != nil || !confirmed { + return confirmErr + } + } + + var result applyResponse + err = client.RequestJSONWithHeaders(cmd.Context(), http.MethodPut, base, nil, map[string]string{"If-Match": fmt.Sprintf("%q", plan.CurrentVersion)}, body, &result) + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Code == "CONFIGURATION_PLAN_STALE" { + if planErr := client.RequestJSON(cmd.Context(), http.MethodPost, base+"/plan", nil, body, &plan); planErr != nil { + return planErr + } + staleReplans++ + continue + } + if err != nil { + return err + } + if a.isMachineOutput() { + return a.writeData(plan, "Applied") + } + return nil } - printApplyResult(a.Out, result) - return nil }, } + cmd.Flags().BoolVar(&yes, "yes", false, "Apply the displayed plan without prompting") + return cmd +} + +func (a *App) confirmApply(reader *bufio.Reader) (bool, error) { + fmt.Fprint(a.Out, "Apply these changes? [y/N] ") + line, err := reader.ReadString('\n') + if err != nil && !errors.Is(err, io.EOF) { + return false, err + } + answer := strings.ToLower(strings.TrimSpace(line)) + return answer == "y" || answer == "yes", nil } func managementCompatibilityError(management *serviceManagement) error { @@ -541,7 +610,7 @@ func managementCompatibilityError(management *serviceManagement) error { return errors.New("this service cannot be managed with techulus.yml") } -func (a *App) finishLink(path string, existing *manifest.Loaded, force bool, service serviceItem, target targetContext) error { +func (a *App) finishLink(path string, existing *manifest.Loaded, service serviceItem, target targetContext) error { serviceID := service.ID if target.Service.ID != "" { serviceID = target.Service.ID @@ -551,11 +620,12 @@ func (a *App) finishLink(path string, existing *manifest.Loaded, force bool, ser if existing.Manifest.Target.ServiceID == serviceID { return a.printLinked(path, target) } - if !force { - return errors.New("manifest is linked to another service; use --force to rebind") - } + return fmt.Errorf("manifest is linked to service %s; remove target.serviceId before relinking", existing.Manifest.Target.ServiceID) } existing.Manifest.Target = &manifest.Target{ServiceID: serviceID} + if existing.Manifest.Service.Hostname == nil { + existing.Manifest.Service.Hostname = service.Hostname + } if err := manifest.Save(path, existing.Manifest); err != nil { return err } @@ -1113,13 +1183,13 @@ func (a *App) resolveServiceTarget(target serviceTargetFlags) (manifest.Manifest return loaded.Manifest, nil } return manifest.Manifest{ - APIVersion: "v1", Target: &manifest.Target{ServiceID: service}, Service: manifest.Service{Name: service}, + APIVersion: "v1", Target: &manifest.Target{ServiceID: service}, }, nil } func serviceBase(value manifest.Manifest) string { - if value.Target == nil { - return "/api/v1/services/" + if value.Target == nil || strings.TrimSpace(value.Target.ServiceID) == "" { + panic("serviceBase called without a valid service target") } return "/api/v1/services/" + url.PathEscape(value.Target.ServiceID) } @@ -1450,7 +1520,10 @@ func fetchLogs(ctx context.Context, client *api.Client, value manifest.Manifest, } func printApplyResult(w io.Writer, result applyResponse) { - output.Section(w, "Apply") + output.Section(w, "Plan") + if result.Target.Service.ID != "" { + output.Field(w, "Target", fmt.Sprintf("%s/%s/%s", result.Target.Project.Slug, result.Target.Environment.Name, result.Target.Service.Name)) + } output.Field(w, "Action", result.Action) if len(result.Changes) == 0 { output.Field(w, "Changes", "none") @@ -1458,7 +1531,7 @@ func printApplyResult(w io.Writer, result applyResponse) { } output.Section(w, fmt.Sprintf("Changes (%d)", len(result.Changes))) for _, change := range result.Changes { - fmt.Fprintf(w, " * %s\n", change) + fmt.Fprintf(w, " * %s: %v -> %v\n", change.Field, change.From, change.To) } } diff --git a/cli/internal/cli/app_test.go b/cli/internal/cli/app_test.go index 3a91a406..90b585a4 100644 --- a/cli/internal/cli/app_test.go +++ b/cli/internal/cli/app_test.go @@ -17,6 +17,7 @@ import ( "techulus/cloud-cli/internal/api" "techulus/cloud-cli/internal/auth" "techulus/cloud-cli/internal/manifest" + "techulus/cloud-cli/internal/output" ) const imageManifest = `apiVersion: v1 @@ -26,7 +27,7 @@ service: source: {type: image, image: nginx:1.27} replicas: 2 placement: {mode: automatic} - hostname: null + hostname: web healthCheck: null startCommand: null ports: [] @@ -133,7 +134,26 @@ func TestInitRecommendsLinkInHumanAndJSON(t *testing.T) { } } -func TestDirectLinkPreservesDesiredConfigurationAndRequiresForceToRebind(t *testing.T) { +func TestInitCreatesConcreteValidHostname(t *testing.T) { + parent := t.TempDir() + d := filepath.Join(parent, "My API "+strings.Repeat("long-", 15)) + if err := os.Mkdir(d, 0o755); err != nil { + t.Fatal(err) + } + app, _ := testApp(t, d, nil) + if err := execute(app, "init"); err != nil { + t.Fatal(err) + } + loaded, err := manifest.Load(d) + if err != nil { + t.Fatal(err) + } + if loaded.Manifest.Service.Hostname == nil || len(*loaded.Manifest.Service.Hostname) > 63 || !strings.HasPrefix(*loaded.Manifest.Service.Hostname, "my-api-long") { + t.Fatalf("hostname = %#v", loaded.Manifest.Service.Hostname) + } +} + +func TestDirectLinkPreservesDesiredConfigurationAndRejectsRebind(t *testing.T) { d := t.TempDir() var requested string s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -163,15 +183,33 @@ func TestDirectLinkPreservesDesiredConfigurationAndRequiresForceToRebind(t *test if err := execute(app, "link", "--service", "s"); err != nil { t.Fatalf("same-target link: %v", err) } - if err := execute(app, "link", "--service", "other"); err == nil || !strings.Contains(err.Error(), "--force") { + beforeMismatch, _ := os.ReadFile(filepath.Join(d, "techulus.yml")) + if err := execute(app, "link", "--service", "other"); err == nil || !strings.Contains(err.Error(), "remove target.serviceId") || !strings.Contains(err.Error(), "s") { t.Fatalf("different-target error = %v", err) } - if err := execute(app, "link", "--service", "other", "--force"); err != nil { + afterMismatch, _ := os.ReadFile(filepath.Join(d, "techulus.yml")) + if !bytes.Equal(beforeMismatch, afterMismatch) { + t.Fatal("target mismatch changed manifest bytes") + } + if err := execute(app, "link", "--service", "other", "--force"); err == nil || !strings.Contains(err.Error(), "unknown flag") { + t.Fatalf("removed --force flag error = %v", err) + } +} + +func TestDirectLinkMaterializesEffectiveHostname(t *testing.T) { + d := t.TempDir() + s := responseServer(t, `{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"Remote Service"}},"service":{"id":"s","name":"Remote Service","source":{"type":"image","image":"nginx"},"ports":[],"replicas":1,"placement":{"mode":"automatic"},"hostname":"remote-service","healthCheck":null,"startCommand":null,"resources":null},"management":{"patchable":true,"blockers":[]}}`) + writeConfig(t, s.URL) + app, _ := testApp(t, d, s.Client()) + if err := execute(app, "link", "--service", "s"); err != nil { + t.Fatal(err) + } + loaded, err := manifest.Load(d) + if err != nil { t.Fatal(err) } - rebound, _ := manifest.Load(d) - if rebound.Manifest.Target.ServiceID != "other" || !reflect.DeepEqual(rebound.Manifest.Service, before.Manifest.Service) { - t.Fatalf("rebound manifest=%#v", rebound.Manifest) + if loaded.Manifest.Service.Hostname == nil || *loaded.Manifest.Service.Hostname != "remote-service" { + t.Fatalf("hostname = %#v", loaded.Manifest.Service.Hostname) } } @@ -185,7 +223,7 @@ func TestDirectLinkRejectsUnmanagedServiceWithoutChangingManifest(t *testing.T) s := responseServer(t, `{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"other","name":"remote"}},"service":{"id":"other","name":"remote","source":{"type":"image","image":"remote:latest"},"ports":[],"replicas":1,"placement":{"mode":"automatic"},"hostname":null,"healthCheck":null,"startCommand":null,"resources":null},"management":{"patchable":false,"blockers":[{"code":"UNSUPPORTED_PORTS","message":"TCP services must be managed in the web UI"}]}}`) writeConfig(t, s.URL) app, _ := testApp(t, d, s.Client()) - err = execute(app, "link", "--service", "other", "--force") + err = execute(app, "link", "--service", "other") if err == nil || !strings.Contains(err.Error(), "TCP services") { t.Fatalf("error = %v", err) } @@ -210,7 +248,7 @@ func TestLinkByIDsFetchesConfigurationAndSupportsPublicGitHub(t *testing.T) { case "/api/v1/projects/p/environments/e/services": w.Write([]byte(`{"services":[{"id":"s","name":"web","source":{"type":"github","repository":"https://github.com/acme/public","branch":"main","rootDir":"cmd/api"}}]}`)) case "/api/v1/services/s/configuration": - w.Write([]byte(`{"current":{"replicas":2,"placement":{"mode":"manual"},"placements":[{"serverId":"server-a","count":1},{"serverId":"server-b","count":1}],"hostname":null,"ports":[],"healthCheck":null,"startCommand":null},"management":{"patchable":true,"blockers":[]}}`)) + w.Write([]byte(`{"current":{"replicas":2,"placement":{"mode":"manual"},"placements":[{"serverId":"server-a","count":1},{"serverId":"server-b","count":1}],"hostname":"web","ports":[],"healthCheck":null,"startCommand":null},"management":{"patchable":true,"blockers":[]}}`)) default: t.Errorf("path=%s", r.URL.Path) } @@ -284,12 +322,12 @@ func TestApplyExactNestedPatchForSources(t *testing.T) { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { method, path = r.Method, r.URL.Path json.NewDecoder(r.Body).Decode(&body) - w.Write([]byte(`{"action":"updated","changes":["source"]}`)) + w.Write([]byte(`{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","desiredVersion":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","changes":[{"field":"source","from":"old","to":"new"}]}`)) })) defer s.Close() writeConfig(t, s.URL) app, _ := testApp(t, d, s.Client()) - if err := execute(app, "apply"); err != nil { + if err := execute(app, "apply", "--yes"); err != nil { t.Fatal(err) } if method != "PUT" || path != "/api/v1/services/s/configuration" { @@ -328,12 +366,12 @@ func TestApplyPlacementPayloads(t *testing.T) { var body map[string]any s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { json.NewDecoder(r.Body).Decode(&body) - w.Write([]byte(`{"action":"updated"}`)) + w.Write([]byte(`{"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","changes":[{"field":"placement","from":"old","to":"new"}]}`)) })) defer s.Close() writeConfig(t, s.URL) app, _ := testApp(t, d, s.Client()) - if err := execute(app, "apply"); err != nil { + if err := execute(app, "apply", "--yes"); err != nil { t.Fatal(err) } if _, exists := body["replicas"]; exists { @@ -346,6 +384,265 @@ func TestApplyPlacementPayloads(t *testing.T) { } } +func TestApplyPlansAndRequiresConfirmation(t *testing.T) { + const firstVersion = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + planWithChanges := fmt.Sprintf(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":%q,"desiredVersion":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","changes":[{"field":"name","from":"old-web","to":"web"},{"field":"source.branch","from":"develop","to":"main"}]}`, firstVersion) + noChanges := fmt.Sprintf(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"action":"noop","currentVersion":%q,"desiredVersion":%q,"changes":[]}`, firstVersion, firstVersion) + + t.Run("no changes skips prompt and write", func(t *testing.T) { + requests := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + if r.Method != http.MethodPost || r.URL.Path != "/api/v1/services/s/configuration/plan" { + t.Errorf("request = %s %s", r.Method, r.URL.Path) + } + w.Write([]byte(noChanges)) + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + if err := execute(app, "apply"); err != nil { + t.Fatal(err) + } + if requests != 1 || !strings.Contains(out.String(), "No changes. The service already matches techulus.yml.") || strings.Contains(out.String(), "Apply these changes?") { + t.Fatalf("requests=%d output=%q", requests, out.String()) + } + }) + + t.Run("noninteractive displays every change and fails", func(t *testing.T) { + requests := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests++ + w.Write([]byte(planWithChanges)) + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + err := execute(app, "apply") + if err == nil || !strings.Contains(err.Error(), "confirmation required") || requests != 1 { + t.Fatalf("error=%v requests=%d", err, requests) + } + assertHumanOutput(t, out.String(), "app/prod/web", "name: old-web -> web", "source.branch: develop -> main") + }) + + t.Run("decline does not write", func(t *testing.T) { + requests := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests++ + w.Write([]byte(planWithChanges)) + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + app.IsInteractive = func() bool { return true } + app.In = strings.NewReader("no\n") + if err := execute(app, "apply"); err != nil { + t.Fatal(err) + } + if requests != 1 || !strings.Contains(out.String(), "Apply these changes? [y/N]") { + t.Fatalf("requests=%d output=%q", requests, out.String()) + } + }) + + t.Run("yes applies the planned version", func(t *testing.T) { + var requests []string + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.Method+" "+r.URL.Path) + switch len(requests) { + case 1: + w.Write([]byte(planWithChanges)) + case 2: + if got := r.Header.Get("If-Match"); got != `"`+firstVersion+`"` { + t.Errorf("If-Match = %q", got) + } + w.Write([]byte(planWithChanges)) + default: + t.Errorf("unexpected request %d", len(requests)) + } + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, _ := testApp(t, d, s.Client()) + app.IsInteractive = func() bool { return true } + app.In = strings.NewReader("yes\n") + if err := execute(app, "apply"); err != nil { + t.Fatal(err) + } + want := []string{"POST /api/v1/services/s/configuration/plan", "PUT /api/v1/services/s/configuration"} + if !reflect.DeepEqual(requests, want) { + t.Fatalf("requests=%v", requests) + } + }) +} + +func TestApplyStalePlanBehavior(t *testing.T) { + const firstVersion = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + const secondVersion = "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + plan := func(version, from string) string { + return fmt.Sprintf(`{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":%q,"changes":[{"field":"source.branch","from":%q,"to":"main"}]}`, version, from) + } + + for _, tc := range []struct { + name string + args []string + input string + wantCalls int + wantSecond bool + }{ + {name: "interactive replans and reconfirms", args: []string{"apply"}, input: "y\ny\n", wantCalls: 4, wantSecond: true}, + {name: "yes replans but does not silently write", args: []string{"apply", "--yes"}, wantCalls: 3}, + } { + t.Run(tc.name, func(t *testing.T) { + calls := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + switch calls { + case 1: + w.Write([]byte(plan(firstVersion, "develop"))) + case 2: + w.WriteHeader(http.StatusConflict) + w.Write([]byte(`{"code":"CONFIGURATION_PLAN_STALE","message":"Service configuration changed after the plan was created"}`)) + case 3: + w.Write([]byte(plan(secondVersion, "release"))) + case 4: + if !tc.wantSecond { + t.Fatal("unexpected second PUT") + } + if got := r.Header.Get("If-Match"); got != `"`+secondVersion+`"` { + t.Errorf("replacement If-Match = %q", got) + } + w.Write([]byte(plan(secondVersion, "release"))) + default: + t.Fatalf("unexpected request %d", calls) + } + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + if tc.input != "" { + app.IsInteractive = func() bool { return true } + app.In = strings.NewReader(tc.input) + } + err := execute(app, tc.args...) + if tc.wantSecond && err != nil { + t.Fatal(err) + } + if !tc.wantSecond && (err == nil || !strings.Contains(err.Error(), "review the new plan")) { + t.Fatalf("error = %v", err) + } + if calls != tc.wantCalls || !strings.Contains(out.String(), "develop -> main") || !strings.Contains(out.String(), "release -> main") { + t.Fatalf("calls=%d output=%q", calls, out.String()) + } + }) + } +} + +func TestApplyStopsAfterRepeatedStalePlans(t *testing.T) { + const version = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + plan := func(index int) string { + return fmt.Sprintf(`{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":%q,"changes":[{"field":"name","from":%q,"to":"web"}]}`, version, fmt.Sprintf("remote-%d", index)) + } + calls := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + if r.Method == http.MethodPut { + w.WriteHeader(http.StatusConflict) + w.Write([]byte(`{"code":"CONFIGURATION_PLAN_STALE","message":"changed"}`)) + return + } + w.Write([]byte(plan(calls))) + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + app.IsInteractive = func() bool { return true } + app.In = strings.NewReader("y\ny\ny\n") + err := execute(app, "apply") + if err == nil || !strings.Contains(err.Error(), "keeps changing") || calls != 7 { + t.Fatalf("error=%v calls=%d", err, calls) + } + if !strings.Contains(out.String(), "remote-7 -> web") { + t.Fatalf("latest plan not displayed: %s", out.String()) + } +} + +func TestApplyYesMachineOutputIsStructuredPlan(t *testing.T) { + const response = `{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","changes":[{"field":"name","from":"old","to":"web"}]}` + for _, mode := range []string{"--agent", "--json"} { + t.Run(mode, func(t *testing.T) { + requests := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests++ + w.Write([]byte(response)) + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + if err := execute(app, mode, "apply", "--yes"); err != nil { + t.Fatal(err) + } + var value any + if requests != 2 || json.Unmarshal(out.Bytes(), &value) != nil { + t.Fatalf("requests=%d output=%q", requests, out.String()) + } + }) + } +} + +func TestApplyStaleMachineErrorIncludesReplacementPlan(t *testing.T) { + const first = `{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","changes":[{"field":"name","from":"old","to":"web"}]}` + const replacement = `{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","changes":[{"field":"name","from":"newer","to":"web"}]}` + calls := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + switch calls { + case 1: + w.Write([]byte(first)) + case 2: + w.WriteHeader(http.StatusConflict) + w.Write([]byte(`{"code":"CONFIGURATION_PLAN_STALE","message":"changed"}`)) + case 3: + w.Write([]byte(replacement)) + default: + t.Fatalf("unexpected request %d", calls) + } + })) + defer s.Close() + writeConfig(t, s.URL) + d := t.TempDir() + writeManifest(t, d, imageManifest) + app, out := testApp(t, d, s.Client()) + err := execute(app, "--agent", "apply", "--yes") + if err == nil || calls != 3 { + t.Fatalf("error=%v calls=%d", err, calls) + } + if err := output.Error(out, err); err != nil { + t.Fatal(err) + } + var envelope struct { + OK bool `json:"ok"` + Error string `json:"error"` + Plan applyResponse `json:"plan"` + } + if json.Unmarshal(out.Bytes(), &envelope) != nil || envelope.OK || envelope.Plan.Changes[0].From != "newer" { + t.Fatalf("output=%s", out.String()) + } +} + func TestCollectionRequestsFollowPagination(t *testing.T) { queries := map[string][]string{} s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cli/internal/cli/types.go b/cli/internal/cli/types.go index 4a2ffac7..c3155b1b 100644 --- a/cli/internal/cli/types.go +++ b/cli/internal/cli/types.go @@ -90,8 +90,16 @@ type servicesResponse struct { NextCursor string `json:"nextCursor,omitempty"` } type applyResponse struct { - Action string `json:"action"` - Changes []string `json:"changes"` + Target targetContext `json:"target"` + Action string `json:"action"` + CurrentVersion string `json:"currentVersion"` + DesiredVersion string `json:"desiredVersion"` + Changes []applyChange `json:"changes"` +} +type applyChange struct { + Field string `json:"field"` + From any `json:"from"` + To any `json:"to"` } type deployResponse struct { Operation string `json:"operation"` diff --git a/cli/internal/manifest/manifest.go b/cli/internal/manifest/manifest.go index 2bbff0bd..eb680d2a 100644 --- a/cli/internal/manifest/manifest.go +++ b/cli/internal/manifest/manifest.go @@ -13,7 +13,11 @@ import ( "gopkg.in/yaml.v3" ) -var windowsAbsolutePath = regexp.MustCompile(`^[A-Za-z]:[\\/]`) +var ( + windowsAbsolutePath = regexp.MustCompile(`^[A-Za-z]:[\\/]`) + hostnamePattern = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) + slugChars = regexp.MustCompile(`[^a-z0-9]+`) +) type Manifest struct { APIVersion string `json:"apiVersion" yaml:"apiVersion"` @@ -204,9 +208,15 @@ func Validate(m Manifest) error { default: return errors.New("service.source.type must be image or github") } - if m.Service.Hostname != nil && *m.Service.Hostname == "" { + if m.Service.Hostname == nil { + return errors.New("service.hostname is required") + } + if *m.Service.Hostname == "" { return errors.New("service.hostname cannot be blank") } + if !hostnamePattern.MatchString(*m.Service.Hostname) || len(*m.Service.Hostname) > 63 { + return errors.New("service.hostname must be at most 63 lowercase letters, numbers, and hyphen-separated segments") + } if m.Service.StartCommand != nil && *m.Service.StartCommand == "" { return errors.New("service.startCommand cannot be blank") } @@ -321,8 +331,10 @@ func CanonicalGitHubRepository(value string) (string, error) { return "https://github.com/" + parts[0] + "/" + parts[1], nil } -var slugChars = regexp.MustCompile(`[^a-z0-9]+`) - func Slugify(v string) string { - return strings.Trim(slugChars.ReplaceAllString(strings.ToLower(v), "-"), "-") + value := strings.Trim(slugChars.ReplaceAllString(strings.ToLower(v), "-"), "-") + if len(value) > 63 { + value = strings.Trim(value[:63], "-") + } + return value } diff --git a/cli/internal/manifest/manifest_test.go b/cli/internal/manifest/manifest_test.go index 0261d28a..57cdcbb7 100644 --- a/cli/internal/manifest/manifest_test.go +++ b/cli/internal/manifest/manifest_test.go @@ -6,7 +6,8 @@ import ( ) func base() Manifest { - return Manifest{APIVersion: "v1", Target: &Target{ServiceID: "s"}, Service: Service{Name: "web", Source: Source{Type: "image", Image: "nginx"}, Replicas: 1, Placement: &Placement{Mode: "automatic"}}} + hostname := "web" + return Manifest{APIVersion: "v1", Target: &Target{ServiceID: "s"}, Service: Service{Name: "web", Source: Source{Type: "image", Image: "nginx"}, Hostname: &hostname, Replicas: 1, Placement: &Placement{Mode: "automatic"}}} } func TestDefaultsAndRoundTrip(t *testing.T) { m := base() @@ -21,6 +22,24 @@ func TestDefaultsAndRoundTrip(t *testing.T) { } } +func TestHostnameValidationAndSlugify(t *testing.T) { + for _, value := range []string{"", "Upper", "two words", "-leading", "trailing-", "two--hyphens", strings.Repeat("a", 64)} { + m := base() + m.Service.Hostname = &value + if err := Validate(m); err == nil { + t.Fatalf("invalid hostname %q accepted", value) + } + } + + if got := Slugify(" My API__Service "); got != "my-api-service" { + t.Fatalf("Slugify() = %q", got) + } + long := Slugify(strings.Repeat("long-name-", 10)) + if len(long) > 63 || strings.HasSuffix(long, "-") || !hostnamePattern.MatchString(long) { + t.Fatalf("invalid truncated slug %q", long) + } +} + func TestPlacementRoundTripAndValidation(t *testing.T) { m := base() m.Service.Replicas = 3 @@ -68,6 +87,7 @@ func TestPlacementIsRequired(t *testing.T) { service: name: web source: {type: image, image: nginx} + hostname: web replicas: 2 `)) if err == nil || !strings.Contains(err.Error(), "service.placement is required") { diff --git a/cli/internal/output/output.go b/cli/internal/output/output.go index 853e18cc..f84f561c 100644 --- a/cli/internal/output/output.go +++ b/cli/internal/output/output.go @@ -17,6 +17,12 @@ type Envelope struct { type ErrorEnvelope struct { OK bool `json:"ok"` Error string `json:"error"` + Plan any `json:"plan,omitempty"` +} + +type errorWithPlan interface { + error + PlanData() any } func JSON(w io.Writer, value any) error { @@ -29,7 +35,11 @@ func OK(w io.Writer, data any, summary string) error { } func Error(w io.Writer, err error) error { - return JSON(w, ErrorEnvelope{OK: false, Error: err.Error()}) + envelope := ErrorEnvelope{OK: false, Error: err.Error()} + if planned, ok := err.(errorWithPlan); ok { + envelope.Plan = planned.PlanData() + } + return JSON(w, envelope) } func Section(w io.Writer, title string) { diff --git a/docs/api/public-api.mdx b/docs/api/public-api.mdx index 46d9b2a8..cc50887e 100644 --- a/docs/api/public-api.mdx +++ b/docs/api/public-api.mdx @@ -110,6 +110,7 @@ The paths in this table are relative to `/api/v1/services/{serviceId}`. | Method | Path suffix | Description | | --- | --- | --- | | `GET`, `PUT` | `/configuration` | Read safe current/active configuration or atomically replace all managed configuration | +| `POST` | `/configuration/plan` | Validate and plan a complete managed configuration replacement | | `GET` | `/status` | Read source, latest build and rollout, and persisted deployments | | `POST` | `/deploy` | Queue an image rollout or GitHub build | | `GET` | `/logs` | Search logs and optionally long poll | @@ -138,7 +139,9 @@ Configuration and revision responses never include secret names, values, or ciph ### Replace configuration -`PUT /configuration` is atomic and replaces the complete managed configuration. The request must contain exactly `name`, `source`, `hostname`, `ports`, `placement`, `healthCheck`, `startCommand`, and `resources`. Omitted or unknown fields are rejected. Use `null` to clear nullable fields, including `resources`. +`POST /configuration/plan` accepts the same complete desired configuration as `PUT /configuration`. It performs no writes and returns the authoritative `target`, `action`, `currentVersion`, `desiredVersion`, and structured `changes` (`field`, `from`, and `to`). Service IDs on these flat service routes are authorized installation-globally rather than being scoped by a project ID in the URL. + +`PUT /configuration` is atomic and replaces the complete managed configuration. Send the `currentVersion` returned by the plan as one quoted strong ETag (for example, `If-Match: "sha256:…"`). Missing, unquoted, multiple, weak, or otherwise invalid headers are rejected; a service change after planning returns `409 CONFIGURATION_PLAN_STALE`. The request must contain exactly `name`, `source`, `hostname`, `ports`, `placement`, `healthCheck`, `startCommand`, and `resources`. Omitted or unknown fields are rejected. `hostname` must be concrete and non-null. Use `null` to clear nullable fields, including `resources`. ```json { @@ -315,4 +318,4 @@ The CLI uses the same endpoints documented above: | `tc metrics` | Query service metrics | | `tc revisions` | List the redacted revision changelog | -`tc link` stores only the selected `target.serviceId` in `techulus.yml`. Image and GitHub services use the same `tc link`, `tc apply`, `tc deploy`, and inspection commands. +`tc link` stores only the selected `target.serviceId` in `techulus.yml`. Relinking a manifest to a different service requires removing `target.serviceId` first. `tc apply` always displays the server-generated plan and prompts before writing; use `tc apply --yes` for noninteractive automation. Image and GitHub services use the same `tc link`, `tc apply`, `tc deploy`, and inspection commands. diff --git a/web/actions/projects.ts b/web/actions/projects.ts index b95fbb29..27d3f370 100644 --- a/web/actions/projects.ts +++ b/web/actions/projects.ts @@ -51,7 +51,10 @@ import type { HealthCheckConfig as ServiceHealthCheckConfig, } from "@/lib/service-config"; import { MIN_SERVERLESS_SLEEP_AFTER_SECONDS } from "@/lib/service-config"; -import { findServicePortValidationIssue } from "@/lib/service-revision-spec"; +import { + findServicePortValidationIssue, + getDefaultServiceHostname, +} from "@/lib/service-revision-spec"; import type { DeleteConfirmation } from "@/lib/two-factor"; import { getZodErrorMessage, slugify } from "@/lib/utils"; import { @@ -281,7 +284,10 @@ export async function createService(input: CreateServiceInput) { } const id = randomUUID(); - const hostname = `${project.slug}-${slugify(name)}-${env.name}`; + const hostname = getDefaultServiceHostname( + `${project.slug}-${name}-${env.name}`, + id, + ); const newServiceCanvasPosition = { canvasX: (SERVICE_CANVAS_WIDTH - SERVICE_CARD_WIDTH) / 2, canvasY: 0, @@ -661,23 +667,35 @@ export async function updateServiceHostname( } const sanitized = slugify(hostname); - if (!sanitized) { - throw new Error("Invalid hostname"); + if (!sanitized || sanitized.length > 63) { + throw new Error( + "Hostname must be a valid DNS label of at most 63 characters", + ); } - const existing = await db - .select({ id: services.id }) - .from(services) - .where(eq(services.hostname, sanitized)); + await db.transaction(async (tx) => { + await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`); + const current = await tx + .select({ id: services.id }) + .from(services) + .where(and(eq(services.id, serviceId), isNull(services.deletedAt))) + .limit(1) + .then((rows) => rows[0]); + if (!current) throw new Error("Service not found"); + const existing = await tx + .select({ id: services.id }) + .from(services) + .where(eq(services.hostname, sanitized)); - if (existing.some((s) => s.id !== serviceId)) { - throw new Error("Hostname is already in use"); - } + if (existing.some((s) => s.id !== serviceId)) { + throw new Error("Hostname is already in use"); + } - await db - .update(services) - .set({ hostname: sanitized }) - .where(eq(services.id, serviceId)); + await tx + .update(services) + .set({ hostname: sanitized }) + .where(and(eq(services.id, serviceId), isNull(services.deletedAt))); + }); return { success: true, hostname: sanitized }; } @@ -687,10 +705,27 @@ export async function updateServiceName(serviceId: string, name: string) { try { const validatedName = nameSchema.parse(name); - await db - .update(services) - .set({ name: validatedName }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + const current = await tx + .select({ name: services.name, hostname: services.hostname }) + .from(services) + .where(and(eq(services.id, serviceId), isNull(services.deletedAt))) + .limit(1) + .then((rows) => rows[0]); + if (!current) throw new Error("Service not found"); + await tx + .update(services) + .set({ + name: validatedName, + hostname: + current.hostname ?? + getDefaultServiceHostname(current.name, serviceId), + }) + .where(and(eq(services.id, serviceId), isNull(services.deletedAt))); + }); return { success: true, name: validatedName }; } catch (error) { @@ -738,6 +773,9 @@ export async function updateServiceGithubRepo( } await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); await tx .update(services) .set(updateData) @@ -794,16 +832,19 @@ export async function updateServiceHealthCheck( throw new Error("Service not found"); } - await db - .update(services) - .set({ - healthCheckCmd: config.cmd, - healthCheckInterval: config.interval, - healthCheckTimeout: config.timeout, - healthCheckRetries: config.retries, - healthCheckStartPeriod: config.startPeriod, - }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`); + await tx + .update(services) + .set({ + healthCheckCmd: config.cmd, + healthCheckInterval: config.interval, + healthCheckTimeout: config.timeout, + healthCheckRetries: config.retries, + healthCheckStartPeriod: config.startPeriod, + }) + .where(eq(services.id, serviceId)); + }); return { success: true }; } @@ -818,10 +859,13 @@ export async function updateServiceStartCommand( throw new Error("Service not found"); } - await db - .update(services) - .set({ startCommand }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`); + await tx + .update(services) + .set({ startCommand }) + .where(eq(services.id, serviceId)); + }); return { success: true }; } @@ -854,13 +898,16 @@ export async function updateServiceResourceLimits( throw new Error("Service not found"); } - await db - .update(services) - .set({ - resourceCpuLimit: validated.cpuCores, - resourceMemoryLimitMb: validated.memoryMb, - }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`); + await tx + .update(services) + .set({ + resourceCpuLimit: validated.cpuCores, + resourceMemoryLimitMb: validated.memoryMb, + }) + .where(eq(services.id, serviceId)); + }); return { success: true }; } @@ -1064,37 +1111,42 @@ export async function updateServiceConfig( throw new Error("Service not found"); } - if (config.source) { - await db - .update(services) - .set({ image: config.source.image }) - .where(eq(services.id, serviceId)); - } + if (config.source || config.healthCheck !== undefined) { + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + if (config.source) { + await tx + .update(services) + .set({ image: config.source.image }) + .where(eq(services.id, serviceId)); + } - if (config.healthCheck !== undefined) { - if (config.healthCheck === null) { - await db - .update(services) - .set({ - healthCheckCmd: null, - healthCheckInterval: null, - healthCheckTimeout: null, - healthCheckRetries: null, - healthCheckStartPeriod: null, - }) - .where(eq(services.id, serviceId)); - } else { - await db - .update(services) - .set({ - healthCheckCmd: config.healthCheck.cmd, - healthCheckInterval: config.healthCheck.interval, - healthCheckTimeout: config.healthCheck.timeout, - healthCheckRetries: config.healthCheck.retries, - healthCheckStartPeriod: config.healthCheck.startPeriod, - }) - .where(eq(services.id, serviceId)); - } + if (config.healthCheck === null) { + await tx + .update(services) + .set({ + healthCheckCmd: null, + healthCheckInterval: null, + healthCheckTimeout: null, + healthCheckRetries: null, + healthCheckStartPeriod: null, + }) + .where(eq(services.id, serviceId)); + } else if (config.healthCheck !== undefined) { + await tx + .update(services) + .set({ + healthCheckCmd: config.healthCheck.cmd, + healthCheckInterval: config.healthCheck.interval, + healthCheckTimeout: config.healthCheck.timeout, + healthCheckRetries: config.healthCheck.retries, + healthCheckStartPeriod: config.healthCheck.startPeriod, + }) + .where(eq(services.id, serviceId)); + } + }); } if (config.ports) { @@ -1552,19 +1604,24 @@ export async function removeServiceVolume(volumeId: string) { throw new Error("Stop the service before removing volumes"); } - await db.delete(serviceVolumes).where(eq(serviceVolumes.id, volumeId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${volume[0].serviceId}))`, + ); + await tx.delete(serviceVolumes).where(eq(serviceVolumes.id, volumeId)); - const remainingVolumes = await db - .select({ id: serviceVolumes.id }) - .from(serviceVolumes) - .where(eq(serviceVolumes.serviceId, volume[0].serviceId)); + const remainingVolumes = await tx + .select({ id: serviceVolumes.id }) + .from(serviceVolumes) + .where(eq(serviceVolumes.serviceId, volume[0].serviceId)); - if (remainingVolumes.length === 0 && service.stateful) { - await db - .update(services) - .set({ stateful: false }) - .where(eq(services.id, service.id)); - } + if (remainingVolumes.length === 0 && service.stateful) { + await tx + .update(services) + .set({ stateful: false }) + .where(eq(services.id, service.id)); + } + }); return { success: true }; } diff --git a/web/app/api/v1/services/[serviceId]/configuration/plan/route.ts b/web/app/api/v1/services/[serviceId]/configuration/plan/route.ts new file mode 100644 index 00000000..46b229cc --- /dev/null +++ b/web/app/api/v1/services/[serviceId]/configuration/plan/route.ts @@ -0,0 +1 @@ +export { postConfigurationPlanRoute as POST } from "@/lib/public-api-routes"; diff --git a/web/lib/inngest/functions/migration-workflow.ts b/web/lib/inngest/functions/migration-workflow.ts index 6ac80c79..9f1a6cc0 100644 --- a/web/lib/inngest/functions/migration-workflow.ts +++ b/web/lib/inngest/functions/migration-workflow.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { and, eq } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import { db } from "@/db"; import { getBackupStorageConfig } from "@/db/queries"; import { @@ -301,26 +301,31 @@ export const migrationWorkflow = inngest.createFunction( } await step.run("deploy-target", async () => { - await db - .update(services) - .set({ migrationStatus: "deploying_target" }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + await tx + .update(services) + .set({ migrationStatus: "deploying_target" }) + .where(eq(services.id, serviceId)); - await db - .delete(serviceReplicas) - .where(eq(serviceReplicas.serviceId, serviceId)); - - await db.insert(serviceReplicas).values({ - id: randomUUID(), - serviceId, - serverId: targetServerId, - count: 1, - }); + await tx + .delete(serviceReplicas) + .where(eq(serviceReplicas.serviceId, serviceId)); - await db - .update(services) - .set({ lockedServerId: targetServerId }) - .where(eq(services.id, serviceId)); + await tx.insert(serviceReplicas).values({ + id: randomUUID(), + serviceId, + serverId: targetServerId, + count: 1, + }); + + await tx + .update(services) + .set({ lockedServerId: targetServerId }) + .where(eq(services.id, serviceId)); + }); await deployServiceInternal(serviceId, actor, { runtimeBaseRevisionId: sourceServiceRevisionId, diff --git a/web/lib/inngest/functions/service-deletion-workflow.ts b/web/lib/inngest/functions/service-deletion-workflow.ts index 13f4b05e..a7fd41be 100644 --- a/web/lib/inngest/functions/service-deletion-workflow.ts +++ b/web/lib/inngest/functions/service-deletion-workflow.ts @@ -8,6 +8,7 @@ import { isNull, lte, or, + sql, } from "drizzle-orm"; import { cron } from "inngest"; import { db } from "@/db"; @@ -276,17 +277,29 @@ export const serviceDeletionWorkflow = inngest.createFunction( } const deletedAt = new Date(); - await db - .update(services) - .set({ - deletedAt, - purgeAfter: addUtcDays(deletedAt, DELETED_SERVICE_RETENTION_DAYS), - originalHostname: setup.service.hostname, - hostname: null, - deletionStatus: null, - deletionError: null, - }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + const current = await tx + .select({ hostname: services.hostname }) + .from(services) + .where(eq(services.id, serviceId)) + .limit(1) + .then((rows) => rows[0]); + if (!current) throw new Error("Service not found"); + await tx + .update(services) + .set({ + deletedAt, + purgeAfter: addUtcDays(deletedAt, DELETED_SERVICE_RETENTION_DAYS), + originalHostname: current.hostname, + hostname: null, + deletionStatus: null, + deletionError: null, + }) + .where(eq(services.id, serviceId)); + }); }); return { status: "deleted", serviceId, backupIds }; @@ -476,18 +489,23 @@ export const serviceRestoreWorkflow = inngest.createFunction( const deployResult = await step.run( "start-restored-deployment", async () => { - await db - .update(services) - .set({ - deletedAt: null, - purgeAfter: null, - hostname: setup.service.originalHostname, - originalHostname: null, - deletionStatus: "restoring", - deletionError: null, - lockedServerId: setup.targetServerId, - }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + await tx + .update(services) + .set({ + deletedAt: null, + purgeAfter: null, + hostname: setup.service.originalHostname, + originalHostname: null, + deletionStatus: "restoring", + deletionError: null, + lockedServerId: setup.targetServerId, + }) + .where(eq(services.id, serviceId)); + }); try { const result = await deployServiceInternal(serviceId, actor, { @@ -498,20 +516,25 @@ export const serviceRestoreWorkflow = inngest.createFunction( } return result; } catch (error) { - await db - .update(services) - .set({ - deletedAt: toDate(setup.service.deletedAt), - purgeAfter: toDate(setup.service.purgeAfter), - hostname: null, - originalHostname: setup.service.originalHostname, - deletionStatus: "failed", - deletionError: - error instanceof Error - ? error.message - : "Restore deployment failed", - }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + await tx + .update(services) + .set({ + deletedAt: toDate(setup.service.deletedAt), + purgeAfter: toDate(setup.service.purgeAfter), + hostname: null, + originalHostname: setup.service.originalHostname, + deletionStatus: "failed", + deletionError: + error instanceof Error + ? error.message + : "Restore deployment failed", + }) + .where(eq(services.id, serviceId)); + }); throw error; } }, @@ -552,19 +575,24 @@ export const serviceRestoreWorkflow = inngest.createFunction( if (!healthyDeployment || failedDeployment) { await step.run("mark-restore-deployment-failed", async () => { - await db - .update(services) - .set({ - deletedAt: toDate(setup.service.deletedAt), - purgeAfter: toDate(setup.service.purgeAfter), - hostname: null, - originalHostname: setup.service.originalHostname, - deletionStatus: "failed", - deletionError: - failedDeployment?.failedStage || - "Restore deployment did not become healthy", - }) - .where(eq(services.id, serviceId)); + await db.transaction(async (tx) => { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtext(${serviceId}))`, + ); + await tx + .update(services) + .set({ + deletedAt: toDate(setup.service.deletedAt), + purgeAfter: toDate(setup.service.purgeAfter), + hostname: null, + originalHostname: setup.service.originalHostname, + deletionStatus: "failed", + deletionError: + failedDeployment?.failedStage || + "Restore deployment did not become healthy", + }) + .where(eq(services.id, serviceId)); + }); }); return { status: "failed", reason: "deployment" }; } diff --git a/web/lib/public-api-routes.ts b/web/lib/public-api-routes.ts index 8721a186..fd206109 100644 --- a/web/lib/public-api-routes.ts +++ b/web/lib/public-api-routes.ts @@ -17,6 +17,7 @@ import { findServiceContext, isPublicApiDomainError, notFound, + planConfiguration, publicApiDomainResponse, replaceConfiguration, replaceConfigurationSchema, @@ -47,6 +48,11 @@ export type PublicServiceParams = { export type PublicServiceContext = { params: Promise }; const readRoles = ["admin", "developer", "reader"] as const; +export function parseConfigurationIfMatch(value: string | null): string | null { + const match = value?.match(/^"(sha256:[a-f0-9]{64})"$/); + return match?.[1] ?? null; +} + async function readScope(request: Request, context: PublicServiceContext) { const auth = await requireApiKeyRole(request, [...readRoles]); if (!auth.ok) return { response: auth.response }; @@ -162,9 +168,15 @@ export async function putConfigurationRoute( parsed.error.issues[0]?.message ?? "Invalid configuration", ); } + const expectedVersion = parseConfigurationIfMatch( + request.headers.get("if-match"), + ); + if (!expectedVersion) { + return badRequest("A valid If-Match configuration version is required"); + } try { return Response.json( - await replaceConfiguration(scope.service, parsed.data), + await replaceConfiguration(scope.service, parsed.data, expectedVersion), ); } catch (error) { return isPublicApiDomainError(error) @@ -173,6 +185,42 @@ export async function putConfigurationRoute( } } +export async function postConfigurationPlanRoute( + request: Request, + context: PublicServiceContext, +) { + const scope = await writeScope(request, context); + if ("response" in scope) return scope.response; + const parsed = replaceConfigurationSchema.safeParse( + await request.json().catch(() => null), + ); + if (!parsed.success) + return badRequest( + parsed.error.issues[0]?.message ?? "Invalid configuration", + ); + try { + const { targetServiceName, ...plan } = await planConfiguration( + scope.service, + parsed.data, + ); + return Response.json({ + target: { + project: { id: scope.target.projectId, slug: scope.target.projectSlug }, + environment: { + id: scope.target.environmentId, + name: scope.target.environmentName, + }, + service: { id: scope.service.id, name: targetServiceName }, + }, + ...plan, + }); + } catch (error) { + return isPublicApiDomainError(error) + ? publicApiDomainResponse(error) + : internalError(error, "plan configuration"); + } +} + const safeDeployment = { id: deployments.id, serviceRevisionId: deployments.serviceRevisionId, diff --git a/web/lib/public-api.ts b/web/lib/public-api.ts index 99ca6141..3f56c78e 100644 --- a/web/lib/public-api.ts +++ b/web/lib/public-api.ts @@ -1,4 +1,4 @@ -import { randomUUID } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { and, desc, eq, inArray, isNull, ne, sql } from "drizzle-orm"; import { z } from "zod"; import { db } from "@/db"; @@ -388,7 +388,9 @@ export async function safeConfiguration(service: NestedService) { }; const current = { source, - hostname: service.hostname, + hostname: + service.hostname?.trim() || + getDefaultServiceHostname(service.name, service.id), stateful: service.stateful, replicas: replicaCount, placements: sortedPlacements, @@ -452,7 +454,7 @@ export async function safeConfiguration(service: NestedService) { const comparableCurrent = { source: current.source, - hostname: current.hostname?.trim() || getDefaultServiceHostname(service.id), + hostname: current.hostname, stateful: current.stateful, placement: current.placement.mode === "automatic" @@ -587,7 +589,7 @@ export const placementSchema = z.discriminatedUnion("mode", [ export const replaceConfigurationSchema = z.strictObject({ name: nameSchema, source: publicSourceSchema, - hostname: hostnameSchema.nullable(), + hostname: hostnameSchema, ports: z.array(portSchema).max(100), placement: placementSchema, healthCheck: healthCheckSchema.nullable(), @@ -637,11 +639,176 @@ function healthCheckFromService(service: NestedService) { : null; } +type ReplacementInput = z.infer; +type ConfigurationChange = { field: string; from: unknown; to: unknown }; + +function canonicalPlanSource(source: PublicSource) { + return source.type === "github" + ? { + ...source, + repository: source.repository?.toLowerCase() ?? null, + } + : source; +} + +function canonicalReplacementState( + service: NestedService, + source: ReturnType, + ports: Array<{ port: number; isPublic: boolean; domain: string | null }>, + placements: Array<{ serverId: string; count: number }>, +) { + const resources = + service.resourceCpuLimit == null && service.resourceMemoryLimitMb == null + ? null + : { + cpuCores: service.resourceCpuLimit, + memoryMb: service.resourceMemoryLimitMb, + }; + return { + name: service.name, + source: canonicalPlanSource(source), + hostname: + service.hostname?.trim() || + getDefaultServiceHostname(service.name, service.id), + ports: ports + .map((port) => ({ + containerPort: port.port, + public: port.isPublic, + domain: port.domain, + })) + .toSorted( + (a, b) => + a.containerPort - b.containerPort || + Number(a.public) - Number(b.public) || + (a.domain ?? "").localeCompare(b.domain ?? "", "en"), + ), + placement: + service.placementMode === "automatic" + ? { mode: "automatic" as const, replicas: service.replicas } + : { + mode: "manual" as const, + placements: placements + .map(({ serverId, count }) => ({ serverId, count })) + .toSorted((a, b) => a.serverId.localeCompare(b.serverId, "en")), + }, + healthCheck: healthCheckFromService(service), + startCommand: service.startCommand?.trim() || null, + resources, + }; +} + +export function canonicalDesired(input: ReplacementInput) { + return { + ...input, + source: canonicalPlanSource(input.source), + ports: input.ports + .map((port) => ({ + ...port, + domain: port.domain ?? null, + })) + .toSorted( + (a, b) => + a.containerPort - b.containerPort || + Number(a.public) - Number(b.public) || + (a.domain ?? "").localeCompare(b.domain ?? "", "en"), + ), + placement: + input.placement.mode === "manual" + ? { + ...input.placement, + placements: input.placement.placements.toSorted((a, b) => + a.serverId.localeCompare(b.serverId, "en"), + ), + } + : input.placement, + }; +} + +function fingerprint(value: unknown) { + return `sha256:${createHash("sha256").update(JSON.stringify(value)).digest("hex")}`; +} + +function configurationChanges( + current: Record, + desired: Record, +) { + const changes: ConfigurationChange[] = []; + const compare = (field: string, from: unknown, to: unknown) => { + if (JSON.stringify(from) === JSON.stringify(to)) return; + if ( + from !== null && + to !== null && + typeof from === "object" && + typeof to === "object" && + !Array.isArray(from) && + !Array.isArray(to) + ) { + for (const key of new Set([ + ...Object.keys(from as Record), + ...Object.keys(to as Record), + ])) + compare( + `${field}.${key}`, + (from as Record)[key], + (to as Record)[key], + ); + return; + } + changes.push({ + field, + from: from === undefined ? null : from, + to: to === undefined ? null : to, + }); + }; + for (const field of Object.keys(desired)) + compare(field, current[field], desired[field]); + return changes; +} + +export function planCanonicalConfiguration( + current: ReturnType, + desiredInput: ReplacementInput, +) { + const canonicalCurrent = { + ...current, + source: canonicalPlanSource(current.source), + }; + const desired = canonicalDesired(desiredInput); + const changes = configurationChanges(canonicalCurrent, desired); + return { + action: changes.length ? ("updated" as const) : ("noop" as const), + currentVersion: fingerprint(canonicalCurrent), + desiredVersion: fingerprint(desired), + changes, + }; +} + +export async function planConfiguration( + service: NestedService, + input: ReplacementInput, +) { + return replaceConfigurationInternal(service, input, null); +} + export async function replaceConfiguration( service: NestedService, - input: z.infer, + input: ReplacementInput, + expectedVersion: string, ) { - if (input.source?.type === "image" && input.source.image !== service.image) { + const { targetServiceName: _, ...plan } = await replaceConfigurationInternal( + service, + input, + expectedVersion, + ); + return plan; +} + +async function replaceConfigurationInternal( + service: NestedService, + input: ReplacementInput, + expectedVersion: string | null, +) { + if (input.source.type === "image") { const validation = await validateDockerImageInternal(input.source.image); if (!validation.valid) { domainError(validation.error || "Invalid image", "INVALID_IMAGE", 400); @@ -659,7 +826,6 @@ export async function replaceConfiguration( .limit(1) .then((rows) => rows[0]); if (!persisted) domainError("Service not found", "NOT_FOUND", 404); - const [ports, volumes, placements, repo] = await Promise.all([ tx .select() @@ -681,8 +847,21 @@ export async function replaceConfiguration( .then((rows) => rows[0]), ]); const source = resolvePersistedSourceFromRows(persisted, repo); + const currentState = canonicalReplacementState( + persisted, + source, + ports, + placements, + ); + const plan = planCanonicalConfiguration(currentState, input); + if (expectedVersion !== null && plan.currentVersion !== expectedVersion) { + domainError( + "Service configuration changed after the plan was created", + "CONFIGURATION_PLAN_STALE", + ); + } if ( - input.placement?.mode === "automatic" && + input.placement.mode === "automatic" && (persisted.stateful || volumes.length > 0) ) { domainError( @@ -701,13 +880,13 @@ export async function replaceConfiguration( domainError(blockers[0].message, blockers[0].code); } - if (input.source && input.source.type !== persisted.sourceType) { + if (input.source.type !== persisted.sourceType) { domainError( "Source type conversion is not supported; change the source in the web UI", "SOURCE_TYPE_CONVERSION", ); } - if (input.source?.type === "github") { + if (input.source.type === "github") { if (source.type !== "github" || !source.repository) { domainError( "The service does not have a valid linked GitHub repository", @@ -724,7 +903,7 @@ export async function replaceConfiguration( ); } } - if (input.placement?.mode === "manual") { + if (input.placement.mode === "manual") { const ids = input.placement.placements.map((item) => item.serverId); const selected = await tx .select({ @@ -762,63 +941,59 @@ export async function replaceConfiguration( ); } - if (input.hostname) { + const duplicateHostname = await tx + .select({ id: services.id }) + .from(services) + .where( + and(eq(services.hostname, input.hostname), ne(services.id, service.id)), + ) + .limit(1) + .then((rows) => rows[0]); + if (duplicateHostname) { + domainError("Hostname is already in use", "HOSTNAME_CONFLICT"); + } + if ( + persisted.serverlessEnabled && + !input.ports.some((port) => port.public && port.domain) + ) { + domainError( + "Serverless services require a public HTTP port with a domain", + "SERVERLESS_PORT_REQUIRED", + 400, + ); + } + const portIssue = findServicePortValidationIssue( + input.ports.map((port) => ({ + containerPort: port.containerPort, + isPublic: port.public, + domain: port.domain ?? null, + protocol: "http" as const, + })), + ); + if (portIssue) { + domainError(portIssue.message, portIssue.code, 400); + } + const domains = input.ports.flatMap((port) => + port.public && port.domain ? [port.domain] : [], + ); + for (const domain of domains) { const duplicate = await tx - .select({ id: services.id }) - .from(services) + .select({ id: servicePorts.id }) + .from(servicePorts) .where( and( - eq(services.hostname, input.hostname), - ne(services.id, service.id), + eq(servicePorts.domain, domain), + ne(servicePorts.serviceId, service.id), ), ) .limit(1) .then((rows) => rows[0]); if (duplicate) { - domainError("Hostname is already in use", "HOSTNAME_CONFLICT"); + domainError("Port domain is already in use", "DOMAIN_CONFLICT"); } } - if (input.ports) { - if ( - persisted.serverlessEnabled && - !input.ports.some((port) => port.public && port.domain) - ) { - domainError( - "Serverless services require a public HTTP port with a domain", - "SERVERLESS_PORT_REQUIRED", - 400, - ); - } - const portIssue = findServicePortValidationIssue( - input.ports.map((port) => ({ - containerPort: port.containerPort, - isPublic: port.public, - domain: port.domain ?? null, - protocol: "http" as const, - })), - ); - if (portIssue) { - domainError(portIssue.message, portIssue.code, 400); - } - const domains = input.ports.flatMap((port) => - port.public && port.domain ? [port.domain] : [], - ); - for (const domain of domains) { - const duplicate = await tx - .select({ id: servicePorts.id }) - .from(servicePorts) - .where( - and( - eq(servicePorts.domain, domain), - ne(servicePorts.serviceId, service.id), - ), - ) - .limit(1) - .then((rows) => rows[0]); - if (duplicate) { - domainError("Port domain is already in use", "DOMAIN_CONFLICT"); - } - } + if (expectedVersion === null) { + return { targetServiceName: persisted.name, ...plan }; } const changes: string[] = []; @@ -830,20 +1005,20 @@ export async function replaceConfiguration( }; if (changed("name", persisted.name, input.name)) set.name = input.name; - if ( - input.hostname !== undefined && - changed("hostname", persisted.hostname, input.hostname) - ) { + const hostnameChanged = changed( + "hostname", + currentState.hostname, + input.hostname, + ); + if (!persisted.hostname?.trim() || hostnameChanged) { set.hostname = input.hostname; } if ( - input.startCommand !== undefined && - changed("startCommand", persisted.startCommand, input.startCommand) + changed("startCommand", currentState.startCommand, input.startCommand) ) { set.startCommand = input.startCommand; } if ( - input.healthCheck !== undefined && changed( "healthCheck", healthCheckFromService(persisted), @@ -886,7 +1061,7 @@ export async function replaceConfiguration( set.resourceMemoryLimitMb = input.resources?.memoryMb ?? null; } if ( - input.source?.type === "image" && + input.source.type === "image" && changed("source.image", persisted.image, input.source.image) ) { set.image = input.source.image; @@ -935,7 +1110,7 @@ export async function replaceConfiguration( ); } } - if (input.source?.type === "github") { + if (input.source.type === "github") { const effectiveBranch = repo?.deployBranch || repo?.defaultBranch || @@ -1010,9 +1185,6 @@ export async function replaceConfiguration( } } - return { - action: changes.length > 0 ? ("updated" as const) : ("noop" as const), - changes, - }; + return { targetServiceName: persisted.name, ...plan }; }); } diff --git a/web/lib/service-revision-spec.ts b/web/lib/service-revision-spec.ts index af8de6cd..0bad7661 100644 --- a/web/lib/service-revision-spec.ts +++ b/web/lib/service-revision-spec.ts @@ -1,10 +1,21 @@ export const SERVICE_REVISION_SCHEMA_VERSION = 3 as const; -export function getDefaultServiceHostname(name: string): string { - return name - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-|-$/g, ""); +export function getDefaultServiceHostname( + name: string, + serviceId: string, +): string { + const dnsLabel = (value: string) => + value + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-|-$/g, ""); + const fromId = dnsLabel(serviceId); + const value = dnsLabel(name) || (fromId ? `service-${fromId}` : "service"); + if (value.length <= 63) return value; + + const suffix = (fromId || "service").slice(0, 8); + const prefix = value.slice(0, 62 - suffix.length).replace(/-+$/g, ""); + return `${prefix}-${suffix}`; } export type ServiceRevisionHealthCheck = { @@ -279,7 +290,9 @@ export function buildServiceRevisionSpec( schemaVersion: SERVICE_REVISION_SCHEMA_VERSION, image, source: overrides.source ?? { type: "image", image }, - hostname: service.hostname?.trim() || getDefaultServiceHostname(service.id), + hostname: + service.hostname?.trim() || + getDefaultServiceHostname(service.name, service.id), stateful: service.stateful ?? false, serverless: { enabled: service.serverlessEnabled ?? false, diff --git a/web/tests/public-api-configuration.test.ts b/web/tests/public-api-configuration.test.ts index 0850281c..904c08f3 100644 --- a/web/tests/public-api-configuration.test.ts +++ b/web/tests/public-api-configuration.test.ts @@ -42,7 +42,7 @@ describe("public API configuration state", () => { schemaVersion: 2, image: "nginx:1.27", source: { type: "image", image: "nginx:1.27" }, - hostname: "service-1", + hostname: "hello-service", stateful: false, serverless: { enabled: false, @@ -84,8 +84,8 @@ describe("public API configuration state", () => { backupSchedule: null, } as never); - expect(configuration.current.hostname).toBeNull(); - expect(configuration.active?.hostname).toBe("service-1"); + expect(configuration.current.hostname).toBe("hello-service"); + expect(configuration.active?.hostname).toBe("hello-service"); expect(configuration.hasPendingChanges).toBe(false); expect(configuration.changes).toEqual([]); }); diff --git a/web/tests/public-api-plan.test.ts b/web/tests/public-api-plan.test.ts new file mode 100644 index 00000000..afe86240 --- /dev/null +++ b/web/tests/public-api-plan.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it } from "vitest"; +import { canonicalDesired, planCanonicalConfiguration } from "@/lib/public-api"; +import { parseConfigurationIfMatch } from "@/lib/public-api-routes"; + +const version = `sha256:${"a".repeat(64)}`; + +describe("configuration plan protocol", () => { + it.each([ + [null, null], + [version, null], + [`W/"${version}"`, null], + [`"${version}", "${version}"`, null], + [`"sha256:${"A".repeat(64)}"`, null], + [`"${version}"`, version], + ])("parses If-Match %j", (input, expected) => { + expect(parseConfigurationIfMatch(input)).toBe(expected); + }); + + it("normalizes an omitted private port domain to the apply representation", () => { + const desired = canonicalDesired({ + name: "web", + source: { type: "image", image: "nginx:1.27" }, + hostname: "web", + ports: [{ containerPort: 8080, public: false }], + placement: { mode: "automatic", replicas: 1 }, + healthCheck: null, + startCommand: null, + resources: null, + }); + expect(desired.ports).toEqual([ + { containerPort: 8080, public: false, domain: null }, + ]); + }); + + it("treats GitHub repository casing as the same repository identity", () => { + const current = { + name: "web", + source: { + type: "github" as const, + repository: "https://github.com/Techulus/Cloud", + branch: "main", + rootDir: null, + }, + hostname: "web", + ports: [], + placement: { mode: "automatic" as const, replicas: 1 }, + healthCheck: null, + startCommand: null, + resources: null, + }; + const result = planCanonicalConfiguration(current, { + ...current, + source: { + ...current.source, + repository: "https://github.com/techulus/cloud", + }, + }); + + expect(result.action).toBe("noop"); + expect(result.changes).toEqual([]); + }); + + it("sorts ports and manual placements deterministically", () => { + const desired = canonicalDesired({ + name: "web", + source: { type: "image", image: "nginx" }, + hostname: "web", + ports: [ + { containerPort: 9000, public: false }, + { containerPort: 8000, public: false }, + ], + placement: { + mode: "manual", + placements: [ + { serverId: "z", count: 1 }, + { serverId: "a", count: 1 }, + ], + }, + healthCheck: null, + startCommand: null, + resources: null, + }); + expect(desired.ports.map((port) => port.containerPort)).toEqual([ + 8000, 9000, + ]); + expect( + desired.placement.mode === "manual" && + desired.placement.placements.map((placement) => placement.serverId), + ).toEqual(["a", "z"]); + }); + + it("reports every managed field change, including removals and null clears", () => { + const current = { + name: "old-web", + source: { + type: "github" as const, + repository: "https://github.com/acme/web", + branch: "develop", + rootDir: "apps/web", + }, + hostname: "web", + ports: [{ containerPort: 8080, public: true, domain: "old.example.com" }], + placement: { + mode: "manual" as const, + placements: [{ serverId: "server-a", count: 1 }], + }, + healthCheck: { + cmd: "curl localhost:8080", + interval: 10, + timeout: 5, + retries: 3, + startPeriod: 30, + }, + startCommand: "npm start", + resources: { cpuCores: 2, memoryMb: 512 }, + }; + const result = planCanonicalConfiguration(current, { + name: "web", + source: { + type: "github", + repository: "https://github.com/acme/web", + branch: "main", + rootDir: null, + }, + hostname: "web-internal", + ports: [{ containerPort: 3000, public: false }], + placement: { mode: "automatic", replicas: 3 }, + healthCheck: null, + startCommand: null, + resources: null, + }); + + expect(result.action).toBe("updated"); + expect(result.currentVersion).toMatch(/^sha256:[a-f0-9]{64}$/); + expect(result.desiredVersion).toMatch(/^sha256:[a-f0-9]{64}$/); + expect(result.changes.map((change) => change.field)).toEqual([ + "name", + "source.branch", + "source.rootDir", + "hostname", + "ports", + "placement.mode", + "placement.placements", + "placement.replicas", + "healthCheck", + "startCommand", + "resources", + ]); + expect( + result.changes.find((change) => change.field === "source.rootDir"), + ).toEqual({ field: "source.rootDir", from: "apps/web", to: null }); + for (const change of JSON.parse(JSON.stringify(result.changes))) { + expect(Object.hasOwn(change, "from")).toBe(true); + expect(Object.hasOwn(change, "to")).toBe(true); + } + }); + + it("produces a no-op and stable desired fingerprint for equivalent ordering", () => { + const current = { + name: "web", + source: { type: "image" as const, image: "nginx" }, + hostname: "web", + ports: [ + { containerPort: 8000, public: false, domain: null }, + { containerPort: 9000, public: false, domain: null }, + ], + placement: { + mode: "manual" as const, + placements: [ + { serverId: "a", count: 1 }, + { serverId: "z", count: 1 }, + ], + }, + healthCheck: null, + startCommand: null, + resources: null, + }; + const desired = { + name: "web", + source: { type: "image" as const, image: "nginx" }, + hostname: "web", + ports: [ + { containerPort: 9000, public: false }, + { containerPort: 8000, public: false }, + ], + placement: { + mode: "manual" as const, + placements: [ + { serverId: "z", count: 1 }, + { serverId: "a", count: 1 }, + ], + }, + healthCheck: null, + startCommand: null, + resources: null, + }; + const first = planCanonicalConfiguration(current, desired); + const second = planCanonicalConfiguration(current, { + ...desired, + ports: desired.ports.toReversed(), + placement: { + ...desired.placement, + placements: desired.placement.placements.toReversed(), + }, + }); + + expect(first.action).toBe("noop"); + expect(first.changes).toEqual([]); + expect(second.desiredVersion).toBe(first.desiredVersion); + }); +}); diff --git a/web/tests/public-api-source.test.ts b/web/tests/public-api-source.test.ts index 94e7363d..7cfcb4c7 100644 --- a/web/tests/public-api-source.test.ts +++ b/web/tests/public-api-source.test.ts @@ -9,7 +9,7 @@ import { const completeConfiguration = (overrides: Record = {}) => ({ name: "web", source: { type: "image", image: "nginx:1.27" }, - hostname: null, + hostname: "web", ports: [], placement: { mode: "automatic", replicas: 1 }, healthCheck: null, diff --git a/web/tests/service-revision-spec.test.ts b/web/tests/service-revision-spec.test.ts index ba313fad..6bb02475 100644 --- a/web/tests/service-revision-spec.test.ts +++ b/web/tests/service-revision-spec.test.ts @@ -69,15 +69,30 @@ function draft( } describe("service revision specification", () => { - it("keeps the default hostname stable when the service is renamed", () => { + it("derives the legacy null-hostname fallback from the current service name", () => { const original = draft(); original.service.hostname = null; const renamed = draft(); renamed.service.hostname = null; renamed.service.name = "Renamed API Service"; + expect(buildServiceRevisionSpec(original).hostname).toBe("api-service"); expect(buildServiceRevisionSpec(renamed).hostname).toBe( - buildServiceRevisionSpec(original).hostname, + "renamed-api-service", + ); + }); + + it("produces a valid DNS label for long and non-ASCII legacy names", () => { + const long = draft(); + long.service.hostname = null; + long.service.name = "a".repeat(100); + const nonAscii = draft(); + nonAscii.service.hostname = null; + nonAscii.service.name = "こんにちは"; + + expect(buildServiceRevisionSpec(long).hostname).toHaveLength(63); + expect(buildServiceRevisionSpec(nonAscii).hostname).toBe( + "service-service-1", ); }); From cdcfb7883300b7c20d882a990b214612ab114ef7 Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 29 Jul 2026 11:25:31 +0000 Subject: [PATCH 3/3] fix: address manifest apply review Amp-Thread-ID: https://ampcode.com/threads/T-019fac48-72fe-7706-9ce1-45eb90eadbef Co-authored-by: Arjun Komath --- cli/internal/cli/app.go | 86 +++++++++++++++++++++++++++-------- cli/internal/cli/app_test.go | 70 ++++++++++++++++++++++++---- cli/internal/output/output.go | 4 +- docs/api/public-api.mdx | 2 +- web/lib/public-api-routes.ts | 17 ++++++- web/lib/public-api.ts | 17 ++++++- 6 files changed, 165 insertions(+), 31 deletions(-) diff --git a/cli/internal/cli/app.go b/cli/internal/cli/app.go index 452e5ad3..990de668 100644 --- a/cli/internal/cli/app.go +++ b/cli/internal/cli/app.go @@ -522,7 +522,11 @@ func (a *App) applyCommand() *cobra.Command { } else { body["placement"] = map[string]any{"mode": "manual", "placements": placement.Servers} } - base := serviceBase(loaded.Manifest) + "/configuration" + base, err := serviceBase(loaded.Manifest) + if err != nil { + return err + } + base += "/configuration" var plan applyResponse if err := client.RequestJSON(cmd.Context(), http.MethodPost, base+"/plan", nil, body, &plan); err != nil { return err @@ -532,7 +536,7 @@ func (a *App) applyCommand() *cobra.Command { staleReplans := 0 for { if !a.isMachineOutput() { - printApplyResult(a.Out, plan) + printApplyResult(a.Out, "Plan", plan) } if len(plan.Changes) == 0 { if a.isMachineOutput() { @@ -577,8 +581,9 @@ func (a *App) applyCommand() *cobra.Command { return err } if a.isMachineOutput() { - return a.writeData(plan, "Applied") + return a.writeData(result, "Applied") } + printApplyResult(a.Out, "Applied", result) return nil } }, @@ -667,18 +672,22 @@ func (a *App) deployCommand() *cobra.Command { if !loaded.Manifest.Linked() { return errors.New("service is not linked: run `tc link`") } + base, err := serviceBase(loaded.Manifest) + if err != nil { + return err + } var persisted struct { Current struct { Source manifest.Source `json:"source"` } `json:"current"` } - if err := client.RequestJSON(cmd.Context(), http.MethodGet, serviceBase(loaded.Manifest)+"/configuration", nil, nil, &persisted); err != nil { + if err := client.RequestJSON(cmd.Context(), http.MethodGet, base+"/configuration", nil, nil, &persisted); err != nil { return err } if !sourcesEqual(loaded.Manifest.Service.Source, persisted.Current.Source) { return errors.New("service source differs from techulus.yml: run `tc apply` before deploying") } - if err := client.RequestJSON(cmd.Context(), http.MethodPost, serviceBase(loaded.Manifest)+"/deploy", nil, nil, &result); err != nil { + if err := client.RequestJSON(cmd.Context(), http.MethodPost, base+"/deploy", nil, nil, &result); err != nil { return err } if a.isMachineOutput() { @@ -716,9 +725,13 @@ func (a *App) statusCommand() *cobra.Command { if err != nil { return err } + base, err := serviceBase(value) + if err != nil { + return err + } var status statusResponse client := a.client(config) - if err := client.RequestJSON(cmd.Context(), http.MethodGet, serviceBase(value)+"/status", nil, nil, &status); err != nil { + if err := client.RequestJSON(cmd.Context(), http.MethodGet, base+"/status", nil, nil, &status); err != nil { return err } if a.isMachineOutput() { @@ -811,9 +824,6 @@ func (a *App) environmentsCommand() *cobra.Command { if id == "" { return errors.New("missing --project") } - if id == "" { - return errors.New("missing --project (or link this directory)") - } out, e := fetchAllEnvironments(cmd.Context(), a.client(cfg), "/api/v1/projects/"+url.PathEscape(id)+"/environments") if e != nil { return e @@ -869,12 +879,16 @@ func (a *App) resourceCommand(name, short, suffix string, q func(*cobra.Command) if e != nil { return e } + base, e := serviceBase(m) + if e != nil { + return e + } query := url.Values{} if q != nil { query = q(cmd) } var out map[string]any - if e = a.client(cfg).RequestJSON(cmd.Context(), http.MethodGet, serviceBase(m)+suffix, query, nil, &out); e != nil { + if e = a.client(cfg).RequestJSON(cmd.Context(), http.MethodGet, base+suffix, query, nil, &out); e != nil { return e } label := strings.ToUpper(name[:1]) + name[1:] @@ -938,6 +952,10 @@ func (a *App) getRolloutResource(cmd *cobra.Command, t serviceTargetFlags, id st if e != nil { return e } + base, e := serviceBase(m) + if e != nil { + return e + } suffix := "/rollouts/" + url.PathEscape(id) query := url.Values{} if logs { @@ -948,7 +966,7 @@ func (a *App) getRolloutResource(cmd *cobra.Command, t serviceTargetFlags, id st } } var out map[string]any - if e = a.client(cfg).RequestJSON(cmd.Context(), http.MethodGet, serviceBase(m)+suffix, query, nil, &out); e != nil { + if e = a.client(cfg).RequestJSON(cmd.Context(), http.MethodGet, base+suffix, query, nil, &out); e != nil { return e } if a.isMachineOutput() { @@ -1187,11 +1205,11 @@ func (a *App) resolveServiceTarget(target serviceTargetFlags) (manifest.Manifest }, nil } -func serviceBase(value manifest.Manifest) string { +func serviceBase(value manifest.Manifest) (string, error) { if value.Target == nil || strings.TrimSpace(value.Target.ServiceID) == "" { - panic("serviceBase called without a valid service target") + return "", errors.New("service is not linked: run `tc link`") } - return "/api/v1/services/" + url.PathEscape(value.Target.ServiceID) + return "/api/v1/services/" + url.PathEscape(value.Target.ServiceID), nil } func sourcePatch(source manifest.Source) map[string]any { @@ -1502,6 +1520,10 @@ func (a *App) sleep(ctx context.Context, duration time.Duration) error { } func fetchLogs(ctx context.Context, client *api.Client, value manifest.Manifest, tail int, cursor, search, logRange string) (logsResponse, error) { + base, err := serviceBase(value) + if err != nil { + return logsResponse{}, err + } query := url.Values{} query.Set("tail", strconv.Itoa(tail)) if search != "" { @@ -1515,12 +1537,12 @@ func fetchLogs(ctx context.Context, client *api.Client, value manifest.Manifest, query.Set("wait", "20") } var result logsResponse - err := client.RequestJSON(ctx, http.MethodGet, serviceBase(value)+"/logs", query, nil, &result) + err = client.RequestJSON(ctx, http.MethodGet, base+"/logs", query, nil, &result) return result, err } -func printApplyResult(w io.Writer, result applyResponse) { - output.Section(w, "Plan") +func printApplyResult(w io.Writer, title string, result applyResponse) { + output.Section(w, title) if result.Target.Service.ID != "" { output.Field(w, "Target", fmt.Sprintf("%s/%s/%s", result.Target.Project.Slug, result.Target.Environment.Name, result.Target.Service.Name)) } @@ -1531,7 +1553,35 @@ func printApplyResult(w io.Writer, result applyResponse) { } output.Section(w, fmt.Sprintf("Changes (%d)", len(result.Changes))) for _, change := range result.Changes { - fmt.Fprintf(w, " * %s: %v -> %v\n", change.Field, change.From, change.To) + fmt.Fprintf(w, " * %s: %s -> %s\n", change.Field, formatApplyValue(change.From), formatApplyValue(change.To)) + } +} + +func formatApplyValue(value any) string { + switch typed := value.(type) { + case nil: + return "null" + case string: + return typed + case []any: + items := make([]string, 0, len(typed)) + for _, item := range typed { + items = append(items, formatApplyValue(item)) + } + return "[" + strings.Join(items, ", ") + "]" + case map[string]any: + keys := make([]string, 0, len(typed)) + for key := range typed { + keys = append(keys, key) + } + slices.Sort(keys) + fields := make([]string, 0, len(keys)) + for _, key := range keys { + fields = append(fields, key+"="+formatApplyValue(typed[key])) + } + return "(" + strings.Join(fields, ", ") + ")" + default: + return fmt.Sprint(value) } } diff --git a/cli/internal/cli/app_test.go b/cli/internal/cli/app_test.go index 90b585a4..81cd36d2 100644 --- a/cli/internal/cli/app_test.go +++ b/cli/internal/cli/app_test.go @@ -387,6 +387,7 @@ func TestApplyPlacementPayloads(t *testing.T) { func TestApplyPlansAndRequiresConfirmation(t *testing.T) { const firstVersion = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" planWithChanges := fmt.Sprintf(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":%q,"desiredVersion":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","changes":[{"field":"name","from":"old-web","to":"web"},{"field":"source.branch","from":"develop","to":"main"}]}`, firstVersion) + appliedWithChanges := fmt.Sprintf(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":%q,"desiredVersion":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","changes":[{"field":"name","from":"applied-old-web","to":"web"}]}`, firstVersion) noChanges := fmt.Sprintf(`{"target":{"project":{"id":"p","slug":"app"},"environment":{"id":"e","name":"prod"},"service":{"id":"s","name":"web"}},"action":"noop","currentVersion":%q,"desiredVersion":%q,"changes":[]}`, firstVersion, firstVersion) t.Run("no changes skips prompt and write", func(t *testing.T) { @@ -461,7 +462,7 @@ func TestApplyPlansAndRequiresConfirmation(t *testing.T) { if got := r.Header.Get("If-Match"); got != `"`+firstVersion+`"` { t.Errorf("If-Match = %q", got) } - w.Write([]byte(planWithChanges)) + w.Write([]byte(appliedWithChanges)) default: t.Errorf("unexpected request %d", len(requests)) } @@ -470,7 +471,7 @@ func TestApplyPlansAndRequiresConfirmation(t *testing.T) { writeConfig(t, s.URL) d := t.TempDir() writeManifest(t, d, imageManifest) - app, _ := testApp(t, d, s.Client()) + app, out := testApp(t, d, s.Client()) app.IsInteractive = func() bool { return true } app.In = strings.NewReader("yes\n") if err := execute(app, "apply"); err != nil { @@ -480,6 +481,7 @@ func TestApplyPlansAndRequiresConfirmation(t *testing.T) { if !reflect.DeepEqual(requests, want) { t.Fatalf("requests=%v", requests) } + assertHumanOutput(t, out.String(), "Plan", "Applied", "name: applied-old-web -> web") }) } @@ -578,14 +580,19 @@ func TestApplyStopsAfterRepeatedStalePlans(t *testing.T) { } } -func TestApplyYesMachineOutputIsStructuredPlan(t *testing.T) { - const response = `{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","changes":[{"field":"name","from":"old","to":"web"}]}` +func TestApplyYesMachineOutputUsesApplyResponse(t *testing.T) { + const plan = `{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","changes":[{"field":"name","from":"planned-old","to":"web"}]}` + const applied = `{"target":{"project":{"slug":"app"},"environment":{"name":"prod"},"service":{"id":"s","name":"web"}},"action":"updated","currentVersion":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","changes":[{"field":"name","from":"applied-old","to":"web"}]}` for _, mode := range []string{"--agent", "--json"} { t.Run(mode, func(t *testing.T) { requests := 0 s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { requests++ - w.Write([]byte(response)) + if requests == 1 { + w.Write([]byte(plan)) + return + } + w.Write([]byte(applied)) })) defer s.Close() writeConfig(t, s.URL) @@ -595,8 +602,23 @@ func TestApplyYesMachineOutputIsStructuredPlan(t *testing.T) { if err := execute(app, mode, "apply", "--yes"); err != nil { t.Fatal(err) } - var value any - if requests != 2 || json.Unmarshal(out.Bytes(), &value) != nil { + var result applyResponse + if mode == "--agent" { + if json.Unmarshal(out.Bytes(), &result) != nil { + t.Fatalf("output=%q", out.String()) + } + } else { + var envelope struct { + OK bool `json:"ok"` + Data applyResponse `json:"data"` + Summary string `json:"summary"` + } + if json.Unmarshal(out.Bytes(), &envelope) != nil || !envelope.OK || envelope.Summary != "Applied" { + t.Fatalf("output=%q", out.String()) + } + result = envelope.Data + } + if requests != 2 || len(result.Changes) != 1 || result.Changes[0].From != "applied-old" { t.Fatalf("requests=%d output=%q", requests, out.String()) } }) @@ -630,7 +652,7 @@ func TestApplyStaleMachineErrorIncludesReplacementPlan(t *testing.T) { if err == nil || calls != 3 { t.Fatalf("error=%v calls=%d", err, calls) } - if err := output.Error(out, err); err != nil { + if err := output.Error(out, fmt.Errorf("apply failed: %w", err)); err != nil { t.Fatal(err) } var envelope struct { @@ -643,6 +665,38 @@ func TestApplyStaleMachineErrorIncludesReplacementPlan(t *testing.T) { } } +func TestApplyOutputFormatsStructuredValuesForHumans(t *testing.T) { + var out bytes.Buffer + printApplyResult(&out, "Plan", applyResponse{ + Action: "updated", + Changes: []applyChange{{ + Field: "ports", + From: []any{map[string]any{ + "containerPort": float64(8080), + "domain": nil, + "public": false, + }}, + To: []any{map[string]any{ + "containerPort": float64(8080), + "domain": "api.example.com", + "public": true, + }}, + }}, + }) + + assertHumanOutput( + t, + out.String(), + "ports: [(containerPort=8080, domain=null, public=false)] -> [(containerPort=8080, domain=api.example.com, public=true)]", + ) +} + +func TestServiceBaseRejectsMissingTarget(t *testing.T) { + if _, err := serviceBase(manifest.Manifest{}); err == nil || !strings.Contains(err.Error(), "not linked") { + t.Fatalf("error = %v", err) + } +} + func TestCollectionRequestsFollowPagination(t *testing.T) { queries := map[string][]string{} s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cli/internal/output/output.go b/cli/internal/output/output.go index f84f561c..d9842f20 100644 --- a/cli/internal/output/output.go +++ b/cli/internal/output/output.go @@ -2,6 +2,7 @@ package output import ( "encoding/json" + "errors" "fmt" "io" "strings" @@ -36,7 +37,8 @@ func OK(w io.Writer, data any, summary string) error { func Error(w io.Writer, err error) error { envelope := ErrorEnvelope{OK: false, Error: err.Error()} - if planned, ok := err.(errorWithPlan); ok { + var planned errorWithPlan + if errors.As(err, &planned) { envelope.Plan = planned.PlanData() } return JSON(w, envelope) diff --git a/docs/api/public-api.mdx b/docs/api/public-api.mdx index cc50887e..dfbfb153 100644 --- a/docs/api/public-api.mdx +++ b/docs/api/public-api.mdx @@ -141,7 +141,7 @@ Configuration and revision responses never include secret names, values, or ciph `POST /configuration/plan` accepts the same complete desired configuration as `PUT /configuration`. It performs no writes and returns the authoritative `target`, `action`, `currentVersion`, `desiredVersion`, and structured `changes` (`field`, `from`, and `to`). Service IDs on these flat service routes are authorized installation-globally rather than being scoped by a project ID in the URL. -`PUT /configuration` is atomic and replaces the complete managed configuration. Send the `currentVersion` returned by the plan as one quoted strong ETag (for example, `If-Match: "sha256:…"`). Missing, unquoted, multiple, weak, or otherwise invalid headers are rejected; a service change after planning returns `409 CONFIGURATION_PLAN_STALE`. The request must contain exactly `name`, `source`, `hostname`, `ports`, `placement`, `healthCheck`, `startCommand`, and `resources`. Omitted or unknown fields are rejected. `hostname` must be concrete and non-null. Use `null` to clear nullable fields, including `resources`. +`PUT /configuration` is atomic and replaces the complete managed configuration. Send the `currentVersion` returned by the plan as one quoted strong ETag (for example, `If-Match: "sha256:…"`). Missing, unquoted, multiple, weak, or otherwise invalid headers are rejected; a service change after planning returns `409 CONFIGURATION_PLAN_STALE`. A successful response returns the authoritative target and the structured change set that was applied. The request must contain exactly `name`, `source`, `hostname`, `ports`, `placement`, `healthCheck`, `startCommand`, and `resources`. Omitted or unknown fields are rejected. `hostname` must be concrete and non-null. Use `null` to clear nullable fields, including `resources`. ```json { diff --git a/web/lib/public-api-routes.ts b/web/lib/public-api-routes.ts index fd206109..66234516 100644 --- a/web/lib/public-api-routes.ts +++ b/web/lib/public-api-routes.ts @@ -175,9 +175,22 @@ export async function putConfigurationRoute( return badRequest("A valid If-Match configuration version is required"); } try { - return Response.json( - await replaceConfiguration(scope.service, parsed.data, expectedVersion), + const result = await replaceConfiguration( + scope.service, + parsed.data, + expectedVersion, ); + return Response.json({ + target: { + project: { id: scope.target.projectId, slug: scope.target.projectSlug }, + environment: { + id: scope.target.environmentId, + name: scope.target.environmentName, + }, + service: { id: scope.service.id, name: parsed.data.name }, + }, + ...result, + }); } catch (error) { return isPublicApiDomainError(error) ? publicApiDomainResponse(error) diff --git a/web/lib/public-api.ts b/web/lib/public-api.ts index 3f56c78e..6bc13d87 100644 --- a/web/lib/public-api.ts +++ b/web/lib/public-api.ts @@ -808,11 +808,16 @@ async function replaceConfigurationInternal( input: ReplacementInput, expectedVersion: string | null, ) { - if (input.source.type === "image") { + let imageValidated = false; + if ( + input.source.type === "image" && + (service.sourceType !== "image" || input.source.image !== service.image) + ) { const validation = await validateDockerImageInternal(input.source.image); if (!validation.valid) { domainError(validation.error || "Invalid image", "INVALID_IMAGE", 400); } + imageValidated = true; } return db.transaction(async (tx) => { @@ -853,6 +858,16 @@ async function replaceConfigurationInternal( ports, placements, ); + if ( + input.source.type === "image" && + input.source.image !== persisted.image && + !imageValidated + ) { + domainError( + "Service image changed while the configuration was being validated", + "CONFIGURATION_PLAN_STALE", + ); + } const plan = planCanonicalConfiguration(currentState, input); if (expectedVersion !== null && plan.currentVersion !== expectedVersion) { domainError(