Skip to content

Add FrontendVPCs support to NodeBalancers #857

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.

Sign up for GitHub

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

Open
komer3 wants to merge 2 commits into
base: main
Choose a base branch
from
+19 −12
Open

Add FrontendVPCs support to NodeBalancers #857

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
852faf5
Add `FrontendVPCs` support to NodeBalancer structs
komer3 Dec 1, 2025
ba8bc59
Merge branch 'main' into nb-frontend-vpc
komer3 Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions nodebalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ type NodeBalancer struct {
IPv4 *string `json:"ipv4"`
// This NodeBalancer's public IPv6 address.
IPv6 *string `json:"ipv6"`
// Frontend address type (e.g., "vpc")
Copy link

@AshleyDumaine AshleyDumaine Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is vpc the only supported type currently / are do we expect other types to be needed?

Copy link
Contributor Author

@komer3 komer3 Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, vpc is the only one right now. May others will be introduced in future

FrontendAddressType *string `json:"frontend_address_type,omitempty"`
Comment on lines +25 to +26
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment should specify what valid values are accepted for FrontendAddressType beyond the example 'vpc'. Consider documenting all possible values or using a constant/enum type to make valid options explicit.

Copilot uses AI. Check for mistakes.
// Frontend VPC subnet ID when using VPC addressing
FrontendVPCSubnetID *int `json:"frontend_vpc_subnet_id,omitempty"`
Copy link

@AshleyDumaine AshleyDumaine Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this PR is for adding frontendVPC support, but why don't we already have (Backend)VPC fields?

Copy link

@AshleyDumaine AshleyDumaine Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I didn't see this field used in your CCM PR, is this field necessary?

Copy link
Contributor Author

@komer3 komer3 Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These field a primarily used for returned response body!

// Throttle connections per second (0-20). Set to 0 (zero) to disable throttling.
ClientConnThrottle int `json:"client_conn_throttle"`

Expand All @@ -46,16 +50,18 @@ type NodeBalancerTransfer struct {
// The total transfer, in MB, used by this NodeBalancer this month.
Total *float64 `json:"total"`
// The total inbound transfer, in MB, used for this NodeBalancer this month.
Out *float64 `json:"out"`
// The total outbound transfer, in MB, used for this NodeBalancer this month.
In *float64 `json:"in"`
// The total outbound transfer, in MB, used for this NodeBalancer this month.
Out *float64 `json:"out"`
Comment on lines -49 to +55
Copy link

@AshleyDumaine AshleyDumaine Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

}

type NodeBalancerVPCOptions struct {
IPv4Range string `json:"ipv4_range,omitempty"`
IPv6Range string `json:"ipv6_range,omitempty"`
SubnetID int `json:"subnet_id"`
IPv4RangeAutoAssign bool `json:"ipv4_range_auto_assign,omitempty"`
IPv4Range string `json:"ipv4_range,omitempty"`
IPv6Range string `json:"ipv6_range,omitempty"`
SubnetID int `json:"subnet_id"`
// IPv4RangeAutoAssign is only used for backend VPC configuration.
// For frontend VPCs, this field is ignored.
Comment on lines +62 to +63
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment creates ambiguity since NodeBalancerVPCOptions is now used for both VPCs (line 80) and FrontendVPCs (line 81). The behavior of IPv4RangeAutoAssign should be clarified based on context of use rather than stating it's 'ignored' for frontend VPCs, as the same struct is used in both contexts.

Suggested change
// IPv4RangeAutoAssign is only used for backend VPC configuration.
// For frontend VPCs, this field is ignored.
// IPv4RangeAutoAssign is only used when NodeBalancerVPCOptions is provided as part of the VPCs field (backend VPC configuration) in NodeBalancerCreateOptions.
// When used in the FrontendVPCs field (frontend VPC configuration), this field is ignored.

Copilot uses AI. Check for mistakes.
IPv4RangeAutoAssign bool `json:"ipv4_range_auto_assign,omitempty"`
}

// NodeBalancerCreateOptions are the options permitted for CreateNodeBalancer
Expand All @@ -67,12 +73,13 @@ type NodeBalancerCreateOptions struct {
// NOTE: ClientUDPSessThrottle may not currently be available to all users.
ClientUDPSessThrottle *int `json:"client_udp_sess_throttle,omitempty"`

Configs []*NodeBalancerConfigCreateOptions `json:"configs,omitempty"`
Tags []string `json:"tags"`
FirewallID int `json:"firewall_id,omitempty"`
Type NodeBalancerPlanType `json:"type,omitempty"`
VPCs []NodeBalancerVPCOptions `json:"vpcs,omitempty"`
IPv4 *string `json:"ipv4,omitempty"`
Configs []*NodeBalancerConfigCreateOptions `json:"configs,omitempty"`
Tags []string `json:"tags"`
FirewallID int `json:"firewall_id,omitempty"`
Type NodeBalancerPlanType `json:"type,omitempty"`
VPCs []NodeBalancerVPCOptions `json:"vpcs,omitempty"`
Copy link

@AshleyDumaine AshleyDumaine Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on adding a new BackendVPCs field and marking the VPCs field as deprecated in favor of the BackendVPCs field?

FrontendVPCs []NodeBalancerVPCOptions `json:"frontend_vpcs,omitempty"`
IPv4 *string `json:"ipv4,omitempty"`
}

// NodeBalancerUpdateOptions are the options permitted for UpdateNodeBalancer
Expand Down