diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d49dfec..36a84e0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,18 @@ on: - main jobs: + unit-test: + runs-on: ubuntu-latest + steps: + - name: Code checkout + uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: 1.24.9 + - name: Run unit tests + run: go test -short ./pkg/... + build-mac: runs-on: macos-latest steps: diff --git a/pkg/cmd/connection_common.go b/pkg/cmd/connection_common.go index b71fbf9..009bb05 100644 --- a/pkg/cmd/connection_common.go +++ b/pkg/cmd/connection_common.go @@ -191,7 +191,11 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { rule["interval"] = f.RuleRetryInterval } if f.RuleRetryResponseStatusCode != "" { - rule["response_status_codes"] = f.RuleRetryResponseStatusCode + codes := strings.Split(f.RuleRetryResponseStatusCode, ",") + for i := range codes { + codes[i] = strings.TrimSpace(codes[i]) + } + rule["response_status_codes"] = codes } rules = append(rules, rule) } diff --git a/pkg/cmd/connection_upsert.go b/pkg/cmd/connection_upsert.go index 95ad92e..840124d 100644 --- a/pkg/cmd/connection_upsert.go +++ b/pkg/cmd/connection_upsert.go @@ -94,7 +94,7 @@ func newConnectionUpsertCmd() *connectionUpsertCmd { cu.cmd.Flags().StringVar(&cu.destinationType, "destination-type", "", "Destination type (CLI, HTTP, MOCK)") cu.cmd.Flags().StringVar(&cu.destinationDescription, "destination-description", "", "Destination description") cu.cmd.Flags().StringVar(&cu.destinationURL, "destination-url", "", "URL for HTTP destinations") - cu.cmd.Flags().StringVar(&cu.destinationCliPath, "destination-cli-path", "/", "CLI path for CLI destinations (default: /)") + cu.cmd.Flags().StringVar(&cu.destinationCliPath, "destination-cli-path", "", "CLI path for CLI destinations (default: / for new connections)") // Use a string flag to allow explicit true/false values var pathForwardingDisabledStr string @@ -257,10 +257,10 @@ func (cu *connectionUpsertCmd) validateSourceFlags() error { return fmt.Errorf("cannot use --source-id with --source-name or --source-type") } - // If creating inline, require both name and type - if (cu.sourceName != "" || cu.sourceType != "") && (cu.sourceName == "" || cu.sourceType == "") { - return fmt.Errorf("both --source-name and --source-type are required for inline source creation") - } + // For upsert, we don't require both --source-name and --source-type. + // If the connection already exists, providing just --source-name is valid + // (the existing source type will be preserved). The API will reject + // incomplete data if this is actually a create. return nil } @@ -272,10 +272,10 @@ func (cu *connectionUpsertCmd) validateDestinationFlags() error { return fmt.Errorf("cannot use --destination-id with --destination-name or --destination-type") } - // If creating inline, require both name and type - if (cu.destinationName != "" || cu.destinationType != "") && (cu.destinationName == "" || cu.destinationType == "") { - return fmt.Errorf("both --destination-name and --destination-type are required for inline destination creation") - } + // For upsert, we don't require both --destination-name and --destination-type. + // If the connection already exists, providing just --destination-name is valid + // (the existing destination type will be preserved). The API will reject + // incomplete data if this is actually a create. return nil } @@ -304,7 +304,11 @@ func (cu *connectionUpsertCmd) runConnectionUpsertCmd(cmd *cobra.Command, args [ cu.DestinationRateLimit != 0 || cu.DestinationAuthMethod != "") && cu.destinationName == "" && cu.destinationType == "" && cu.destinationID == "" - needsExisting := cu.dryRun || (!cu.hasAnySourceFlag() && !cu.hasAnyDestinationFlag()) || hasSourceConfigOnly || hasDestinationConfigOnly + // Also need to fetch existing when name is provided without type (to fill in the type) + hasPartialSourceInline := (cu.sourceName != "" && cu.sourceType == "" && cu.sourceID == "") + hasPartialDestinationInline := (cu.destinationName != "" && cu.destinationType == "" && cu.destinationID == "") + + needsExisting := cu.dryRun || (!cu.hasAnySourceFlag() && !cu.hasAnyDestinationFlag()) || hasSourceConfigOnly || hasDestinationConfigOnly || hasPartialSourceInline || hasPartialDestinationInline var existing *hookdeck.Connection var isUpdate bool @@ -411,6 +415,10 @@ func (cu *connectionUpsertCmd) buildUpsertRequest(existing *hookdeck.Connection, if cu.sourceID != "" { req.SourceID = &cu.sourceID } else if cu.sourceName != "" || cu.sourceType != "" { + // For upsert updates, fill in missing source type from existing connection + if cu.sourceType == "" && isUpdate && existing != nil && existing.Source != nil { + cu.sourceType = existing.Source.Type + } sourceInput, err := cu.buildSourceInput() if err != nil { return nil, err @@ -442,6 +450,14 @@ func (cu *connectionUpsertCmd) buildUpsertRequest(existing *hookdeck.Connection, if cu.destinationID != "" { req.DestinationID = &cu.destinationID } else if cu.destinationName != "" || cu.destinationType != "" { + // For upsert updates, fill in missing destination type from existing connection + if cu.destinationType == "" && isUpdate && existing != nil && existing.Destination != nil { + cu.destinationType = existing.Destination.Type + } + // Default CLI path to "/" for new CLI destinations when not explicitly set + if strings.ToUpper(cu.destinationType) == "CLI" && cu.destinationCliPath == "" { + cu.destinationCliPath = "/" + } destinationInput, err := cu.buildDestinationInput() if err != nil { return nil, err @@ -469,10 +485,13 @@ func (cu *connectionUpsertCmd) buildUpsertRequest(existing *hookdeck.Connection, } } - // Also preserve source if not specified + // Preserve existing source/destination if not specified if req.SourceID == nil && req.Source == nil && isUpdate && existing != nil && existing.Source != nil { req.SourceID = &existing.Source.ID } + if req.DestinationID == nil && req.Destination == nil && isUpdate && existing != nil && existing.Destination != nil { + req.DestinationID = &existing.Destination.ID + } // Handle Rules rules, err := buildConnectionRules(&cu.connectionCreateCmd.connectionRuleFlags) diff --git a/pkg/cmd/connection_upsert_test.go b/pkg/cmd/connection_upsert_test.go new file mode 100644 index 0000000..5a6490f --- /dev/null +++ b/pkg/cmd/connection_upsert_test.go @@ -0,0 +1,239 @@ +package cmd + +import ( + "encoding/json" + "testing" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func strPtr(s string) *string { + return &s +} + +// TestBuildConnectionRulesRetryStatusCodesArray verifies that buildConnectionRules +// produces response_status_codes as a []string array, not a single string. +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 3. +func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) { + tests := []struct { + name string + flags connectionRuleFlags + wantCodes []string + wantCodeCount int + wantRuleCount int + }{ + { + name: "comma-separated status codes should produce array", + flags: connectionRuleFlags{ + RuleRetryStrategy: "linear", + RuleRetryCount: 3, + RuleRetryInterval: 5000, + RuleRetryResponseStatusCode: "500,502,503,504", + }, + wantCodes: []string{"500", "502", "503", "504"}, + wantCodeCount: 4, + wantRuleCount: 1, + }, + { + name: "single status code should produce single-element array", + flags: connectionRuleFlags{ + RuleRetryStrategy: "exponential", + RuleRetryResponseStatusCode: "500", + }, + wantCodes: []string{"500"}, + wantCodeCount: 1, + wantRuleCount: 1, + }, + { + name: "status codes with spaces should be trimmed", + flags: connectionRuleFlags{ + RuleRetryStrategy: "linear", + RuleRetryResponseStatusCode: "500, 502, 503", + }, + wantCodes: []string{"500", "502", "503"}, + wantCodeCount: 3, + wantRuleCount: 1, + }, + { + name: "no status codes should not include response_status_codes", + flags: connectionRuleFlags{ + RuleRetryStrategy: "linear", + RuleRetryCount: 3, + }, + wantCodes: nil, + wantCodeCount: 0, + wantRuleCount: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rules, err := buildConnectionRules(&tt.flags) + require.NoError(t, err, "buildConnectionRules should not error") + require.Len(t, rules, tt.wantRuleCount, "Expected %d rule(s)", tt.wantRuleCount) + + if tt.wantRuleCount == 0 { + return + } + + retryRule := rules[len(rules)-1] + assert.Equal(t, "retry", retryRule["type"], "Last rule should be retry") + + if tt.wantCodes == nil { + _, exists := retryRule["response_status_codes"] + assert.False(t, exists, "response_status_codes should not be present when not specified") + return + } + + statusCodes, ok := retryRule["response_status_codes"] + require.True(t, ok, "response_status_codes should be present") + + codesSlice, ok := statusCodes.([]string) + require.True(t, ok, "response_status_codes should be []string, got %T", statusCodes) + assert.Equal(t, tt.wantCodeCount, len(codesSlice)) + assert.Equal(t, tt.wantCodes, codesSlice) + + // Verify it serializes to a JSON array + jsonBytes, err := json.Marshal(retryRule) + require.NoError(t, err) + + var parsed map[string]interface{} + err = json.Unmarshal(jsonBytes, &parsed) + require.NoError(t, err) + + jsonCodes, ok := parsed["response_status_codes"].([]interface{}) + require.True(t, ok, "JSON response_status_codes should be an array, got %T", parsed["response_status_codes"]) + assert.Len(t, jsonCodes, tt.wantCodeCount) + }) + } +} + +// TestUpsertBuildRequestRulesOnlyPreservesDestinationByID verifies that when +// upserting with only rule flags, the request uses destination_id (not a full +// destination object that could include incomplete auth config). +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 1. +func TestUpsertBuildRequestRulesOnlyPreservesDestinationByID(t *testing.T) { + cu := &connectionUpsertCmd{ + connectionCreateCmd: &connectionCreateCmd{}, + } + cu.name = "test-conn" + // Only set rule flags, no source/destination flags + cu.RuleRetryStrategy = "linear" + cu.RuleRetryCount = 3 + + existing := &hookdeck.Connection{ + ID: "conn_123", + Name: strPtr("test-conn"), + Source: &hookdeck.Source{ + ID: "src_123", + Name: "test-source", + Type: "WEBHOOK", + }, + Destination: &hookdeck.Destination{ + ID: "dst_123", + Name: "test-dest", + Type: "HTTP", + Config: map[string]interface{}{ + "url": "https://api.example.com", + "auth_type": "AWS_SIGNATURE", + }, + }, + } + + req, err := cu.buildUpsertRequest(existing, true) + require.NoError(t, err) + + // Should reference existing destination by ID, not recreate it + assert.NotNil(t, req.DestinationID, "Should use DestinationID") + assert.Equal(t, "dst_123", *req.DestinationID) + assert.Nil(t, req.Destination, "Should NOT send full Destination object") + + // Source should also be preserved by ID + assert.NotNil(t, req.SourceID, "Should use SourceID") + assert.Equal(t, "src_123", *req.SourceID) + assert.Nil(t, req.Source, "Should NOT send full Source object") + + // Rules should be present + assert.NotEmpty(t, req.Rules) +} + +// TestUpsertHasAnyDestinationFlagIgnoresDefault verifies that hasAnyDestinationFlag +// returns false when no destination flags are explicitly set (the cli-path default +// of "/" was previously causing this to always return true). +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 1. +func TestUpsertHasAnyDestinationFlagIgnoresDefault(t *testing.T) { + cu := &connectionUpsertCmd{ + connectionCreateCmd: &connectionCreateCmd{}, + } + // No destination flags set at all (cli-path default is "" for upsert) + + result := cu.hasAnyDestinationFlag() + assert.False(t, result, "hasAnyDestinationFlag should be false with no destination flags set") +} + +// TestUpsertValidateSourceFlagsAllowsNameOnly verifies that validateSourceFlags +// allows --source-name without --source-type for the upsert command. +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 2. +func TestUpsertValidateSourceFlagsAllowsNameOnly(t *testing.T) { + cu := &connectionUpsertCmd{ + connectionCreateCmd: &connectionCreateCmd{}, + } + cu.sourceName = "my-source" + // sourceType intentionally empty + + err := cu.validateSourceFlags() + assert.NoError(t, err, "validateSourceFlags should allow --source-name alone for upsert") +} + +// TestUpsertValidateDestinationFlagsAllowsNameOnly verifies the same relaxation +// for destination flags. +func TestUpsertValidateDestinationFlagsAllowsNameOnly(t *testing.T) { + cu := &connectionUpsertCmd{ + connectionCreateCmd: &connectionCreateCmd{}, + } + cu.destinationName = "my-dest" + // destinationType intentionally empty + + err := cu.validateDestinationFlags() + assert.NoError(t, err, "validateDestinationFlags should allow --destination-name alone for upsert") +} + +// TestUpsertBuildRequestFillsSourceTypeFromExisting verifies that when +// --source-name is provided without --source-type during an update, +// the existing source type is used. +func TestUpsertBuildRequestFillsSourceTypeFromExisting(t *testing.T) { + cu := &connectionUpsertCmd{ + connectionCreateCmd: &connectionCreateCmd{}, + } + cu.name = "test-conn" + cu.sourceName = "new-source-name" + // sourceType intentionally empty - should be filled from existing + + existing := &hookdeck.Connection{ + ID: "conn_123", + Name: strPtr("test-conn"), + Source: &hookdeck.Source{ + ID: "src_123", + Name: "old-source-name", + Type: "WEBHOOK", + }, + Destination: &hookdeck.Destination{ + ID: "dst_123", + Name: "test-dest", + Type: "HTTP", + Config: map[string]interface{}{ + "url": "https://api.example.com", + }, + }, + } + + req, err := cu.buildUpsertRequest(existing, true) + require.NoError(t, err) + + // Source should be an inline input (not just ID) since name was changed + require.NotNil(t, req.Source, "Should have Source input for name change") + assert.Equal(t, "new-source-name", req.Source.Name) + assert.Equal(t, "WEBHOOK", req.Source.Type, "Should fill type from existing source") +} diff --git a/pkg/cmd/source_create_update_test.go b/pkg/cmd/source_create_update_test.go index 61059ab..9a081e2 100644 --- a/pkg/cmd/source_create_update_test.go +++ b/pkg/cmd/source_create_update_test.go @@ -1,21 +1,31 @@ package cmd import ( - "strings" "testing" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -// TestSourceCreateRequiresName asserts that create without --name fails (Cobra required-flag validation). -func TestSourceCreateRequiresName(t *testing.T) { - rootCmd.SetArgs([]string{"gateway", "source", "create", "--type", "WEBHOOK"}) - err := rootCmd.Execute() - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "name") || strings.Contains(err.Error(), "required"), - "error should mention name or required, got: %s", err.Error()) +// TestSourceCreateRequiresNameAndType verifies that the source create command +// marks --name and --type as required flags via cobra's MarkFlagRequired. +func TestSourceCreateRequiresNameAndType(t *testing.T) { + sc := newSourceCreateCmd() + + nameFlag := sc.cmd.Flags().Lookup("name") + assert.NotNil(t, nameFlag, "--name flag should exist") + + typeFlag := sc.cmd.Flags().Lookup("type") + assert.NotNil(t, typeFlag, "--type flag should exist") + + // Cobra marks required flags with the "required" annotation + nameAnnotations := nameFlag.Annotations + assert.Contains(t, nameAnnotations, "cobra_annotation_bash_completion_one_required_flag", + "--name should be marked as required") + + typeAnnotations := typeFlag.Annotations + assert.Contains(t, typeAnnotations, "cobra_annotation_bash_completion_one_required_flag", + "--type should be marked as required") } // TestSourceUpdateRequestEmpty asserts the "no updates specified" logic for update. diff --git a/test/acceptance/connection_test.go b/test/acceptance/connection_test.go index 6b3ddf5..7ab68e3 100644 --- a/test/acceptance/connection_test.go +++ b/test/acceptance/connection_test.go @@ -2195,6 +2195,60 @@ func TestConnectionCreateOutputStructure(t *testing.T) { t.Logf("Successfully verified connection create output structure") } +// TestConnectionCreateRetryResponseStatusCodes verifies that creating a connection +// with --rule-retry-response-status-codes sends the codes as an array to the API. +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 3. +func TestConnectionCreateRetryResponseStatusCodes(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-create-statuscodes-" + timestamp + sourceName := "test-src-sc-" + timestamp + destName := "test-dst-sc-" + timestamp + + var conn Connection + err := cli.RunJSON(&conn, + "gateway", "connection", "create", + "--name", connName, + "--source-name", sourceName, + "--source-type", "WEBHOOK", + "--destination-name", destName, + "--destination-type", "HTTP", + "--destination-url", "https://api.example.com/webhook", + "--rule-retry-strategy", "linear", + "--rule-retry-count", "3", + "--rule-retry-interval", "5000", + "--rule-retry-response-status-codes", "500,502,503,504", + ) + require.NoError(t, err, "Should create connection with retry response status codes") + require.NotEmpty(t, conn.ID, "Connection should have an ID") + + t.Cleanup(func() { + deleteConnection(t, cli, conn.ID) + }) + + require.NotEmpty(t, conn.Rules, "Connection should have rules") + + foundRetry := false + for _, rule := range conn.Rules { + if rule["type"] == "retry" { + foundRetry = true + + statusCodes, ok := rule["response_status_codes"].([]interface{}) + require.True(t, ok, "response_status_codes should be an array, got: %T (%v)", rule["response_status_codes"], rule["response_status_codes"]) + assert.Len(t, statusCodes, 4, "Should have 4 status codes") + break + } + } + assert.True(t, foundRetry, "Should have a retry rule") + + t.Logf("Successfully created connection %s with retry status codes as array", conn.ID) +} + // TestConnectionWithDestinationPathForwarding tests path_forwarding_disabled and http_method fields func TestConnectionWithDestinationPathForwarding(t *testing.T) { if testing.Short() { diff --git a/test/acceptance/connection_update_test.go b/test/acceptance/connection_update_test.go index 9d97418..aebeb33 100644 --- a/test/acceptance/connection_update_test.go +++ b/test/acceptance/connection_update_test.go @@ -316,6 +316,52 @@ func TestConnectionUpdateDelayRule(t *testing.T) { t.Logf("Connection update with delay rule verified: %s", connID) } +// TestConnectionUpdateRetryResponseStatusCodes verifies that +// --rule-retry-response-status-codes is sent as an array to the API. +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 3. +func TestConnectionUpdateRetryResponseStatusCodes(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + + connID := createTestConnection(t, cli) + require.NotEmpty(t, connID, "Connection ID should not be empty") + + t.Cleanup(func() { + deleteConnection(t, cli, connID) + }) + + // Update with retry rule including response status codes + var updated Connection + err := cli.RunJSON(&updated, + "gateway", "connection", "update", connID, + "--rule-retry-strategy", "linear", + "--rule-retry-count", "3", + "--rule-retry-interval", "5000", + "--rule-retry-response-status-codes", "500,502,503", + ) + require.NoError(t, err, "Should update connection with retry response status codes") + require.NotEmpty(t, updated.Rules, "Connection should have rules") + + // Find the retry rule and verify status codes are an array + foundRetry := false + for _, rule := range updated.Rules { + if rule["type"] == "retry" { + foundRetry = true + + statusCodes, ok := rule["response_status_codes"].([]interface{}) + require.True(t, ok, "response_status_codes should be an array, got: %T (%v)", rule["response_status_codes"], rule["response_status_codes"]) + assert.Len(t, statusCodes, 3, "Should have 3 status codes") + break + } + } + assert.True(t, foundRetry, "Should have a retry rule") + + t.Logf("Successfully verified retry status codes are sent as array via update: %s", connID) +} + // TestConnectionUpdateWithRulesJSON verifies update with --rules JSON string func TestConnectionUpdateWithRulesJSON(t *testing.T) { if testing.Short() { diff --git a/test/acceptance/connection_upsert_test.go b/test/acceptance/connection_upsert_test.go index 3a68c2e..fc6702f 100644 --- a/test/acceptance/connection_upsert_test.go +++ b/test/acceptance/connection_upsert_test.go @@ -1,6 +1,7 @@ package acceptance import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -241,4 +242,203 @@ func TestConnectionUpsertPartialUpdates(t *testing.T) { t.Logf("Successfully updated connection %s source config", connID) }) + + // Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 1: + // Upserting with only rule flags on a connection with destination auth should NOT + // send auth_type without credentials. + t.Run("UpsertRulesOnlyPreservesDestinationAuth", func(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-upsert-rules-auth-" + timestamp + sourceName := "test-upsert-src-ra-" + timestamp + destName := "test-upsert-dst-ra-" + timestamp + + // Create a connection WITH destination authentication (bearer token) + var createResp map[string]interface{} + err := cli.RunJSON(&createResp, + "gateway", "connection", "create", + "--name", connName, + "--source-type", "WEBHOOK", + "--source-name", sourceName, + "--destination-type", "HTTP", + "--destination-name", destName, + "--destination-url", "https://api.example.com/webhook", + "--destination-auth-method", "bearer", + "--destination-bearer-token", "test_secret_token_123", + ) + require.NoError(t, err, "Should create connection with bearer auth") + + connID, ok := createResp["id"].(string) + require.True(t, ok && connID != "", "Expected connection ID") + + t.Cleanup(func() { + deleteConnection(t, cli, connID) + }) + + // Verify auth was set + dest, ok := createResp["destination"].(map[string]interface{}) + require.True(t, ok, "Expected destination object") + destConfig, ok := dest["config"].(map[string]interface{}) + require.True(t, ok, "Expected destination config") + assert.Equal(t, "BEARER_TOKEN", destConfig["auth_type"], "Auth type should be BEARER_TOKEN after creation") + + // Upsert with ONLY rule flags (no source/destination flags) + var upsertResp map[string]interface{} + err = cli.RunJSON(&upsertResp, + "gateway", "connection", "upsert", connName, + "--rule-retry-strategy", "linear", + "--rule-retry-count", "3", + "--rule-retry-interval", "5000", + ) + require.NoError(t, err, "Should upsert with only rule flags without auth_type error") + + // Verify rules were applied + rules, ok := upsertResp["rules"].([]interface{}) + require.True(t, ok, "Expected rules array") + + foundRetry := false + for _, r := range rules { + rule, ok := r.(map[string]interface{}) + if ok && rule["type"] == "retry" { + foundRetry = true + assert.Equal(t, "linear", rule["strategy"]) + break + } + } + assert.True(t, foundRetry, "Should have a retry rule") + + // Verify auth was preserved + upsertDest, ok := upsertResp["destination"].(map[string]interface{}) + require.True(t, ok, "Expected destination in upsert response") + upsertDestConfig, ok := upsertDest["config"].(map[string]interface{}) + require.True(t, ok, "Expected destination config in upsert response") + assert.Equal(t, "BEARER_TOKEN", upsertDestConfig["auth_type"], "Auth type should still be BEARER_TOKEN") + + t.Logf("Successfully upserted connection %s with only rule flags, auth preserved", connID) + }) + + // Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 3: + // --rule-retry-response-status-codes must be sent as an array, not a string. + t.Run("UpsertRetryResponseStatusCodesAsArray", func(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-upsert-statuscodes-" + timestamp + sourceName := "test-upsert-src-sc-" + timestamp + destName := "test-upsert-dst-sc-" + timestamp + + // Create a connection with full source/dest so the upsert provides all required fields + var upsertResp map[string]interface{} + err := cli.RunJSON(&upsertResp, + "gateway", "connection", "upsert", connName, + "--source-name", sourceName, + "--source-type", "WEBHOOK", + "--destination-name", destName, + "--destination-type", "HTTP", + "--destination-url", "https://api.example.com/webhook", + "--rule-retry-strategy", "linear", + "--rule-retry-count", "3", + "--rule-retry-interval", "5000", + "--rule-retry-response-status-codes", "500,502,503,504", + ) + require.NoError(t, err, "Should upsert with retry response status codes as array") + + connID, _ := upsertResp["id"].(string) + t.Cleanup(func() { + if connID != "" { + deleteConnection(t, cli, connID) + } + }) + + // Verify the retry rule has status codes as an array + rules, ok := upsertResp["rules"].([]interface{}) + require.True(t, ok, "Expected rules array") + + foundRetry := false + for _, r := range rules { + rule, ok := r.(map[string]interface{}) + if ok && rule["type"] == "retry" { + foundRetry = true + + statusCodes, ok := rule["response_status_codes"].([]interface{}) + require.True(t, ok, "response_status_codes should be array, got: %T (%v)", rule["response_status_codes"], rule["response_status_codes"]) + assert.Len(t, statusCodes, 4, "Should have 4 status codes") + + codes := make([]string, len(statusCodes)) + for i, c := range statusCodes { + codes[i] = strings.TrimSpace(c.(string)) + } + assert.Contains(t, codes, "500") + assert.Contains(t, codes, "502") + assert.Contains(t, codes, "503") + assert.Contains(t, codes, "504") + break + } + } + assert.True(t, foundRetry, "Should have a retry rule") + + t.Logf("Successfully verified retry status codes sent as array") + }) + + // Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 2: + // Upserting with --source-name alone (without --source-type) should work for + // existing connections (the existing type is preserved). + t.Run("UpsertSourceNameWithoutTypeOnExistingConnection", func(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + + connName := "test-upsert-srconly-" + timestamp + sourceName := "test-upsert-src-so-" + timestamp + destName := "test-upsert-dst-so-" + timestamp + newSourceName := "test-upsert-newsrc-" + timestamp + + // Create a connection first + var createResp map[string]interface{} + err := cli.RunJSON(&createResp, + "gateway", "connection", "create", + "--name", connName, + "--source-type", "WEBHOOK", + "--source-name", sourceName, + "--destination-type", "HTTP", + "--destination-name", destName, + "--destination-url", "https://api.example.com/webhook", + ) + require.NoError(t, err, "Should create connection") + + connID, ok := createResp["id"].(string) + require.True(t, ok && connID != "", "Expected connection ID") + + t.Cleanup(func() { + deleteConnection(t, cli, connID) + }) + + // Upsert with --source-name only (no --source-type) + // Previously this failed with "both --source-name and --source-type are required" + var upsertResp map[string]interface{} + err = cli.RunJSON(&upsertResp, + "gateway", "connection", "upsert", connName, + "--source-name", newSourceName, + ) + require.NoError(t, err, "Should upsert with --source-name only on existing connection") + + // Verify the source was updated + upsertSource, ok := upsertResp["source"].(map[string]interface{}) + require.True(t, ok, "Expected source in upsert response") + assert.Equal(t, newSourceName, upsertSource["name"], "Source name should be updated") + + t.Logf("Successfully upserted connection %s with source-name only", connID) + }) }