Merge https://github.com/kubernetes-sigs/cluster-api-provider-openstack:release-0.10 into main

Makefile

@@ -306,7 +306,7 @@ generate-api-docs-%: $(GEN_CRD_API_REFERENCE_DOCS) FORCE
 
 .PHONY: docker-build
 docker-build: ## Build the docker image for controller-manager
-	docker build -f Dockerfile --build-arg goproxy=$(GOPROXY) --build-arg ARCH=$(ARCH) --build-arg LDFLAGS="$(LDFLAGS)" . -t $(CONTROLLER_IMG_TAG)
+	docker build -f Dockerfile --build-arg goproxy=$(GOPROXY) --build-arg ARCH=$(ARCH) --build-arg ldflags="$(LDFLAGS)" . -t $(CONTROLLER_IMG_TAG)
 
 .PHONY: docker-push
 docker-push: ## Push the docker image

api/v1alpha6/openstackcluster_conversion.go

@@ -478,7 +478,10 @@ 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)
+
+	if (*dst).Enabled != nil && !*(*dst).Enabled {
+		(*dst).Enabled = (*previous).Enabled
+	}
 }
 
 func Convert_v1alpha6_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Bastion, s apiconversion.Scope) error {

api/v1alpha7/openstackcluster_conversion.go

@@ -416,7 +416,10 @@ func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
 	restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec)
 	optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
 	optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
