Skip to content

Commit ffa82b3

Browse files
dulekdulek
committed
API: Cleanup Bastion
This commit works with .spec.bastion: * makes availablityZone, floatingIP and instance pointers * adds floatingIP IPv4 validation * adds validation that if Bastion is enabled, instance cannot be null
1 parent 27b3cef commit ffa82b3

18 files changed

+260
-74
lines changed

‎api/v1alpha5/conversion.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,9 @@ func Convert_v1beta1_Bastion_To_v1alpha5_Bastion(in *infrav1.Bastion, out *Basti
671671
if err != nil {
672672
return err
673673
}
674-
in.FloatingIP = out.Instance.FloatingIP
674+
if in.FloatingIP != nil {
675+
out.Instance.FloatingIP = *in.FloatingIP
676+
}
675677
return nil
676678
}
677679

@@ -680,7 +682,9 @@ func Convert_v1alpha5_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Basti
680682
if err != nil {
681683
return err
682684
}
683-
in.Instance.FloatingIP = out.FloatingIP
685+
if in.Instance.FloatingIP != "" {
686+
out.FloatingIP = &in.Instance.FloatingIP
687+
}
684688
return nil
685689
}
686690

‎api/v1alpha5/zz_generated.conversion.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1alpha6/openstackcluster_conversion.go

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,13 @@ func Convert_v1alpha6_OpenStackClusterStatus_To_v1beta1_OpenStackClusterStatus(i
412412
/* Bastion */
413413

414414
func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
415-
if *previous != nil && *dst != nil {
416-
restorev1beta1MachineSpec(&(*previous).Instance, &(*dst).Instance)
415+
if *previous != nil {
416+
if *dst != nil && (*previous).Instance != nil && (*dst).Instance != nil {
417+
restorev1beta1MachineSpec((*previous).Instance, (*dst).Instance)
418+
}
419+
420+
optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
421+
optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
417422
}
418423
}
419424

@@ -445,13 +450,30 @@ func Convert_v1alpha6_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Basti
445450
return err
446451
}
447452

448-
if in.Instance.ServerGroupID != "" {
449-
out.Instance.ServerGroup = &infrav1.ServerGroupFilter{ID: in.Instance.ServerGroupID}
450-
} else {
451-
out.Instance.ServerGroup = nil
453+
if !reflect.ValueOf(in.Instance).IsZero() {
454+
out.Instance = &infrav1.OpenStackMachineSpec{}
455+
456+
err = Convert_v1alpha6_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(&in.Instance, out.Instance, s)
457+
if err != nil {
458+
return err
459+
}
460+
461+
if in.Instance.ServerGroupID != "" {
462+
out.Instance.ServerGroup = &infrav1.ServerGroupFilter{ID: in.Instance.ServerGroupID}
463+
} else {
464+
out.Instance.ServerGroup = nil
465+
}
466+
467+
err = optional.Convert_string_To_optional_String(&in.Instance.FloatingIP, &out.FloatingIP, s)
468+
if err != nil {
469+
return err
470+
}
452471
}
453472

454-
out.FloatingIP = in.Instance.FloatingIP
473+
// nil the Instance if it's basically an empty object.
474+
if out.Instance != nil && reflect.ValueOf(*out.Instance).IsZero() {
475+
out.Instance = nil
476+
}
455477
return nil
456478
}
457479

@@ -461,10 +483,16 @@ func Convert_v1beta1_Bastion_To_v1alpha6_Bastion(in *infrav1.Bastion, out *Basti
461483
return err
462484
}
463485

464-
if in.Instance.ServerGroup != nil && in.Instance.ServerGroup.ID != "" {
465-
out.Instance.ServerGroupID = in.Instance.ServerGroup.ID
486+
if in.Instance != nil {
487+
err = Convert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in.Instance, &out.Instance, s)
488+
if err != nil {
489+
return err
490+
}
491+
492+
if in.Instance.ServerGroup != nil && in.Instance.ServerGroup.ID != "" {
493+
out.Instance.ServerGroupID = in.Instance.ServerGroup.ID
494+
}
466495
}
467496

