Skip to content

Commit a90fbde

Browse files
author
Rahul Sharma
committed
address review comments
1 parent 395b9a0 commit a90fbde

File tree

6 files changed

+67
-50
lines changed

6 files changed

+67
-50
lines changed

‎api/v1alpha2/linodevpc_types.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,13 @@ type LinodeVPCSpec struct {
6464
}
6565

6666
// VPCCreateOptionsIPv6 defines the options for creating an IPv6 range in a VPC.
67-
// Its copied from linodego.VPCCreateOptionsIPv6 and should be kept in sync.
67+
// It's copied from linodego.VPCCreateOptionsIPv6 and should be kept in sync.
6868
// Values supported by the linode API should be used here.
6969
// See https://techdocs.akamai.com/linode-api/reference/post-vpc for more details.
7070
type VPCCreateOptionsIPv6 struct {
71-
Range *string `json:"range,omitempty"`
71+
// Range is the IPv6 prefix for the VPC.
72+
Range *string `json:"range,omitempty"`
73+
// IPv6 inventory from which the VPC prefix should be allocated.
7274
AllocationClass *string `json:"allocation_class,omitempty"`
7375
}
7476

‎config/crd/bases/infrastructure.cluster.x-k8s.io_linodevpcs.yaml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,16 @@ spec:
9090
items:
9191
description: |-
9292
VPCCreateOptionsIPv6 defines the options for creating an IPv6 range in a VPC.
93-
Its copied from linodego.VPCCreateOptionsIPv6 and should be kept in sync.
93+
It's copied from linodego.VPCCreateOptionsIPv6 and should be kept in sync.
9494
Values supported by the linode API should be used here.
9595
See https://techdocs.akamai.com/linode-api/reference/post-vpc for more details.
9696
properties:
9797
allocation_class:
98+
description: IPv6 inventory from which the VPC prefix should
99+
be allocated.
98100
type: string
99101
range:
102+
description: Range is the IPv6 prefix for the VPC.
100103
type: string
101104
type: object
102105
type: array

‎docs/src/reference/out.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,7 +1194,7 @@ _Appears in:_
11941194

11951195

11961196
VPCCreateOptionsIPv6 defines the options for creating an IPv6 range in a VPC.
1197-
Its copied from linodego.VPCCreateOptionsIPv6 and should be kept in sync.
1197+
It's copied from linodego.VPCCreateOptionsIPv6 and should be kept in sync.
11981198
Values supported by the linode API should be used here.
11991199
See https://techdocs.akamai.com/linode-api/reference/post-vpc for more details.
12001200

@@ -1205,8 +1205,8 @@ _Appears in:_
12051205

12061206
| Field | Description | Default | Validation |
12071207
| --- | --- | --- | --- |
1208-
| `range` _string_ | | | |
1209-
| `allocation_class` _string_ | | | |
1208+
| `range` _string_ | Range is the IPv6 prefix for the VPC. | | |
1209+
| `allocation_class` _string_ | IPv6 inventory from which the VPC prefix should be allocated. | | |
12101210

12111211

12121212
#### VPCIPv4

