Fix connection upsert bugs with rules, auth, and partial updates#221
Merged
Fix connection upsert bugs with rules, auth, and partial updates#221
Conversation
…estination auth Fixes three bugs reported in #209: 1. Auth type sent without credentials: The `destination-cli-path` flag defaulted to "/" in the upsert command, making `hasAnyDestinationFlag()` always return true. This caused `buildDestinationInputForUpdate` to copy the entire existing destination config (including auth_type) even when no destination flags were provided, sending auth_type without credentials. Fix: Change the default to "" for upsert and add a destination ID preservation fallback. 2. Source name validation issue: `validateSourceFlags()` required both --source-name and --source-type even during upsert updates. For updates, providing just --source-name should be valid since the existing type can be preserved. Fix: Relax validation for upsert and fill in missing type from existing connection. 3. Status code format problem: `buildConnectionRules` sent `response_status_codes` as a single comma-separated string instead of parsing it into a string array, causing API rejection. Fix: Split the comma-separated string into a []string array. Note: Bug 3 fix affects all commands using buildConnectionRules (create, update, upsert) since the function is shared. https://claude.ai/code/session_01D8k5gyzeqzjZUanVrwv5dL
…ames Move acceptance tests from separate connection_upsert_bugs_test.go into the existing connection_upsert_test.go (as subtests of TestConnectionUpsertPartialUpdates) and connection_update_test.go. Rename unit tests from connection_upsert_bugs_test.go to connection_upsert_test.go with descriptive test function names. https://claude.ai/code/session_01D8k5gyzeqzjZUanVrwv5dL
- Add unit-test job to test.yml that runs `go test -short ./pkg/...` on every PR. Previously only GoReleaser builds ran, so unit test failures were never caught. - Fix TestSourceCreateRequiresName: the old test executed the real root command which hit ValidateAPIKey() before cobra's required-flag check, causing a misleading "not authenticated" error. Replaced with a direct check that --name and --type have the required-flag annotation. - Add TestConnectionCreateRetryResponseStatusCodes acceptance test for Bug 3 coverage on the create command path (update and upsert were already covered). https://claude.ai/code/session_01D8k5gyzeqzjZUanVrwv5dL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes three critical bugs in the connection upsert command that prevented proper handling of retry rules, destination authentication, and partial source/destination updates.
fixes #209
Key Changes
Bug 1: Preserve destination/source by ID when upserting rules only
buildUpsertRequest()to reference existing source/destination by ID instead of sending full objects when only rule flags are providedBug 2: Allow partial source/destination updates without type
validateSourceFlags()andvalidateDestinationFlags()to allow--source-nameor--destination-namealone on existing connectionsdestination-cli-pathfrom"/"to""to avoid false positives in flag detectionBug 3: Send retry response status codes as array, not string
buildConnectionRules()to split comma-separated status codes and send them as a[]stringarray instead of a single string"500, 502, 503"Testing
pkg/cmd/connection_upsert_test.gocovering all three bugstest/acceptance/connection_upsert_test.gofor regression testingtest/acceptance/connection_test.goandtest/acceptance/connection_update_test.goto verify status codes are sent as arraysImplementation Details
https://claude.ai/code/session_01D8k5gyzeqzjZUanVrwv5dL