diff --git a/templates/cli/internal/cmd/initfunction_test.go b/templates/cli/internal/cmd/initfunction_test.go index c6ac57e221..45122026e5 100644 --- a/templates/cli/internal/cmd/initfunction_test.go +++ b/templates/cli/internal/cmd/initfunction_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "os" @@ -156,6 +157,68 @@ func TestNewGitHubRepositoryIsCreatedOnlyAfterReview(t *testing.T) { } } +func TestPendingRepositoryIsNotCreatedWithoutDeployment(t *testing.T) { + vcsPosts := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("content-type", "application/json") + switch { + case strings.Contains(request.URL.Path, "/providerRepositories"): + vcsPosts++ + response.WriteHeader(http.StatusCreated) + _, _ = response.Write([]byte(`{"providerRepositoryId":"repository"}`)) + case request.Method == http.MethodGet && request.URL.Path == "/functions/checkout": + response.WriteHeader(http.StatusNotFound) + _, _ = response.Write([]byte(`{"message":"not found","code":404}`)) + case request.Method == http.MethodPost && request.URL.Path == "/functions": + body := jsonx.NewObject() + if err := json.NewDecoder(request.Body).Decode(body); err != nil { + t.Errorf("decode settings body: %v", err) + } + if _, exists := body.Get("installationId"); exists { + t.Error("settings-only push sent VCS fields for a pending repository") + } + response.WriteHeader(http.StatusCreated) + _, _ = response.Write([]byte(`{"$id":"checkout"}`)) + case request.Method == http.MethodGet && request.URL.Path == "/proxy/rules": + _, _ = response.Write([]byte(`{"total":0,"rules":[]}`)) + default: + t.Errorf("unexpected request: %s %s", request.Method, request.URL.String()) + response.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + preferencesWith(t, fmt.Sprintf( + `{"current":"test","test":{"endpoint":%q,"key":"secret"}}`, server.URL)) + directory := t.TempDir() + inDirectory(t, directory) + path := filepath.Join(directory, config.LocalFileName) + contents := `{"projectId":"project","functions":[{"$id":"checkout","name":"Checkout","runtime":"node-22","entrypoint":"src/main.js","installationId":"installation","providerRepositoryName":"team/checkout","providerRepositoryPrivate":true,"providerRepositoryPending":true,"providerBranch":"main","providerRootDirectory":"./"}]}` + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + + command := &cobra.Command{Use: "push function"} + command.SetOut(&bytes.Buffer{}) + command.SetErr(&bytes.Buffer{}) + if err := runPushDeployable(command, deployables[0], deployOptions{ + ResourceID: "checkout", + Code: false, + }); err != nil { + t.Fatal(err) + } + if vcsPosts != 0 { + t.Fatalf("settings-only push created %d repositories", vcsPosts) + } + reloaded, err := config.LoadLocal(path) + if err != nil { + t.Fatal(err) + } + if !reloaded.ResourceEntries("functions")[0].GetBool("providerRepositoryPending") { + t.Fatal("settings-only push cleared pending repository intent") + } +} + func TestFunctionPreviewShowsSelectedConfiguration(t *testing.T) { buffer := &bytes.Buffer{} printFunctionPreview(buffer, functionPreview{ @@ -401,6 +464,7 @@ func TestRemoveOtherPreviewRulesPreservesCustomDomain(t *testing.T) { } func TestCreateGitFunctionDeploymentSeedsTemplateOnce(t *testing.T) { + path := filepath.Join(t.TempDir(), config.LocalFileName) var body map[string]any server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { if request.URL.Path != "/functions/checkout/deployments/template" { @@ -409,13 +473,18 @@ func TestCreateGitFunctionDeploymentSeedsTemplateOnce(t *testing.T) { if err := json.NewDecoder(request.Body).Decode(&body); err != nil { t.Errorf("decode body: %v", err) } + persisted, err := config.LoadLocal(path) + if err != nil { + t.Errorf("load config during request: %v", err) + } else if persisted.ResourceEntries("functions")[0].GetString("templateRepository") != "" { + t.Error("template coordinates were not consumed before the request") + } response.Header().Set("content-type", "application/json") response.WriteHeader(http.StatusAccepted) _, _ = response.Write([]byte(`{"$id":"deployment-1","status":"waiting"}`)) })) defer server.Close() - path := filepath.Join(t.TempDir(), config.LocalFileName) contents := `{"projectId":"project","functions":[{"$id":"checkout","templateRepository":"templates","templateOwner":"appwrite","templateRootDirectory":"node/starter","templateReference":"1.0.1","templateReferenceType":"tag"}]}` if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { t.Fatal(err) diff --git a/templates/cli/internal/cmd/pushdeploy.go b/templates/cli/internal/cmd/pushdeploy.go index ab680e5b2f..2c1594d2ed 100644 --- a/templates/cli/internal/cmd/pushdeploy.go +++ b/templates/cli/internal/cmd/pushdeploy.go @@ -680,12 +680,6 @@ func runPushDeployable( return nil } - if resource.Name == "function" { - if err := context.materializePendingFunctionRepositories(entries); err != nil { - return err - } - } - pushCode := options.Code if pushCode { confirmed, err := context.prompter.Confirm(prompt.Question{ @@ -699,6 +693,12 @@ func runPushDeployable( pushCode = confirmed } + if pushCode && resource.Name == "function" { + if err := context.materializePendingFunctionRepositories(entries); err != nil { + return err + } + } + activate := true if pushCode && !options.ActivateSet { // --force answers this rather than asking anyway: --force means "do not @@ -736,7 +736,17 @@ func runPushDeployable( } summary := pushSummary{} - if resource.Name == "function" && len(entries) > 1 { + parallel := resource.Name == "function" && len(entries) > 1 + for _, entry := range entries { + if entry.GetString("templateRepository") != "" { + // Template deployment consumes and persists one-shot state. Keep that + // rare batch sequential so workers never write the shared config at + // the same time. + parallel = false + break + } + } + if parallel { // Individual spinners own one terminal row and cannot safely redraw the // same row from several goroutines. A synchronized writer keeps every // line intact and deliberately makes parallel progress use the spinner's @@ -1074,11 +1084,13 @@ func (c *pushContext) pushDeployable( } err = c.api.Call("PUT", resource.Path+"/"+url.PathEscape(id), - writeBody(entry, resource.WriteKeys, resource.OmitWhenEmpty, "", ""), nil) + pendingSafeBody(entry, writeBody( + entry, resource.WriteKeys, resource.OmitWhenEmpty, "", "")), nil) } else { err = c.api.Call("POST", resource.Path, - writeBody(entry, resource.WriteKeys, resource.OmitWhenEmpty, - resource.IDField, id), nil) + pendingSafeBody(entry, writeBody( + entry, resource.WriteKeys, resource.OmitWhenEmpty, + resource.IDField, id)), nil) } if err != nil { recordPushFailure(command, resource, name, err.Error(), summary) @@ -1135,6 +1147,24 @@ func recordPushFailure( summary.Failed = append(summary.Failed, failedDeployment{Name: name, Reason: reason}) } +// pendingSafeBody strips VCS fields while a new repository is still pending. +// A settings-only push must not connect the function before that repository +// exists. +func pendingSafeBody(entry, body *jsonx.Object) *jsonx.Object { + if !entry.GetBool("providerRepositoryPending") { + return body + } + for _, key := range []string{ + "installationId", "providerRepositoryId", "providerBranch", + "providerSilentMode", "providerRootDirectory", "providerBranches", + "providerPaths", + } { + body.Delete(key) + } + + return body +} + // writeBody builds a create or update body from the config entry. // // Only keys the config actually carries are sent. An absent key is omitted @@ -1407,7 +1437,8 @@ func (c *pushContext) createGitFunctionDeployment( body.Set("activate", activate) path := "/functions/" + functionID + "/deployments/vcs" - if entry.GetString("templateRepository") != "" { + template := entry.GetString("templateRepository") != "" + if template { path = "/functions/" + functionID + "/deployments/template" body.Set("repository", entry.GetString("templateRepository")) body.Set("owner", entry.GetString("templateOwner")) @@ -1419,24 +1450,40 @@ func (c *pushContext) createGitFunctionDeployment( body.Set("reference", entry.GetString("providerBranch")) } - deployment := jsonx.NewObject() - if err := c.api.Call("POST", path, body, deployment); err != nil { - return nil, err - } - - // Template coordinates are one-shot. Keeping them would merge the starter - // into the repository again on every explicit CLI deployment. - if entry.GetString("templateRepository") != "" { + // Consume template coordinates before the request. Once an HTTP request is + // attempted its outcome can be ambiguous, so leaving them consumed is the + // only way a retry cannot seed the starter twice. + if template { + saved := map[string]any{} for _, key := range []string{ "templateRepository", "templateOwner", "templateRootDirectory", "templateReference", "templateReferenceType", } { + if value, ok := entry.Get(key); ok { + saved[key] = value + } entry.Delete(key) } c.local.UpsertByID("functions", entry) if err := c.local.Write(); err != nil { - return nil, fmt.Errorf("deployment created but template state could not be saved: %w", err) + for key, value := range saved { + entry.Set(key, value) + } + c.local.UpsertByID("functions", entry) + + return nil, fmt.Errorf("template state could not be saved; nothing was deployed: %w", err) + } + } + + deployment := jsonx.NewObject() + if err := c.api.Call("POST", path, body, deployment); err != nil { + if template { + return nil, fmt.Errorf( + "%w; template state was consumed before the request -- check the function's deployments before retrying", + err) } + + return nil, err } return deployment, nil