forbid disabling share networks

pkg/apis/openstack/validation/infrastructure.go

@@ -74,11 +74,15 @@ func ValidateInfrastructureConfig(infra *api.InfrastructureConfig, nodesCIDR *st
 func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *api.InfrastructureConfig, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	newNetworks := newConfig.Networks
-	oldNetworks := oldConfig.Networks
-	// share network changes are allowed, therefore ignore them on comparing
-	newNetworks.ShareNetwork = nil
-	oldNetworks.ShareNetwork = nil
+	newNetworks := newConfig.DeepCopy().Networks
+	oldNetworks := oldConfig.DeepCopy().Networks
+
+	// only enablement of share network enablement is allowed as update operation. Therefore we ignore it, when checking for other updates.
+	// TODO: allow both enabling and disabling of share networks.
+	if oldNetworks.ShareNetwork == nil || !oldNetworks.ShareNetwork.Enabled {
+		newNetworks.ShareNetwork = nil
+		oldNetworks.ShareNetwork = nil
+	}
 	allErrs = append(allErrs, apivalidation.ValidateImmutableField(newNetworks, oldNetworks, fldPath.Child("networks"))...)
 	allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.FloatingPoolName, oldConfig.FloatingPoolName, fldPath.Child("floatingPoolName"))...)
 	allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.FloatingPoolSubnetName, oldConfig.FloatingPoolSubnetName, fldPath.Child("floatingPoolSubnetName"))...)

pkg/apis/openstack/validation/infrastructure_test.go

@@ -171,14 +171,24 @@ var _ = Describe("InfrastructureConfig validation", func() {
 			}))))
 		})
 
-		It("should allow changing the share network section", func() {
+		It("should allow enabling the share network section", func() {
 			newInfrastructureConfig := infrastructureConfig.DeepCopy()
 			newInfrastructureConfig.Networks.ShareNetwork = &api.ShareNetwork{Enabled: true}
 
 			errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, nilPath)
-
 			Expect(errorList).To(BeEmpty())
 		})
+		It("should forbid disabling the share network section", func() {
+			infrastructureConfig.Networks.ShareNetwork = &api.ShareNetwork{Enabled: true}
+			newInfrastructureConfig := infrastructureConfig.DeepCopy()
+			newInfrastructureConfig.Networks.ShareNetwork = nil
+
+			errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, nilPath)
+			Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
+				"Type":  Equal(field.ErrorTypeInvalid),
+				"Field": Equal("networks"),
+			}))))
+		})
 
 		It("should forbid changing the floating pool", func() {
 			newInfrastructureConfig := infrastructureConfig.DeepCopy()

pkg/controller/infrastructure/infraflow/delete.go

@@ -235,6 +235,10 @@ func (fctx *FlowContext) deleteSSHKeyPair(ctx context.Context) error {
 }
 
 func (fctx *FlowContext) deleteShareNetwork(ctx context.Context) error {
+	if sn := fctx.config.Networks.ShareNetwork; sn == nil || !sn.Enabled {
+		return nil
+	}
+
 	log := shared.LogFromContext(ctx)
 	networkID := ptr.Deref(fctx.state.Get(IdentifierNetwork), "")
 	subnetID := ptr.Deref(fctx.state.Get(IdentifierSubnet), "")

pkg/controller/infrastructure/infraflow/reconcile.go

@@ -467,8 +467,8 @@ func (fctx *FlowContext) ensureSSHKeyPair(ctx context.Context) error {
 }
 
 func (fctx *FlowContext) ensureShareNetwork(ctx context.Context) error {
-	if fctx.config.Networks.ShareNetwork == nil || !fctx.config.Networks.ShareNetwork.Enabled {
-		return fctx.deleteShareNetwork(ctx)
+	if sn := fctx.config.Networks.ShareNetwork; sn == nil || !sn.Enabled {
+		return nil
 	}
 
 	log := shared.LogFromContext(ctx)