Check for valid service override endpoint type on create and update

Depends-On: openstack-k8s-operators/lib-common#505

api/go.mod

@@ -77,3 +77,5 @@ require (
 // mschuppert: map to latest commit from release-4.13 tag
 // must consistent within modules and service operators
 replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 //allow-merging
+
+replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/stuggi/lib-common/modules/common v0.0.0-20240508083919-01ef8c1d28c5

api/go.sum

@@ -81,8 +81,6 @@ github.com/onsi/gomega v1.33.0 h1:snPCflnZrpMsy94p4lXVEkHo12lmPnc3vY5XBbreexE=
 github.com/onsi/gomega v1.33.0/go.mod h1:+925n5YtiFsLzzafLUHzVMBpvvRAzrydIBiSIxjX3wY=
 github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxCMwNRnMjhhIDOWHJowi6q8G6koI=
 github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6 h1:WLsG3Ko+phW5xZJjncypLWGASoLqKrt05qN9Zxsad6g=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
@@ -108,6 +106,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stuggi/lib-common/modules/common v0.0.0-20240508083919-01ef8c1d28c5 h1:RwBWawOf7fLsojvTdCeCZ+O5k3VqOqoX5f7PRQfa9lY=
+github.com/stuggi/lib-common/modules/common v0.0.0-20240508083919-01ef8c1d28c5/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=

api/v1beta1/ironic_webhook.go

@@ -22,6 +22,7 @@ import (
 	"net"
 	"strings"
 
+	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -133,6 +134,10 @@ func (spec *IronicSpecCore) ValidateCreate(basePath *field.Path) field.ErrorList
 		allErrs = append(allErrs, err)
 	}
 
+	if err := validateAPISpec(spec, basePath); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
 	if err := validateConductorSpec(spec, basePath); err != nil {
 		allErrs = append(allErrs, err...)
 	}
@@ -194,6 +199,10 @@ func (spec *IronicSpecCore) ValidateUpdate(old IronicSpecCore, basePath *field.P
 		allErrs = append(allErrs, err)
 	}
 
+	if err := validateAPISpec(spec, basePath); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
 	if err := validateConductorSpec(spec, basePath); err != nil {
 		allErrs = append(allErrs, err...)
 	}
@@ -221,8 +230,25 @@ func (r *Ironic) ValidateDelete() (admission.Warnings, error) {
 	return nil, nil
 }
 
+func validateAPISpec(spec *IronicSpecCore, basePath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	// validate the service override key is valid
+	allErrs = append(allErrs, service.ValidateRoutedOverrides(
+		basePath.Child("ironicAPI").Child("override").Child("service"),
+		spec.IronicAPI.Override.Service)...)
+
+	return allErrs
+}
+
 func validateInspectorSpec(spec *IronicSpecCore, basePath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
+
+	// validate the service override key is valid
+	allErrs = append(allErrs, service.ValidateRoutedOverrides(
+		basePath.Child("ironicInspector").Child("override").Child("service"),
+		spec.IronicInspector.Override.Service)...)
+
 	fieldPath := basePath.Child("ironicInspector").Child("dhcpRanges")
 
 	// Validate DHCP ranges

go.mod

@@ -90,3 +90,5 @@ replace github.com/openstack-k8s-operators/ironic-operator/api => ./api
 // mschuppert: map to latest commit from release-4.13 tag
 // must consistent within modules and service operators
 replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 //allow-merging
+
+replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/stuggi/lib-common/modules/common v0.0.0-20240508083919-01ef8c1d28c5

go.sum

@@ -78,8 +78,6 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240429104248-2
 github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240429104248-25176c735750/go.mod h1:QN2DJpfEc+mbvvfhoCuJ/UhQzvw12Mf+8nS0QX1HGIg=
 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240429164853-7e1e3b111ee9 h1:aS7xUxC/uOXfw0T4ARTu0G1qb6eJ0WnB2JRv9donPOE=
 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240429164853-7e1e3b111ee9/go.mod h1:Y/ge/l24phVaJb9S8mYRjtnDkohFkX/KEOUXLArcyvQ=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6 h1:WLsG3Ko+phW5xZJjncypLWGASoLqKrt05qN9Zxsad6g=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240429052447-09a614506ca6 h1:/mhzQQ9FF70z00zZD7dpgOoNXvEu9q68oob3oAiJW08=
 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:mrRNYeg8jb1zgGsufpN1/IB3sdbaST8btTBLwQ+taaA=
 github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240429052447-09a614506ca6 h1:Xx8uGcklRNHgBKTftNcK/Ob3qxiKwxUf5fVjtWCvOHI=
@@ -104,6 +102,8 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stuggi/lib-common/modules/common v0.0.0-20240508083919-01ef8c1d28c5 h1:RwBWawOf7fLsojvTdCeCZ+O5k3VqOqoX5f7PRQfa9lY=
+github.com/stuggi/lib-common/modules/common v0.0.0-20240508083919-01ef8c1d28c5/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=

tests/functional/ironic_controller_test.go

@@ -18,17 +18,21 @@ package functional_test
 
 import (
 	"fmt"
+	"os"
 
 	. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
 	. "github.com/onsi/gomega"    //revive:disable:dot-imports
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	//revive:disable-next-line:dot-imports
+
 	. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
 
 	ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	mariadb_test "github.com/openstack-k8s-operators/mariadb-operator/api/test/helpers"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -321,3 +325,74 @@ var _ = Describe("Ironic controller", func() {
 	})
 
 })
+
+var _ = Describe("Ironic Webhook", func() {
+
+	BeforeEach(func() {
+		err := os.Setenv("OPERATOR_TEMPLATES", "../../templates")
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("rejects with wrong IronicAPI service override endpoint type", func() {
+		spec := GetDefaultIronicSpec()
+		apiSpec := GetDefaultIronicAPISpec()
+		apiSpec["override"] = map[string]interface{}{
+			"service": map[string]interface{}{
+				"internal": map[string]interface{}{},
+				"wrooong":  map[string]interface{}{},
+			},
+		}
+		spec["ironicAPI"] = apiSpec
+
+		raw := map[string]interface{}{
+			"apiVersion": "ironic.openstack.org/v1beta1",
+			"kind":       "Ironic",
+			"metadata": map[string]interface{}{
+				"name":      ironicNames.IronicName.Name,
+				"namespace": ironicNames.IronicName.Namespace,
+			},
+			"spec": spec,
+		}
+
+		unstructuredObj := &unstructured.Unstructured{Object: raw}
+		_, err := controllerutil.CreateOrPatch(
+			th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(
+			ContainSubstring(
+				"invalid: spec.ironicAPI.override.service[wrooong]: " +
+					"Invalid value: \"wrooong\": invalid endpoint type: wrooong"),
+		)
+	})
+	It("rejects with wrong IronicInspector service override endpoint type", func() {
+		spec := GetDefaultIronicSpec()
+		apiSpec := GetDefaultIronicInspectorSpec()
+		apiSpec["override"] = map[string]interface{}{
+			"service": map[string]interface{}{
+				"internal": map[string]interface{}{},
+				"wrooong":  map[string]interface{}{},
+			},
+		}
+		spec["ironicInspector"] = apiSpec
+
+		raw := map[string]interface{}{
+			"apiVersion": "ironic.openstack.org/v1beta1",
+			"kind":       "Ironic",
+			"metadata": map[string]interface{}{
+				"name":      ironicNames.IronicName.Name,
+				"namespace": ironicNames.IronicName.Namespace,
+			},
+			"spec": spec,
+		}
+
+		unstructuredObj := &unstructured.Unstructured{Object: raw}
+		_, err := controllerutil.CreateOrPatch(
+			th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(
+			ContainSubstring(
+				"invalid: spec.ironicInspector.override.service[wrooong]: " +
+					"Invalid value: \"wrooong\": invalid endpoint type: wrooong"),
+		)
+	})
+})

tests/functional/suite_test.go

@@ -117,6 +117,13 @@ var _ = BeforeSuite(func() {
 			routev1CRDs,
 		},
 		ErrorIfCRDPathMissing: true,
+		WebhookInstallOptions: envtest.WebhookInstallOptions{
+			Paths: []string{filepath.Join("..", "..", "config", "webhook")},
+			// NOTE(gibi): if localhost is resolved to ::1 (ipv6) then starting
+			// the webhook fails as it try to parse the address as ipv4 and
+			// failing on the colons in ::1
+			LocalServingHost: "127.0.0.1",
+		},
 	}
 
 	// cfg is defined in this file globally.
@@ -175,6 +182,9 @@ var _ = BeforeSuite(func() {
 	kclient, err := kubernetes.NewForConfig(cfg)
 	Expect(err).ToNot(HaveOccurred(), "failed to create kclient")
 
+	err = (&ironicv1.Ironic{}).SetupWebhookWithManager(k8sManager)
+	Expect(err).NotTo(HaveOccurred())
+
 	err = (&controllers.IronicNeutronAgentReconciler{
 		Client:  k8sManager.GetClient(),
 		Scheme:  k8sManager.GetScheme(),