Migrate byok to aws-sdk-go-v2, fix on-prem login URL trailing path, docs terminology - #3426
Open
David Adams (davidadas) wants to merge 3 commits into
Open
Migrate byok to aws-sdk-go-v2, fix on-prem login URL trailing path, docs terminology#3426David Adams (davidadas) wants to merge 3 commits into
David Adams (davidadas) wants to merge 3 commits into
Conversation
…form" Aligns the generated docs tab name with the product's current name.
v1 is in maintenance mode; v2's arn package is drop-in compatible (verified against 8 real ARN strings, including edge cases, with zero behavioral differences). go mod tidy then removed v1 entirely, since byok was its only remaining caller.
A URL with a trailing path (e.g. an MDS URL copied from a browser address bar) was baked verbatim into the stored base URL, silently breaking every subsequent API request built on top of it. The CCloud branch was left untouched to avoid changing existing accepted behavior there (e.g. trailing-slash URLs).
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
This PR bundles three related maintenance/bugfix updates across login URL validation, BYOK AWS dependency usage, and docs generation labeling.
Changes:
- Migrates
internal/byokARN parsing fromaws-sdk-gov1 toaws-sdk-go-v2and removes the v1 dependency fromgo.mod/go.sum. - Fixes Confluent Platform
confluent login --urlvalidation to discard any trailing path afterscheme://host:portso the stored base URL doesn’t include/login(or similar). - Renames the generated docs on-prem tab label from “On-Premises” to “Confluent Platform”.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/login/command.go | Strips trailing path from on-prem login URLs while keeping Confluent Cloud URL behavior unchanged. |
| internal/login/command_test.go | Adds test coverage for trailing-path stripping and a related invalid URL case. |
| internal/byok/command_create.go | Switches ARN parsing import to AWS SDK v2. |
| internal/byok/command_create_test.go | Adds a unit test for AWS KMS key ARN detection. |
| go.mod | Replaces AWS SDK v1 dependency with AWS SDK v2 core module as a direct requirement. |
| go.sum | Removes AWS SDK v1 checksums after the migration. |
| cmd/docs/main.go | Updates docs tab/context labeling to “Confluent Platform”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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
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.
Release Notes
Bug Fixes
confluent login --urlfor Confluent Platform silently keeping a trailing path (e.g. copied from a browser address bar) baked into the stored base URL, breaking subsequent API requests.Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
Three independent changes bundled together:
byokfromaws-sdk-gov1 to v2 (APIE-472, Confluent Cloud):aws-sdk-gov1 is in maintenance mode;byokwas its only remaining caller in this repo. Verified the v1 and v2arnpackages are drop-in compatible by running both side-by-side against 8 real ARN strings, including edge cases, with zero behavioral differences.go mod tidythen removed v1 entirely.login --url(APIE-1348, Confluent Platform): a URL with a trailing path (e.g. an MDS URL copied from a browser address bar) was baked verbatim into the stored base URL, silently breaking every subsequent API request built on top of it. The Confluent Cloud branch of the same validation function was left untouched to avoid changing existing accepted behavior there (e.g. trailing-slash URLs).Blast Radius
byokAWS SDK migration is an internal dependency swap with verified drop-in-compatible behavior; no customer-facing change expected.--urlcontaining a trailing path now get a correctly stripped base URL instead of silently broken subsequent requests; no impact to URLs without a trailing path.References
Test & Review
TestIsAWSKeyunit test and 2 newTestValidateUrlcases covering the trailing-path fix; full targeted test suite (internal/login,internal/byok) run locally, all passing.