fix(cli): warn that a GCP key may exist when CreateKey fails ambiguously

CHANGELOG.md

@@ -13,6 +13,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- `configure gcp` key cleanup: the minted key is now deleted when the upload
+  panics, a second Ctrl-C aborts the cleanup, the local key file is removed
+  first, and a possibly orphaned key is reported when `CreateKey` fails
+  ambiguously (cancellation, timeout, 5xx or network error), with the `gcloud`
+  commands to list and delete it (#2117).
 - `--min-savings-pct` now rejects values below 0 or above 100 at validation.
   Previously a negative value was accepted as no filter and a value above 100
   silently dropped every recommendation (#2161).

cmd/configure_gcp.go

@@ -565,6 +565,19 @@ func (k *iamKeyProvisioner) CreateKey(ctx context.Context, saEmail string) (keyN
 	return key.Name, key.PrivateKeyData, nil
 }
 
+// createKeyError wraps a CreateKey failure. Only a 4xx googleapi error proves
+// the server rejected the request; anything else (cancellation, timeout, 5xx,
+// network error) may have followed a committed mint, so warn the operator.
+func createKeyError(saEmail string, err error) error {
+	var gerr *googleapi.Error
+	if errors.As(err, &gerr) && gerr.Code >= 400 && gerr.Code < 500 {
+		return fmt.Errorf("failed to create service account key: %w", err)
+	}
+	return fmt.Errorf("failed to create service account key: %w; the key may have been created before the request ended: "+
+		"list keys with `gcloud iam service-accounts keys list --iam-account=%s --managed-by=user` and delete any key created just now "+
+		"with `gcloud iam service-accounts keys delete <KEY_ID> --iam-account=%s`", err, saEmail, saEmail)
+}
+
 func (k *iamKeyProvisioner) DeleteKey(ctx context.Context, keyName string) error {
 	_, err := k.svc.Projects.ServiceAccounts.Keys.Delete(keyName).Context(ctx).Do()
 	return err
@@ -633,7 +646,7 @@ func writeServiceAccountKey(ctx context.Context, p gcpKeyProvisioner, saEmail, k
 
 	keyName, privateKeyData, err := p.CreateKey(ctx, saEmail)
 	if err != nil {
-		return "", fmt.Errorf("failed to create service account key: %w", err)
+		return "", createKeyError(saEmail, err)
 	}
 
 	// From here on, any failure must delete the newly minted remote key so it

cmd/configure_gcp_signal_test.go

@@ -305,6 +305,9 @@ func runGCPInterruptChild(dir string) error {
 	if !errors.Is(err, context.Canceled) {
 		return fmt.Errorf("expected cancellation from the real coordinator, got %w", err)
 	}
+	if os.Getenv("CUDLY_GCP_SIGNAL_PHASE") == "mint" && !strings.Contains(err.Error(), "may have been created") {
+		return fmt.Errorf("interrupted CreateKey must warn that a key may exist, got %w", err)
+	}
 	return nil
 }
 

cmd/configure_gcp_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1"
+	"google.golang.org/api/googleapi"
 )
 
 // --- addMemberToPolicyBinding -------------------------------------------------
@@ -219,19 +220,51 @@ func TestWriteServiceAccountKey_RollbackFailureSurfaced(t *testing.T) {
 // local file is left behind.
 func TestWriteServiceAccountKey_CreateFailureNoOrphan(t *testing.T) {
 	m := &mockGCPKeyProvisioner{
-		createErr: errors.New("iam 403"),
+		createErr: &googleapi.Error{Code: 403},
 	}
 	keyFile := filepath.Join(t.TempDir(), "cudly-gcp-key.json")
 
 	_, err := writeServiceAccountKey(context.Background(), m, "sa@proj.iam.gserviceaccount.com", keyFile)
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to create service account key")
+	assert.NotContains(t, err.Error(), "may have been created", "a 4xx rejection minted nothing")
 	assert.False(t, m.deleteCalled, "no remote key was minted, so DeleteKey must not be called")
 
 	_, statErr := os.Stat(keyFile)
 	assert.True(t, os.IsNotExist(statErr), "the reserved key file must be removed when minting fails")
 }
 
+// TestWriteServiceAccountKey_CreateFailureAmbiguous verifies that only a 4xx
+// googleapi error is a definite failure; every other outcome may have minted a key.
+func TestWriteServiceAccountKey_CreateFailureAmbiguous(t *testing.T) {
+	const sa = "sa@proj.iam.gserviceaccount.com"
+	cases := []struct {
+		name      string
+		err       error
+		ambiguous bool
+	}{
+		{"403 definite", &googleapi.Error{Code: 403}, false},
+		{"500 ambiguous", &googleapi.Error{Code: 500}, true},
+		{"canceled ambiguous", context.Canceled, true},
+		{"non-googleapi ambiguous", errors.New("connection reset"), true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			m := &mockGCPKeyProvisioner{createErr: tc.err}
+			_, err := writeServiceAccountKey(context.Background(), m, sa, filepath.Join(t.TempDir(), "k.json"))
+			require.Error(t, err)
+			require.ErrorIs(t, err, tc.err)
+			if !tc.ambiguous {
+				assert.NotContains(t, err.Error(), "may have been created")
+				return
+			}
+			assert.Contains(t, err.Error(), "may have been created")
+			assert.Contains(t, err.Error(), "gcloud iam service-accounts keys list --iam-account="+sa+" --managed-by=user")
+			assert.Contains(t, err.Error(), "gcloud iam service-accounts keys delete <KEY_ID> --iam-account="+sa)
+		})
+	}
+}
+
 // TestWriteServiceAccountKey_ReserveFailureNoMint verifies that if the
 // destination file already exists (exclusive-create fails), the remote key is
 // never minted, so there is nothing to orphan.