Merge pull request kubernetes-sigs#1990 from shiftstack/bastionenabled

⚠️ Bastion is enabled by default if specified

Author: k8s-ci-robot

api/v1alpha5/zz_generated.conversion.go

@@ -416,6 +416,7 @@ func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
 
 	optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
 	optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
+	optional.RestoreBool(&(*previous).Enabled, &(*dst).Enabled)
 }
 
 func Convert_v1alpha6_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Bastion, s apiconversion.Scope) error {

api/v1alpha6/openstackcluster_conversion.go

@@ -384,6 +384,7 @@ func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
 
 		optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
 		optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
+		optional.RestoreBool(&(*previous).Enabled, &(*dst).Enabled)
 	}
 }
 

api/v1alpha6/zz_generated.conversion.go

@@ -719,12 +719,18 @@ var (
 )
 
 // Bastion represents basic information about the bastion node. If you enable bastion, the spec has to be specified.
-// +kubebuilder:validation:XValidation:rule="!self.enabled || has(self.spec)",message="you need to specify the spec if bastion is enabled"
+// +kubebuilder:validation:XValidation:rule="!self.enabled || has(self.spec)",message="spec is required if bastion is enabled"
 type Bastion struct {
-	// Enabled means that bastion is enabled. Defaults to false.
-	// +kubebuilder:validation:Required
-	// +kubebuilder:default:=false
-	Enabled bool `json:"enabled"`
+	// Enabled means that bastion is enabled. The bastion is enabled by
+	// default if this field is not specified. Set this field to false to disable the
+	// bastion.
+	//
+	// It is not currently possible to remove the bastion from the cluster
+	// spec without first disabling it by setting this field to false and
+	// waiting until the bastion has been deleted.
+	// +kubebuilder:default:=true
+	// +optional
+	Enabled optional.Bool `json:"enabled,omitempty"`
 
 	// Spec for the bastion itself
 	Spec *OpenStackMachineSpec `json:"spec,omitempty"`
@@ -741,6 +747,13 @@ type Bastion struct {
 	FloatingIP optional.String `json:"floatingIP,omitempty"`
 }
 
+func (b *Bastion) IsEnabled() bool {
+	if b == nil {
+		return false
+	}
+	return b.Enabled == nil || *b.Enabled
+}
+
 type APIServerLoadBalancer struct {
 	// Enabled defines whether a load balancer should be created. This value
 	// defaults to true if an APIServerLoadBalancer is given.

api/v1alpha7/openstackcluster_conversion.go

@@ -219,7 +219,7 @@ func contains(arr []string, target string) bool {
 
 func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster) (bool, error) {
 	// Resolve and store resources for the bastion
-	if openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.Enabled {
+	if openStackCluster.Spec.Bastion.IsEnabled() {
 		if openStackCluster.Status.Bastion == nil {
 			openStackCluster.Status.Bastion = &infrav1.BastionStatus{}
 		}
@@ -406,7 +406,7 @@ func reconcileBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openS
 	}
 
 	// No Bastion defined
-	if openStackCluster.Spec.Bastion == nil || !openStackCluster.Spec.Bastion.Enabled {
+	if !openStackCluster.Spec.Bastion.IsEnabled() {
 		// Delete any existing bastion
 		if openStackCluster.Status.Bastion != nil {
 			if err := deleteBastion(scope, cluster, openStackCluster); err != nil {
@@ -440,6 +440,8 @@ func reconcileBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openS
 		return nil, fmt.Errorf("failed computing bastion hash from instance spec: %w", err)
 	}
 	if bastionHashHasChanged(bastionHash, openStackCluster.ObjectMeta.Annotations) {
+		scope.Logger().Info("Bastion instance spec has changed, deleting existing bastion")
+
 		if err := deleteBastion(scope, cluster, openStackCluster); err != nil {
 			return nil, err
 		}
@@ -544,7 +546,7 @@ func bastionAddFloatingIP(openStackCluster *infrav1.OpenStackCluster, clusterRes
 func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster *clusterv1.Cluster) (*compute.InstanceSpec, error) {
 	bastion := openStackCluster.Spec.Bastion
 	if bastion == nil {
-		return nil, fmt.Errorf("bastion spec is nil")
+		bastion = &infrav1.Bastion{}
 	}
 	if bastion.Spec == nil {
 		// For the case when Bastion is deleted but we don't have spec, let's use an empty one.