‎internal/controller/linodemachine_controller_helpers.go‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ import (
5757
const (
5858
maxBootstrapDataBytesCloudInit = 16384
5959
vlanIPFormat = "%s/11"
60-
ipv6Range = "/64" // Default IPv6 range for VPC interfaces
60+
defaultNodeIPv6CIDRRange = "/64" // Default IPv6 range for VPC interfaces
6161
)
6262

6363
var (
@@ -463,12 +463,12 @@ func getVPCInterfaceConfig(ctx context.Context, machineScope *scope.MachineScope
463463

464464
subnetName := machineScope.LinodeCluster.Spec.Network.SubnetName // name of subnet to use
465465

466-
ipv6RangeConfig := []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range{}
466+
var ipv6RangeConfig []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range
467467
if subnetName != "" {
468468
for _, subnet := range linodeVPC.Spec.Subnets {
469469
if subnet.Label == subnetName {
470470
subnetID = subnet.SubnetID
471-
ipv6RangeConfig = getIPv6RangeConfig(subnet.IPv6)
471+
ipv6RangeConfig = machineIPv6RangeConfig(len(subnet.IPv6))
472472
break
473473
}
474474
}
@@ -478,7 +478,7 @@ func getVPCInterfaceConfig(ctx context.Context, machineScope *scope.MachineScope
478478
}
479479
} else {
480480
subnetID = linodeVPC.Spec.Subnets[0].SubnetID // get first subnet if nothing specified
481-
ipv6RangeConfig = getIPv6RangeConfig(linodeVPC.Spec.Subnets[0].IPv6)
481+
ipv6RangeConfig = machineIPv6RangeConfig(len(linodeVPC.Spec.Subnets[0].IPv6))
482482
}
483483

484484
if subnetID == 0 {
@@ -506,6 +506,7 @@ func getVPCInterfaceConfig(ctx context.Context, machineScope *scope.MachineScope
506506
},
507507
}
508508

509+
// If IPv6 range config is not empty, add it to the interface configuration
509510
if len(ipv6RangeConfig) > 0 {
510511
vpcIntfCreateOpts.IPv6 = &linodego.InstanceConfigInterfaceCreateOptionsIPv6{
511512
Ranges: ipv6RangeConfig,
@@ -537,12 +538,12 @@ func getVPCInterfaceConfigFromDirectID(ctx context.Context, machineScope *scope.
537538
}
538539

539540
// If subnet name specified, find matching subnet; otherwise use first subnet
540-
ipv6RangeConfig := []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range{}
541+
var ipv6RangeConfig []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range
541542
if subnetName != "" {
542543
for _, subnet := range vpc.Subnets {
543544
if subnet.Label == subnetName {
544545
subnetID = subnet.ID
545-
ipv6RangeConfig = getIPv6RangeConfig(subnet.IPv6)
546+
ipv6RangeConfig = machineIPv6RangeConfig(len(subnet.IPv6))
546547
break
547548
}
548549
}
@@ -551,7 +552,7 @@ func getVPCInterfaceConfigFromDirectID(ctx context.Context, machineScope *scope.
551552
}
552553
} else {
553554
subnetID = vpc.Subnets[0].ID
554-
ipv6RangeConfig = getIPv6RangeConfig(vpc.Subnets[0].IPv6)
555+
ipv6RangeConfig = machineIPv6RangeConfig(len(vpc.Subnets[0].IPv6))
555556
}
556557

557558
// Check if a VPC interface already exists
@@ -577,7 +578,7 @@ func getVPCInterfaceConfigFromDirectID(ctx context.Context, machineScope *scope.
577578
},
578579
}
579580

580-
// If IPv6 ranges are specified, add them to the interface configuration
581+
// If IPv6 range config is not empty, add it to the interface configuration
581582
if len(ipv6RangeConfig) > 0 {
582583
vpcIntfCreateOpts.IPv6 = &linodego.InstanceConfigInterfaceCreateOptionsIPv6{
583584
Ranges: ipv6RangeConfig,
@@ -587,15 +588,18 @@ func getVPCInterfaceConfigFromDirectID(ctx context.Context, machineScope *scope.
587588
return vpcIntfCreateOpts, nil
588589
}
589590

590-
// getIPv6RangeConfig returns the IPv6 range configuration for a given slice of VPC IPv6 ranges
591-
func getIPv6RangeConfig(ipv6 []linodego.VPCIPv6Range) []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range {
592-
ipv6RangeConfig := []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range{}
593-
if len(ipv6) > 0 {
594-
ipv6RangeConfig = append(ipv6RangeConfig, linodego.InstanceConfigInterfaceCreateOptionsIPv6Range{
595-
Range: ptr.To(ipv6Range),
596-
})
591+
// machineIPv6RangeConfig returns the IPv6 range configuration if subnet has IPv6 ranges.
592+
// for now, we support only a single IPv6 range for machine per subnet.
593+
// If this changes, we may need to adjust this logic.
594+
func machineIPv6RangeConfig(numIPv6RangesInSubnet int) []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range {
595+
if numIPv6RangesInSubnet == 0 {
596+
return nil // No IPv6 ranges available in subnet, return empty slice
597+
}
598+
return []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range{
599+
{
600+
Range: ptr.To(defaultNodeIPv6CIDRRange),
601+
},
597602
}
598-
return ipv6RangeConfig
599603
}
600604

601605
func linodeMachineSpecToInstanceCreateConfig(machineSpec infrav1alpha2.LinodeMachineSpec, machineTags []string) *linodego.InstanceCreateOptions {

‎internal/controller/linodevpc_controller_helpers.go‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,28 +69,21 @@ func reconcileVPC(ctx context.Context, vpcScope *scope.VPCScope, logger logr.Log
6969
return err
7070
}
7171

72-
vpcScope.LinodeVPC.Spec.VPCID = &vpc.ID
73-
for _, ipv6 := range vpc.IPv6 {
74-
vpcScope.LinodeVPC.Spec.IPv6 = append(vpcScope.LinodeVPC.Spec.IPv6, linodego.VPCIPv6Range{Range: ipv6.Range})
75-
}
72+
setVPCFields(&vpcScope.LinodeVPC.Spec, vpc)
7673
updateVPCSpecSubnets(vpcScope, vpc)
7774

7875
return nil
7976
}
8077

8178
func reconcileExistingVPC(ctx context.Context, vpcScope *scope.VPCScope, vpc *linodego.VPC) error {
82-
// Labels are unique
83-
vpcScope.LinodeVPC.Spec.VPCID = &vpc.ID
84-
for _, ipv6 := range vpc.IPv6 {
85-
vpcScope.LinodeVPC.Spec.IPv6 = append(vpcScope.LinodeVPC.Spec.IPv6, linodego.VPCIPv6Range{Range: ipv6.Range})
86-
}
79+
setVPCFields(&vpcScope.LinodeVPC.Spec, vpc)
8780

8881
// build a map of existing subnets to easily check for existence
8982
existingSubnets := make(map[string]int, len(vpc.Subnets))
90-
existingSubnetsIpv6 := make(map[string][]linodego.VPCIPv6Range, len(vpc.Subnets))
83+
existingSubnetsIPv6 := make(map[string][]linodego.VPCIPv6Range, len(vpc.Subnets))
9184
for _, subnet := range vpc.Subnets {
9285
existingSubnets[subnet.Label] = subnet.ID
93-
existingSubnetsIpv6[subnet.Label] = subnet.IPv6
86+
existingSubnetsIPv6[subnet.Label] = subnet.IPv6
9487
}
9588

9689
// adopt or create subnets
@@ -100,7 +93,7 @@ func reconcileExistingVPC(ctx context.Context, vpcScope *scope.VPCScope, vpc *li
10093
}
10194
if id, ok := existingSubnets[subnet.Label]; ok {
10295
vpcScope.LinodeVPC.Spec.Subnets[idx].SubnetID = id
103-
vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6 = existingSubnetsIpv6[subnet.Label]
96+
vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6 = existingSubnetsIPv6[subnet.Label]
10497
} else {
10598
ipv6 := []linodego.VPCSubnetCreateOptionsIPv6{}
10699
for _, ipv6Range := range subnet.IPv6Range {
@@ -118,10 +111,7 @@ func reconcileExistingVPC(ctx context.Context, vpcScope *scope.VPCScope, vpc *li
118111
if err != nil {
119112
return err
120113
}
121-
vpcScope.LinodeVPC.Spec.Subnets[idx].SubnetID = newSubnet.ID
122-
for _, ipv6 := range newSubnet.IPv6 {
123-
vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6 = append(vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6, linodego.VPCIPv6Range{Range: ipv6.Range})
124-
}
114+
setSubnetFields(&vpcScope.LinodeVPC.Spec.Subnets[idx], newSubnet)
125115
}
126116
}
127117

@@ -133,16 +123,33 @@ func updateVPCSpecSubnets(vpcScope *scope.VPCScope, vpc *linodego.VPC) {
133123
for idx, specSubnet := range vpcScope.LinodeVPC.Spec.Subnets {
134124
for _, vpcSubnet := range vpc.Subnets {
135125
if specSubnet.Label == vpcSubnet.Label {
136-
vpcScope.LinodeVPC.Spec.Subnets[idx].SubnetID = vpcSubnet.ID
137-
for _, ipv6 := range vpcSubnet.IPv6 {
138-
vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6 = append(vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6, linodego.VPCIPv6Range{Range: ipv6.Range})
139-
}
126+
setSubnetFields(&vpcScope.LinodeVPC.Spec.Subnets[idx], &vpcSubnet)
140127
break
141128
}
142129
}
143130
}
144131
}
145132

133+
// setVPCFields sets the VPCID and IPv6 in the LinodeVPCSpec from the Linode VPC.
134+
func setVPCFields(vpc *infrav1alpha2.LinodeVPCSpec, linodeVPC *linodego.VPC) {
135+
vpc.VPCID = &linodeVPC.ID
136+
// Clear existing IPv6 ranges and set new ones
137+
vpc.IPv6 = nil
138+
for _, ipv6 := range linodeVPC.IPv6 {
139+
vpc.IPv6 = append(vpc.IPv6, linodego.VPCIPv6Range{Range: ipv6.Range})
140+
}
141+
}
142+
143+
// setSubnetFields sets the SubnetID and IPv6 in the VPCSubnetCreateOptions from the Linode VPCSubnet.
144+
func setSubnetFields(subnet *infrav1alpha2.VPCSubnetCreateOptions, vpcSubnet *linodego.VPCSubnet) {
145+
subnet.SubnetID = vpcSubnet.ID
146+
// Clear existing IPv6 ranges and set new ones
147+
subnet.IPv6 = nil
148+
for _, ipv6 := range vpcSubnet.IPv6 {
149+
subnet.IPv6 = append(subnet.IPv6, linodego.VPCIPv6Range{Range: ipv6.Range})
150+
}
151+
}
152+
146153
func linodeVPCSpecToVPCCreateConfig(vpcSpec infrav1alpha2.LinodeVPCSpec) *linodego.VPCCreateOptions {
147154
vpcIPv6 := make([]linodego.VPCCreateOptionsIPv6, len(vpcSpec.IPv6Range))
148155
for idx, ipv6 := range vpcSpec.IPv6Range {

‎internal/webhook/v1alpha2/linodevpc_webhook.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,12 @@ func validateSubnetIPv4CIDR(cidr string, path *field.Path) (*netipx.IPSet, *fiel
303303
}
304304

305305
func validateIPv6Range(ipv6Range *string, path *field.Path) *field.Error {
306-
var errs = []error{
307-
errors.New("IPv6 range must be either 'auto' or start with /. Example: /52"),
308-
errors.New("IPv6 range doesn't contain a valid number after /"),
309-
errors.New("IPv6 range must be between /0 and /128"),
310-
}
306+
const (
307+
errIPv6RangeFormat = "IPv6 range must be either 'auto' or start with /. Example: /52"
308+
errIPv6RangeNoNumber = "IPv6 range doesn't contain a valid number after /"
309+
errIPv6RangeBounds = "IPv6 range must be between /0 and /128"
310+
)
311+
311312
ipv6RangeStr := "auto"
312313
if ipv6Range != nil {
313314
ipv6RangeStr = *ipv6Range
@@ -319,16 +320,16 @@ func validateIPv6Range(ipv6Range *string, path *field.Path) *field.Error {
319320
}
320321

321322
if ipv6RangeStr == "" || !strings.HasPrefix(ipv6RangeStr, "/") {
322-
return field.Invalid(path, ipv6RangeStr, errs[0].Error())
323+
return field.Invalid(path, ipv6RangeStr, errIPv6RangeFormat)
323324
}
324325

325326
numStr := strings.TrimPrefix(ipv6RangeStr, "/")
326327
num, err := strconv.Atoi(numStr)
327328
if err != nil {
328-
return field.Invalid(path, ipv6RangeStr, errs[1].Error())
329+
return field.Invalid(path, ipv6RangeStr, errIPv6RangeNoNumber)
329330
}
330331
if num < 0 || num > 128 {
331-
return field.Invalid(path, ipv6RangeStr, errs[2].Error())
332+
return field.Invalid(path, ipv6RangeStr, errIPv6RangeBounds)
332333
}
333334
return nil
334335
}

0 commit comments

Comments
 (0)