-	optional.RestoreBool(&(*previous).Enabled, &(*dst).Enabled)
+
+	if (*dst).Enabled != nil && !*(*dst).Enabled {
+		(*dst).Enabled = (*previous).Enabled
+	}
 }
 
 func Convert_v1alpha7_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Bastion, s apiconversion.Scope) error {

api/v1beta1/types.go

@@ -795,7 +795,7 @@ type Bastion struct {
 	// waiting until the bastion has been deleted.
 	// +kubebuilder:default:=true
 	// +optional
-	Enabled optional.Bool `json:"enabled,omitempty"`
+	Enabled *bool `json:"enabled,omitempty"`
 
 	// Spec for the bastion itself
 	Spec *OpenStackMachineSpec `json:"spec,omitempty"`

config/manager/manager.yaml

@@ -46,6 +46,7 @@ spec:
             privileged: false
             runAsUser: 65532
             runAsGroup: 65532
+          terminationMessagePolicy: FallbackToLogsOnError
       terminationGracePeriodSeconds: 10
       securityContext:
         runAsNonRoot: true

main.go

@@ -233,6 +233,7 @@ func main() {
 	cfg, err := config.GetConfigWithContext(os.Getenv("KUBECONTEXT"))
 	if err != nil {
 		setupLog.Error(err, "unable to get kubeconfig")
+		os.Exit(1)
 	}
 	cfg.QPS = restConfigQPS
 	cfg.Burst = restConfigBurst

pkg/cloud/services/networking/port.go

@@ -192,7 +192,7 @@ func (s *Service) CreatePort(eventObject runtime.Object, portSpec *infrav1.Resol
 
 	port, err := s.client.CreatePort(builder)
 	if err != nil {
-		record.Warnf(eventObject, "FailedCreatePort", "Failed to create port %s: %v", port.Name, err)
+		record.Warnf(eventObject, "FailedCreatePort", "Failed to create port %s: %v", portSpec.Name, err)
 		return nil, err
 	}
 

pkg/webhooks/fuzz_test.go

@@ -0,0 +1,92 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package webhooks
+
+import (
+	"context"
+	"runtime/debug"
+	"testing"
+
+	"github.com/onsi/gomega/format"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
+	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
+)
+
+type pointerToObject[T any] interface {
+	*T
+	runtime.Object
+}
+
+// fuzzCustomValidator fuzzes a CustomValidator with objects of the validator's expected type.
+func fuzzCustomValidator[O any, PO pointerToObject[O]](t *testing.T, name string, validator webhook.CustomValidator) {
+	t.Helper()
+	fuzz := utilconversion.GetFuzzer(scheme.Scheme)
+	ctx := context.TODO()
+
+	t.Run(name, func(t *testing.T) {
+		for i := 0; i < 1000; i++ {
+			var previous PO = new(O)
+			var dst PO = new(O)
+			fuzz.Fuzz(previous)
+			fuzz.Fuzz(dst)
+
+			checkPanic := func(f func(), name string, args ...runtime.Object) {
+				defer func() {
+					if r := recover(); r != nil {
+						t.Errorf("PANIC in %s", name)
+						for i, arg := range args {
+							t.Errorf("arg %d:\n%s", i, format.Object(arg, 1))
+						}
+						t.Errorf("Stack trace:\n%s", debug.Stack())
+						t.FailNow()
+					}
+				}()
+				f()
+			}
+
+			checkPanic(func() {
+				_, _ = validator.ValidateCreate(ctx, dst)
+			}, "ValidateCreate()", dst)
+			checkPanic(func() {
+				_, _ = validator.ValidateUpdate(ctx, previous, dst)
+			}, "ValidateUpdate()", previous, dst)
+			checkPanic(func() {
+				_, _ = validator.ValidateDelete(ctx, previous)
+			}, "ValidateDelete()", previous)
+		}
+	})
+}
+
+func Test_FuzzClusterWebhook(t *testing.T) {
+	fuzzCustomValidator[infrav1.OpenStackCluster](t, "OpenStackCluster", &openStackClusterWebhook{})
+}
+
+func Test_FuzzClusterTemplateWebhook(t *testing.T) {
+	fuzzCustomValidator[infrav1.OpenStackClusterTemplate](t, "OpenStackClusterTemplate", &openStackClusterTemplateWebhook{})
+}
+
+func Test_FuzzMachineWebhook(t *testing.T) {
+	fuzzCustomValidator[infrav1.OpenStackMachine](t, "OpenStackMachine", &openStackMachineWebhook{})
+}
+
+func Test_FuzzMachineTemplateWebhook(t *testing.T) {
+	fuzzCustomValidator[infrav1.OpenStackMachineTemplate](t, "OpenStackMachineTemplate", &openStackMachineTemplateWebhook{})
+}

pkg/webhooks/openstackcluster_webhook.go

@@ -121,6 +121,10 @@ func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, new
 
 	// Allow changes to the managed allNodesSecurityGroupRules.
 	if newObj.Spec.ManagedSecurityGroups != nil {
+		if oldObj.Spec.ManagedSecurityGroups == nil {
+			oldObj.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{}
+		}
+
 		oldObj.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules = []infrav1.SecurityGroupRuleSpec{}
 		newObj.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules = []infrav1.SecurityGroupRuleSpec{}
 

test/e2e/suites/apivalidations/openstackcluster_test.go

@@ -215,6 +215,24 @@ var _ = Describe("OpenStackCluster API validations", func() {
 			Expect(cluster.Spec.ControlPlaneEndpoint).To(Equal(*infrav1Cluster.Spec.ControlPlaneEndpoint), "Control plane endpoint should be restored")
 			Expect(cluster.Spec.IdentityRef.Kind).To(Equal("FakeKind"), "IdentityRef.Kind should be restored")
 		})
+
+		It("should not enable an explicitly disabled bastion when converting to v1beta1", func() {
+			cluster.Spec.Bastion = &infrav1alpha7.Bastion{Enabled: false}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+
+			// Fetch the infrav1 version of the cluster
+			infrav1Cluster := &infrav1.OpenStackCluster{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, infrav1Cluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
+
+			infrav1Bastion := infrav1Cluster.Spec.Bastion
+
+			// NOTE(mdbooth): It may be reasonable to remove the
+			// bastion if it is disabled with no other properties.
+			// It would be reasonable to update the assertions
+			// accordingly if we did that.
+			Expect(infrav1Bastion).ToNot(BeNil(), "Bastion should not have been removed")
+			Expect(infrav1Bastion.Enabled).To(Equal(ptr.To(false)), "Bastion should remain disabled")
+		})
 	})
 
 	Context("v1alpha6", func() {
@@ -252,5 +270,23 @@ var _ = Describe("OpenStackCluster API validations", func() {
 			Expect(cluster.Spec.ControlPlaneEndpoint).To(Equal(*infrav1Cluster.Spec.ControlPlaneEndpoint), "Control plane endpoint should be restored")
 			Expect(cluster.Spec.IdentityRef.Kind).To(Equal("FakeKind"), "IdentityRef.Kind should be restored")
 		})
+
+		It("should not enable an explicitly disabled bastion when converting to v1beta1", func() {
+			cluster.Spec.Bastion = &infrav1alpha6.Bastion{Enabled: false}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+
+			// Fetch the infrav1 version of the cluster
+			infrav1Cluster := &infrav1.OpenStackCluster{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, infrav1Cluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
+
+			infrav1Bastion := infrav1Cluster.Spec.Bastion
+
+			// NOTE(mdbooth): It may be reasonable to remove the
+			// bastion if it is disabled with no other properties.
+			// It would be reasonable to update the assertions
+			// accordingly if we did that.
+			Expect(infrav1Bastion).ToNot(BeNil(), "Bastion should not have been removed")
+			Expect(infrav1Bastion.Enabled).To(Equal(ptr.To(false)), "Bastion should remain disabled")
+		})
 	})
 })