-
Notifications
You must be signed in to change notification settings - Fork 20
Egress PNI support for CLI #3258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds CLI support for configuring and displaying egress CIDR routes on AWS Access Point Private Network Interface resources, including wiring through to the networking-access-point SDK model and updating integration test fixtures accordingly.
Changes:
- Add
--routessupport tonetwork access-point private-network-interface create|updateand plumbEgressRoutesinto request payloads. - Include “Egress Routes” in human and JSON outputs for describe/list/update flows.
- Update test server + golden fixtures to reflect the new field and add new integration test cases.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/network/command_access_point_private_network_interface_create.go |
Adds --routes flag and sets EgressRoutes in AWS PNI create payload. |
internal/network/command_access_point_private_network_interface_update.go |
Adds --routes flag and sets EgressRoutes in AWS PNI update payload. |
internal/network/command_access_point_private_network_interface_list.go |
Adds EgressRoutes to list output model population. |
internal/network/command_access_point_private_network_interface.go |
Extends output struct + printing to include EgressRoutes. |
test/test-server/networking_handlers.go |
Test server returns default EgressRoutes for AWS PNI and supports updating routes. |
test/network_test.go |
Adds create-with-routes and update-routes integration test cases. |
test/fixtures/output/network/access-point/private-network-interface/create-help.golden |
Updates help text to document --routes. |
test/fixtures/output/network/access-point/private-network-interface/create-with-routes.golden |
New golden output for create including routes. |
test/fixtures/output/network/access-point/private-network-interface/update-help.golden |
Updates help text to document --routes. |
test/fixtures/output/network/access-point/private-network-interface/update-autocomplete.golden |
Updates autocomplete output for new flag. |
test/fixtures/output/network/access-point/private-network-interface/update.golden |
Adds Egress Routes row to update output. |
test/fixtures/output/network/access-point/private-network-interface/update-network-interfaces.golden |
Adds Egress Routes row to update output when updating ENIs. |
test/fixtures/output/network/access-point/private-network-interface/update-routes.golden |
New golden output for update routes case. |
test/fixtures/output/network/access-point/private-network-interface/list.golden |
Adds Egress Routes column to list table output. |
test/fixtures/output/network/access-point/private-network-interface/list-json.golden |
Adds egress_routes field to list JSON output. |
test/fixtures/output/network/access-point/private-network-interface/describe.golden |
Adds Egress Routes row to describe output. |
test/fixtures/output/network/access-point/private-network-interface/describe-json.golden |
Adds egress_routes field to describe JSON output. |
go.mod |
Adds a replace to redirect networking-access-point to an internal module version. |
go.sum |
Updates dependency checksums for the new/replaced SDK module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (s *CLITestSuite) TestNetworkAccessPointPrivateNetworkInterfaceCreate() { | ||
| tests := []CLITest{ | ||
| {args: "network access-point private-network-interface create --cloud aws --gateway gw-123456 --network-interfaces eni-00000000000000000,eni-00000000000000001 --account 000000000000", fixture: "network/access-point/private-network-interface/create.golden"}, | ||
| {args: "network access-point private-network-interface create --cloud aws --gateway gw-123456 --network-interfaces eni-00000000000000000,eni-00000000000000001 --account 000000000000 --routes 172.31.0.0/16,192.168.1.0/24", fixture: "network/access-point/private-network-interface/create-with-routes.golden"}, | ||
| } |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description still contains placeholder Release Notes entries and an unchecked checklist. Please complete the Release Notes (and remove placeholders) and update the checklist/testing sections before merging so reviewers/users can understand the customer-facing impact and verification status.
| if err != nil { | ||
| return err | ||
| } | ||
| if len(networkInterfaces) > 0 { | ||
|
|
||
| routes, err := cmd.Flags().GetStringSlice("routes") |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This update request always includes Spec.DisplayName (set earlier from the --name flag defaulting to ""), so running update with only --network-interfaces or --routes may unintentionally clear the access point display name. Consider only setting DisplayName in the payload when cmd.Flags().Changed("name") is true (and otherwise leaving it unset).
| if networkInterfaces := body.Spec.GetConfig().NetworkingV1AwsPrivateNetworkInterface.GetNetworkInterfaces(); len(networkInterfaces) > 0 { | ||
| accessPoint.Spec.Config.NetworkingV1AwsPrivateNetworkInterface.SetNetworkInterfaces(body.Spec.GetConfig().NetworkingV1AwsPrivateNetworkInterface.GetNetworkInterfaces()) | ||
| } | ||
| if routes := body.Spec.GetConfig().NetworkingV1AwsPrivateNetworkInterface.GetEgressRoutes(); len(routes) > 0 { | ||
| accessPoint.Spec.Config.NetworkingV1AwsPrivateNetworkInterface.SetEgressRoutes(body.Spec.GetConfig().NetworkingV1AwsPrivateNetworkInterface.GetEgressRoutes()) |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this handler, the response access point’s display name is updated unconditionally later via accessPoint.Spec.SetDisplayName(body.Spec.GetDisplayName()). For patch-style updates where display_name is omitted, GetDisplayName() typically returns "", so this can unintentionally wipe the existing name. Consider guarding the name update with a nil check on the incoming DisplayName pointer (consistent with other update handlers in this file).
| sigs.k8s.io/yaml v1.3.0 // indirect | ||
| ) | ||
|
|
||
| replace github.com/confluentinc/ccloud-sdk-go-v2/networking-access-point => github.com/confluentinc/ccloud-sdk-go-v2-internal/networking-access-point v0.11.0 |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replace pins github.com/confluentinc/ccloud-sdk-go-v2/networking-access-point to the ccloud-sdk-go-v2-internal module. Since go.mod still requires networking-access-point v0.5.0, this creates a confusing version mismatch (the build will use the replacement even though the required version is older). Consider bumping the required version to match the replacement, and confirm whether introducing a dependency on the -internal module is intended for all consumers/build environments.
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # go.sum
Release Notes
Breaking Changes
New Features
--routesflag tonetwork access-point private-network-interface createandupdatecommands. Display egress routes indescribeandlistoutput.Bug Fixes
Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
Adds --routes flag (comma-separated CIDRs) to PNI access point create/update commands, and displays egress_routes in describe/list output. Uses internal SDK v0.11.0 for the egress_routes field. TODO: Replace with public SDK once released.
Blast Radius
Low. Additive optional flag, no existing behavior changed.
References
https://confluentinc.atlassian.net/browse/APIE-778
Test & Review
Bug bash doc for CLI: https://confluentinc.atlassian.net/wiki/spaces/~71202022dd5651f4f94e99a18f5bef3de17c2d/pages/5155718005/Egress+PNI+CLI+Bug+Bash+Manual+Tests+Overview