468-
out.Instance.FloatingIP = in.FloatingIP
469-
return nil
497+
return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s)
470498
}

‎api/v1alpha6/openstackmachine_conversion.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa
133133
}
134134

135135
if kd != k || vd != v {
136+
if dst.ServerMetadata == nil {
137+
dst.ServerMetadata = make(map[string]string)
138+
}
136139
delete(dst.ServerMetadata, kd)
137140
dst.ServerMetadata[k] = v
138141
}

‎api/v1alpha6/zz_generated.conversion.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1alpha7/openstackcluster_conversion.go

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ limitations under the License.
1717
package v1alpha7
1818

1919
import (
20+
"reflect"
21+
2022
apiconversion "k8s.io/apimachinery/pkg/conversion"
2123
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2224
ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion"
@@ -373,8 +375,13 @@ func restorev1alpha7Bastion(previous **Bastion, dst **Bastion) {
373375
}
374376

375377
func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
376-
if *previous != nil && *dst != nil {
377-
restorev1beta1MachineSpec(&(*previous).Instance, &(*dst).Instance)
378+
if *previous != nil {
379+
if *dst != nil && (*previous).Instance != nil && (*dst).Instance != nil {
380+
restorev1beta1MachineSpec((*previous).Instance, (*dst).Instance)
381+
}
382+
383+
optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
384+
optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
378385
}
379386
}
380387

@@ -384,13 +391,30 @@ func Convert_v1alpha7_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Basti
384391
return err
385392
}
386393

387-
if in.Instance.ServerGroupID != "" {
388-
out.Instance.ServerGroup = &infrav1.ServerGroupFilter{ID: in.Instance.ServerGroupID}
389-
} else {
390-
out.Instance.ServerGroup = nil
394+
if !reflect.ValueOf(in.Instance).IsZero() {
395+
out.Instance = &infrav1.OpenStackMachineSpec{}
396+
397+
err = Convert_v1alpha7_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(&in.Instance, out.Instance, s)
398+
if err != nil {
399+
return err
400+
}
401+
402+
if in.Instance.ServerGroupID != "" {
403+
out.Instance.ServerGroup = &infrav1.ServerGroupFilter{ID: in.Instance.ServerGroupID}
404+
} else {
405+
out.Instance.ServerGroup = nil
406+
}
407+
408+
err = optional.Convert_string_To_optional_String(&in.Instance.FloatingIP, &out.FloatingIP, s)
409+
if err != nil {
410+
return err
411+
}
391412
}
392413

393-
out.FloatingIP = in.Instance.FloatingIP
414+
// nil the Instance if it's basically an empty object.
415+
if out.Instance != nil && reflect.ValueOf(*out.Instance).IsZero() {
416+
out.Instance = nil
417+
}
394418
return nil
395419
}
396420

@@ -400,12 +424,18 @@ func Convert_v1beta1_Bastion_To_v1alpha7_Bastion(in *infrav1.Bastion, out *Basti
400424
return err
401425
}
402426

403-
if in.Instance.ServerGroup != nil && in.Instance.ServerGroup.ID != "" {
404-
out.Instance.ServerGroupID = in.Instance.ServerGroup.ID
427+
if in.Instance != nil {
428+
err = Convert_v1beta1_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec(in.Instance, &out.Instance, s)
429+
if err != nil {
430+
return err
431+
}
432+
433+
if in.Instance.ServerGroup != nil && in.Instance.ServerGroup.ID != "" {
434+
out.Instance.ServerGroupID = in.Instance.ServerGroup.ID
435+
}
405436
}
406437

407-
out.Instance.FloatingIP = in.FloatingIP
408-
return nil
438+
return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s)
409439
}
410440

