From 8c008b998f561798a579fae50f812f2065ed86f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Hampel?= Date: Wed, 10 Jun 2026 11:42:20 +0200 Subject: [PATCH] fix(credentials): resolve canonical from --name when --canonical is omitted (CLI-123) `credential update` was sending PUT to the collection endpoint /organizations/{org}/credentials when --canonical was not provided, because an empty canonical string caused the route to degenerate. The API returns 405 on PUT to the collection. Option B: when --canonical is omitted, list the org's credentials and match by --name / --path, mirroring the create --update lookup. If no match is found, a clear error is returned instead of a silent 405. Explicit --canonical still wins when provided. Adds an offline regression test that asserts PUT targets the item route /organizations/{org}/credentials/{canonical}, not the collection. --- cmd/cycloid/credentials/update.go | 14 +++ .../credentials/update_offline_test.go | 86 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 cmd/cycloid/credentials/update_offline_test.go diff --git a/cmd/cycloid/credentials/update.go b/cmd/cycloid/credentials/update.go index 91490593..3e84f0b8 100644 --- a/cmd/cycloid/credentials/update.go +++ b/cmd/cycloid/credentials/update.go @@ -238,6 +238,20 @@ func update(cmd *cobra.Command, args []string) error { return err } + // If --canonical was not provided, resolve it from --name/--path by listing + // the org's credentials — mirrors the create --update lookup (CLI-123). + if credential == "" { + creds, _, listErr := m.ListCredentials(org, credT) + if listErr != nil { + return fmt.Errorf("unable to resolve credential from --name %q: %w", name, listErr) + } + existing := findCredentialForUpdate(creds, credential, credentialPath, name) + if existing == nil || existing.Canonical == nil { + return fmt.Errorf("no credential found matching --name %q; pass --canonical to target it directly", name) + } + credential = *existing.Canonical + } + outCred, _, err := m.UpdateCredential(org, name, credT, rawCred, credentialPath, credential, description) return cyout.PrintWithOptions(cmd, outCred, err, "unable to update credential", printer.Options{}) } diff --git a/cmd/cycloid/credentials/update_offline_test.go b/cmd/cycloid/credentials/update_offline_test.go new file mode 100644 index 00000000..b2548337 --- /dev/null +++ b/cmd/cycloid/credentials/update_offline_test.go @@ -0,0 +1,86 @@ +package credentials_test + +// Regression test for CLI-123: `credential update` with --name only (no --canonical) was +// sending PUT to the collection endpoint /organizations/{org}/credentials → 405. +// After the fix, it resolves the canonical via ListCredentials and sends PUT to the item +// endpoint /organizations/{org}/credentials/{canonical}. + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/cycloidio/cycloid-cli/cmd/cycloid/credentials" +) + +func TestUpdateByNameResolvesCanonical_ItemRoute(t *testing.T) { + const ( + org = "test-org" + credName = "nm-testing-custom-secret" + credCanon = "nm-testing-custom-secret" + ) + + type req struct{ method, path string } + var captured []req + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured = append(captured, req{r.Method, r.URL.Path}) + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/credentials"): + // ListCredentials — return one credential matching the test name + _, _ = fmt.Fprintf(w, `{"data":[{"canonical":%q,"name":%q,"path":%q,"type":"custom"}]}`, + credCanon, credName, credCanon) + case r.Method == http.MethodPut: + // UpdateCredential — minimal valid response + _, _ = fmt.Fprintf(w, `{"data":{"canonical":%q,"name":%q,"path":%q,"type":"custom","description":""}}`, + credCanon, credName, credCanon) + default: + http.Error(w, fmt.Sprintf("unexpected %s %s", r.Method, r.URL.Path), http.StatusInternalServerError) + } + })) + defer srv.Close() + + // Inject test server URL, org, and token via global Viper / env (what NewAPI+GetOrg read). + viper.Set("api-url", srv.URL) + viper.Set("org", org) + t.Setenv("CY_API_KEY", "test-token") + t.Cleanup(func() { + viper.Set("api-url", "") + viper.Set("org", "") + }) + + updateCmd := credentials.NewUpdateCommand() + updateCmd.SetArgs([]string{"custom", "--name", credName, "--field", "tesnmsecret2=toto"}) + err := updateCmd.Execute() + require.NoError(t, err) + + // Assert ListCredentials was called for canonical resolution. + listPath := fmt.Sprintf("/organizations/%s/credentials", org) + var listSeen bool + for _, r := range captured { + if r.method == http.MethodGet && r.path == listPath { + listSeen = true + } + } + assert.True(t, listSeen, "expected GET %s (ListCredentials) to be called for name resolution", listPath) + + // Assert PUT targeted the ITEM route, not the collection — regression guard for CLI-123. + itemPath := fmt.Sprintf("/organizations/%s/credentials/%s", org, credCanon) + var putPath string + for _, r := range captured { + if r.method == http.MethodPut { + putPath = r.path + break + } + } + require.NotEmpty(t, putPath, "expected a PUT request to be sent") + assert.Equal(t, itemPath, putPath, + "PUT must target item route (CLI-123 regression: was hitting collection %q)", listPath) +}