Skip to content

Commit 51894f6

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

File tree

6 files changed

+39
-30
lines changed

6 files changed

+39
-30
lines changed

‎api/v1alpha2/linodevpc_types.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ type LinodeVPCSpec struct {
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: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,11 @@ spec:
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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -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: 19 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(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(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(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(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,17 @@ 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 per subnet. If this changes, we may need to adjust this logic.
593+
func machineIPv6RangeConfig(ipv6 []linodego.VPCIPv6Range) []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range {
594+
if len(ipv6) == 0 {
595+
return nil // No IPv6 ranges available in subnet, return empty slice
596+
}
597+
return []linodego.InstanceConfigInterfaceCreateOptionsIPv6Range{
598+
{
599+
Range: ptr.To(defaultNodeIPv6CIDRRange),
600+
},
597601
}
598-
return ipv6RangeConfig
599602
}
600603

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

‎internal/controller/linodevpc_controller_helpers.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,10 @@ func reconcileExistingVPC(ctx context.Context, vpcScope *scope.VPCScope, vpc *li
8787

8888
// build a map of existing subnets to easily check for existence
8989
existingSubnets := make(map[string]int, len(vpc.Subnets))
90-
existingSubnetsIpv6 := make(map[string][]linodego.VPCIPv6Range, len(vpc.Subnets))
90+
existingSubnetsIPv6 := make(map[string][]linodego.VPCIPv6Range, len(vpc.Subnets))
9191
for _, subnet := range vpc.Subnets {
9292
existingSubnets[subnet.Label] = subnet.ID
93-
existingSubnetsIpv6[subnet.Label] = subnet.IPv6
93+
existingSubnetsIPv6[subnet.Label] = subnet.IPv6
9494
}
9595

9696
// adopt or create subnets
@@ -100,7 +100,7 @@ func reconcileExistingVPC(ctx context.Context, vpcScope *scope.VPCScope, vpc *li
100100
}
101101
if id, ok := existingSubnets[subnet.Label]; ok {
102102
vpcScope.LinodeVPC.Spec.Subnets[idx].SubnetID = id
103-
vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6 = existingSubnetsIpv6[subnet.Label]
103+
vpcScope.LinodeVPC.Spec.Subnets[idx].IPv6 = existingSubnetsIPv6[subnet.Label]
104104
} else {
105105
ipv6 := []linodego.VPCSubnetCreateOptionsIPv6{}
106106
for _, ipv6Range := range subnet.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)