411441
func Convert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in *infrav1.BastionStatus, out *BastionStatus, s apiconversion.Scope) error {

‎api/v1alpha7/openstackmachine_conversion.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ func restorev1alpha7MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa
107107
}
108108

109109
if kd != k || vd != v {
110+
if dst.ServerMetadata == nil {
111+
dst.ServerMetadata = make(map[string]string)
112+
}
110113
delete(dst.ServerMetadata, kd)
111114
dst.ServerMetadata[k] = v
112115
}

‎api/v1alpha7/zz_generated.conversion.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1beta1/types.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -596,21 +596,27 @@ var (
596596
InstanceStateUndefined = InstanceState("")
597597
)
598598

599-
// Bastion represents basic information about the bastion node.
599+
// Bastion represents basic information about the bastion node. If you enable bastion, the spec has to be specified.
600+
// +kubebuilder:validation:XValidation:rule="!self.enabled || has(self.instance)",message="you need to specify the instance if bastion is enabled"
600601
type Bastion struct {
601-
//+optional
602+
// Enabled means that bastion is enabled. Defaults to false.
603+
// +kubebuilder:validation:Required
604+
// +kubebuilder:default:=false
602605
Enabled bool `json:"enabled"`
603606

604607
// Instance for the bastion itself
605-
Instance OpenStackMachineSpec `json:"instance,omitempty"`
608+
Instance *OpenStackMachineSpec `json:"instance,omitempty"`
606609

610+
// AvailabilityZone is the failure domain that will be used to create the Bastion Instance.
607611
//+optional
608-
AvailabilityZone string `json:"availabilityZone,omitempty"`
612+
AvailabilityZone optional.String `json:"availabilityZone,omitempty"`
609613

610-
// FloatingIP which will be associated to the bastion machine.
611-
// The floating IP should already exist and should not be associated with a port.
614+
// FloatingIP which will be associated to the bastion machine. It's the IP address, not UUID.
615+
// The floating IP should already exist and should not be associated with a port. If FIP of this address does not
616+
// exist, CAPO will try to create it, but by default only OpenStack administrators have privileges to do so.
612617
//+optional
613-
FloatingIP string `json:"floatingIP,omitempty"`
618+
//+kubebuilder:validation:Format:=ipv4
619+
FloatingIP optional.String `json:"floatingIP,omitempty"`
614620
}
615621

616622
type APIServerLoadBalancer struct {

‎api/v1beta1/zz_generated.deepcopy.go

Lines changed: 15 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4917,13 +4917,20 @@ spec:
49174917
to first set `enabled: false` which will remove the bastion and then changes can be made.
49184918
properties:
49194919
availabilityZone:
4920+
description: AvailabilityZone is the failure domain that will
4921+
be used to create the Bastion Instance.
49204922
type: string
49214923
enabled:
4924+
default: false
4925+
description: Enabled means that bastion is enabled. Defaults to
4926+
false.
49224927
type: boolean
49234928
floatingIP:
49244929
description: |-
4925-
FloatingIP which will be associated to the bastion machine.
4926-
The floating IP should already exist and should not be associated with a port.
4930+
FloatingIP which will be associated to the bastion machine. It's the IP address, not UUID.
4931+
The floating IP should already exist and should not be associated with a port. If FIP of this address does not
4932+
exist, CAPO will try to create it, but by default only OpenStack administrators have privileges to do so.
4933+
format: ipv4
49274934
type: string
49284935
instance:
49294936
description: Instance for the bastion itself
@@ -5572,7 +5579,12 @@ spec:
55725579
- flavor
55735580
- image
55745581
type: object
5582+
required:
5583+
- enabled
55755584
type: object
5585+
x-kubernetes-validations:
5586+
- message: you need to specify the instance if bastion is enabled
5587+
rule: '!self.enabled || has(self.instance)'
55765588
controlPlaneAvailabilityZones:
55775589
description: |-
55785590
ControlPlaneAvailabilityZones is the set of availability zones which

0 commit comments

Comments
 (0)