amadan

carlos / platform Public

Merge branch 'fix/preserve-adopted-app-pipeline' into main

Preserve existing release pipelines when restoring app registration
codex 4d5eab8a2cbe72dfcb3a2d727b1f5b42f7210602 parent 8c54ec7parent 5ad2d87
2 files changed, +77 −9
  • internal/console/apps_claim_test.go +68 −0
  • internal/console/server.go +9 −9

CI passed — run details

diff --git a/internal/console/apps_claim_test.go b/internal/console/apps_claim_test.go
index c62d01316..aa958c0dd 100644
--- a/internal/console/apps_claim_test.go
+++ b/internal/console/apps_claim_test.go
@@ -1,10 +1,13 @@
package console
import (
+ "bytes"
"context"
+ "errors"
"fmt"
"net/http"
"net/url"
+ "os"
"strings"
"testing"
"time"
@@ -13,6 +16,71 @@ import (
"github.com/carlosframework/platform/internal/svcrole"
)
+func TestOwnerClaimPreservesExistingPipeline(t *testing.T) {
+ for _, state := range []string{"legacy", "custom", "unreadable"} {
+ t.Run(state, func(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+ const app = "existing-site"
+ seedBucketApp(t, fx, "bab", app)
+ key := release.PipelineKey("bab", app)
+ switch state {
+ case "custom":
+ _, err := release.SetPipeline(ctx, fx.rel, fx.priv, "bab", app, []release.PipelineChannel{
+ {Name: "preview"},
+ {Name: "production", Bake: "24h", Rules: release.ChannelRules{Passkey: true, PromoteApprovals: 2, ChangeApprovals: 2}},
+ }, "paul@keymail.dev")
+ if err != nil {
+ t.Fatal(err)
+ }
+ case "unreadable":
+ if _, err := fx.rel.Put(ctx, key, []byte("invalid signed pipeline"), ""); err != nil {
+ t.Fatal(err)
+ }
+ }
+ before, etag, beforeErr := fx.rel.Get(ctx, key)
+ if state == "legacy" && !errors.Is(beforeErr, os.ErrNotExist) {
+ t.Fatalf("legacy pipeline should be absent: %v", beforeErr)
+ }
+ if state != "legacy" && beforeErr != nil {
+ t.Fatal(beforeErr)
+ }
+ token := cliToken(t, fx, "paul@keymail.dev", "claim-existing-site")
+ status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", map[string]string{"name": app}, token)
+ if status != http.StatusOK {
+ t.Fatalf("owner adoption = %d %v", status, out)
+ }
+ if has, err := fx.st.HasAppIn(OpsAccountID, app); err != nil || !has {
+ t.Fatalf("app registration = %v, %v", has, err)
+ }
+ after, afterETag, err := fx.rel.Get(ctx, key)
+ if state == "legacy" {
+ if !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("adoption created a pipeline for a legacy app: %v", err)
+ }
+ } else if err != nil || !bytes.Equal(before, after) || etag != afterETag {
+ t.Fatalf("adoption changed existing pipeline: err=%v, etag before=%q after=%q", err, etag, afterETag)
+ }
+ })
+ }
+}
+
+func TestOwnerClaimNewAppGetsDefaultPipeline(t *testing.T) {
+ fx := newConsoleFixture(t)
+ token := cliToken(t, fx, "paul@keymail.dev", "claim-fresh-site")
+ status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", map[string]string{"name": "fresh-site"}, token)
+ if status != http.StatusOK {
+ t.Fatalf("new app = %d %v", status, out)
+ }
+ pipeline, err := release.GetPipeline(context.Background(), fx.rel, fx.pub, "bab", "fresh-site")
+ if err != nil || pipeline == nil {
+ t.Fatalf("new app pipeline = %v, %v", pipeline, err)
+ }
+ if len(pipeline.Channels) != 1 || pipeline.Channels[0].Name != "edge" {
+ t.Fatalf("new app channels = %#v", pipeline.Channels)
+ }
+}
+
// seedBucketApp puts an object under an account's own app prefix WITHOUT a
// row, which is exactly what a direct-bucket ship does (direct mode skips the
// claim) and exactly the state that makes #79 unrecoverable: the objects
diff --git a/internal/console/server.go b/internal/console/server.go
index 58f4c09f9..581b63162 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -3493,6 +3493,7 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem
// objects. For a non-ops account the legacy prefix belongs to somebody
// else's app and must NOT block the name.
ownPrefix := release.AppPrefix(acct.Sqid, name)
+ adoptingExistingApp := false
prefixes := []string{ownPrefix}
if acct.ID == OpsAccountID {
prefixes = append(prefixes, release.LegacyAppPrefix(name))
@@ -3535,6 +3536,7 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem
// owed an `accounts migrate -from legacy`, and a name handed out
// over them puts an app in that migration's way.
if ownerClaim && ownerExempt(prefix, ownPrefix) {
+ adoptingExistingApp = true
continue
}
return claimTaken, "", nil
@@ -3562,15 +3564,13 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem
// account's own manifest is what the agent materializes, and minting
// already wrote it.
s.ensureAccountManifest(ctx, acct)
- // Default pipeline (release pipelines v2 spec §2): every fresh claim
- // gets the one-channel edge default up front, through the ONE place
- // both doors funnel through — so neither can end up creating an app
- // that skips it. Best-effort, like the placement write above and for
- // the same reason: an app that ends up pipeline-less is not broken, it
- // is the pre-pipelines world (spec §2, "absence means the legacy
- // ladder") — exactly as safe as every app created before this existed.
- if _, err := release.SetPipeline(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, name, release.DefaultPipelineChannels(), addr); err != nil {
- log.Printf("console: claim %s/%s: set default pipeline: %v", acct.Sqid, name, err)
+ // Existing objects belong to an app whose console record is being
+ // restored. Preserve its pipeline, including absence (the legacy ladder).
+ // Only a new app gets the default edge pipeline (pipelines v2 spec §2).
+ if !adoptingExistingApp {
+ if _, err := release.SetPipeline(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, name, release.DefaultPipelineChannels(), addr); err != nil {
+ log.Printf("console: claim %s/%s: set default pipeline: %v", acct.Sqid, name, err)
+ }
}
return claimOK, placed, nil
}