Add missing logic for DefaultConfigOverwrite

api/bases/placement.openstack.org_placementapis.yaml

@@ -73,10 +73,8 @@ spec:
               defaultConfigOverwrite:
                 additionalProperties:
                   type: string
-                description: 'ConfigOverwrite - interface to overwrite default config
-                  files like e.g. policy.json. But can also be used to add additional
-                  files. Those get added to the service config dir in /etc/<service>
-                  . TODO: -> implement'
+                description: DefaultConfigOverwrite - interface to overwrite default
+                  config files like policy.yaml.
                 type: object
               networkAttachments:
                 description: NetworkAttachments is a list of NetworkAttachment resource

api/v1beta1/placementapi_types.go

@@ -94,9 +94,7 @@ type PlacementAPISpec struct {
 	CustomServiceConfig string `json:"customServiceConfig"`
 
 	// +kubebuilder:validation:Optional
-	// ConfigOverwrite - interface to overwrite default config files like e.g. policy.json.
-	// But can also be used to add additional files. Those get added to the service config dir in /etc/<service> .
-	// TODO: -> implement
+	// DefaultConfigOverwrite - interface to overwrite default config files like policy.yaml.
 	DefaultConfigOverwrite map[string]string `json:"defaultConfigOverwrite,omitempty"`
 
 	// +kubebuilder:validation:Optional

api/v1beta1/placementapi_webhook.go

@@ -22,7 +22,12 @@ limitations under the License.
 package v1beta1
 
 import (
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -78,15 +83,31 @@ var _ webhook.Validator = &PlacementAPI{}
 func (r *PlacementAPI) ValidateCreate() error {
 	placementapilog.Info("validate create", "name", r.Name)
 
-	// TODO(user): fill in your validation logic upon object creation.
+	errors := r.Spec.ValidateCreate(field.NewPath("spec"))
+	if len(errors) != 0 {
+		placementapilog.Info("validation failed", "name", r.Name)
+		return apierrors.NewInvalid(
+			schema.GroupKind{Group: "placement.openstack.org", Kind: "PlacementAPI"},
+			r.Name, errors)
+	}
 	return nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *PlacementAPI) ValidateUpdate(old runtime.Object) error {
 	placementapilog.Info("validate update", "name", r.Name)
+	oldPlacement, ok := old.(*PlacementAPI)
+	if !ok || oldPlacement == nil {
+		return apierrors.NewInternalError(fmt.Errorf("unable to convert existing object"))
+	}
 
-	// TODO(user): fill in your validation logic upon object update.
+	errors := r.Spec.ValidateUpdate(oldPlacement.Spec, field.NewPath("spec"))
+	if len(errors) != 0 {
+		placementapilog.Info("validation failed", "name", r.Name)
+		return apierrors.NewInvalid(
+			schema.GroupKind{Group: "placement.openstack.org", Kind: "PlacementAPI"},
+			r.Name, errors)
+	}
 	return nil
 }
 
@@ -97,3 +118,30 @@ func (r *PlacementAPI) ValidateDelete() error {
 	// TODO(user): fill in your validation logic upon object deletion.
 	return nil
 }
+
+func (r PlacementAPISpec) ValidateCreate(basePath *field.Path) field.ErrorList {
+	return r.ValidateDefaultConfigOverwrite(basePath)
+}
+
+func (r PlacementAPISpec) ValidateUpdate(old PlacementAPISpec, basePath *field.Path) field.ErrorList {
+	return r.ValidateDefaultConfigOverwrite(basePath)
+}
+
+func (r PlacementAPISpec) ValidateDefaultConfigOverwrite(
+	basePath *field.Path,
+) field.ErrorList {
+	var errors field.ErrorList
+	for requested := range r.DefaultConfigOverwrite {
+		if requested != "policy.yaml" {
+			errors = append(
+				errors,
+				field.Invalid(
+					basePath.Child("defaultConfigOverwrite"),
+					requested,
+					"Only the following keys are valid: policy.yaml",
+				),
+			)
+		}
+	}
+	return errors
+}

config/crd/bases/placement.openstack.org_placementapis.yaml

@@ -73,10 +73,8 @@ spec:
               defaultConfigOverwrite:
                 additionalProperties:
                   type: string
-                description: 'ConfigOverwrite - interface to overwrite default config
-                  files like e.g. policy.json. But can also be used to add additional
-                  files. Those get added to the service config dir in /etc/<service>
-                  . TODO: -> implement'
+                description: DefaultConfigOverwrite - interface to overwrite default
+                  config files like policy.yaml.
                 type: object
               networkAttachments:
                 description: NetworkAttachments is a list of NetworkAttachment resource

controllers/placementapi_controller.go

@@ -1175,7 +1175,6 @@ func (r *PlacementAPIReconciler) ensureDeployment(
 }
 
 // generateServiceConfigMaps - create create configmaps which hold scripts and service configuration
-// TODO add DefaultConfigOverwrite
 func (r *PlacementAPIReconciler) generateServiceConfigMaps(
 	ctx context.Context,
 	h *helper.Helper,
@@ -1195,7 +1194,6 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps(
 	// customData hold any customization for the service.
 	// custom.conf is going to /etc/<service>/<service>.conf.d
 	// all other files get placed into /etc/<service> to allow overwrite of e.g. policy.json
-	// TODO: make sure custom.conf can not be overwritten
 	customData := map[string]string{common.CustomServiceConfigFileName: instance.Spec.CustomServiceConfig}
 	for key, data := range instance.Spec.DefaultConfigOverwrite {
 		customData[key] = data

templates/placementapi/config/placement-api-config.json

@@ -40,6 +40,13 @@
             "perm": "0400",
             "optional": true,
             "merge": true
+        },
+        {
+            "source": "/var/lib/openstack/config/policy.yaml",
+            "dest": "/etc/placement/policy.yaml",
+            "owner": "placement",
+            "perm": "0600",
+            "optional": true
         }
     ],
     "permissions": [

templates/placementapi/config/placement.conf

@@ -24,3 +24,6 @@ www_authenticate_uri = {{ .KeystonePublicURL }}
 auth_url = {{ .KeystoneInternalURL }}
 auth_type = password
 interface = internal
+
+[oslo_policy]
+policy_file=/etc/placement/policy.yaml

tests/functional/placementapi_controller_test.go

@@ -226,7 +226,12 @@ var _ = Describe("PlacementAPI controller", func() {
 		var keystoneAPI *keystonev1.KeystoneAPI
 
 		BeforeEach(func() {
-			DeferCleanup(th.DeleteInstance, CreatePlacementAPI(names.PlacementAPIName, GetDefaultPlacementAPISpec()))
+			spec := GetDefaultPlacementAPISpec()
+			spec["customServiceConfig"] = "foo = bar"
+			spec["defaultConfigOverwrite"] = map[string]interface{}{
+				"policy.yaml": "\"placement:resource_providers:list\": \"!\"",
+			}
+			DeferCleanup(th.DeleteInstance, CreatePlacementAPI(names.PlacementAPIName, spec))
 			DeferCleanup(
 				k8sClient.Delete, ctx, CreatePlacementAPISecret(namespace, SecretName))
 			keystoneAPIName := keystone.CreateKeystoneAPI(namespace)
@@ -242,19 +247,28 @@ var _ = Describe("PlacementAPI controller", func() {
 				corev1.ConditionTrue,
 			)
 		})
-		It("should create a ConfigMap for placement.conf", func() {
+		It("should create a configuration Secret", func() {
 			cm := th.GetSecret(names.ConfigMapName)
 
-			Expect(cm.Data["placement.conf"]).Should(
+			conf := cm.Data["placement.conf"]
+			Expect(conf).Should(
 				ContainSubstring("auth_url = %s", keystoneAPI.Status.APIEndpoints["internal"]))
-			Expect(cm.Data["placement.conf"]).Should(
+			Expect(conf).Should(
 				ContainSubstring("www_authenticate_uri = %s", keystoneAPI.Status.APIEndpoints["public"]))
-			Expect(cm.Data["placement.conf"]).Should(
+			Expect(conf).Should(
 				ContainSubstring("username = placement"))
-			Expect(cm.Data["placement.conf"]).Should(
+			Expect(conf).Should(
 				ContainSubstring("password = 12345678"))
-			Expect(cm.Data["placement.conf"]).Should(
+			Expect(conf).Should(
 				ContainSubstring("connection = mysql+pymysql://placement:12345678@/placement"))
+
+			custom := cm.Data["custom.conf"]
+			Expect(custom).Should(ContainSubstring("foo = bar"))
+
+			policy := cm.Data["policy.yaml"]
+			Expect(policy).Should(
+				ContainSubstring("\"placement:resource_providers:list\": \"!\""))
+
 		})
 
 		It("creates service account, role and rolebindig", func() {

tests/functional/placementapi_webhook_test.go

@@ -17,11 +17,15 @@ limitations under the License.
 package functional_test
 
 import (
+	"errors"
 	"os"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	placementv1 "github.com/openstack-k8s-operators/placement-operator/api/v1beta1"
 )
@@ -68,4 +72,37 @@ var _ = Describe("PlacementAPI Webhook", func() {
 			))
 		})
 	})
+
+	It("rejects PlacementAPI with wrong defaultConfigOverwrite", func() {
+		spec := GetDefaultPlacementAPISpec()
+		spec["defaultConfigOverwrite"] = map[string]interface{}{
+			"policy.yaml":   "support",
+			"api-paste.ini": "not supported",
+		}
+		raw := map[string]interface{}{
+			"apiVersion": "placement.openstack.org/v1beta1",
+			"kind":       "PlacementAPI",
+			"metadata": map[string]interface{}{
+				"name":      placementApiName.Name,
+				"namespace": placementApiName.Namespace,
+			},
+			"spec": spec,
+		}
+		unstructuredObj := &unstructured.Unstructured{Object: raw}
+		_, err := controllerutil.CreateOrPatch(
+			ctx, k8sClient, unstructuredObj, func() error { return nil })
+
+		Expect(err).Should(HaveOccurred())
+		var statusError *k8s_errors.StatusError
+		Expect(errors.As(err, &statusError)).To(BeTrue())
+		Expect(statusError.ErrStatus.Details.Kind).To(Equal("PlacementAPI"))
+		Expect(statusError.ErrStatus.Message).To(
+			ContainSubstring(
+				"invalid: spec.defaultConfigOverwrite: " +
+					"Invalid value: \"api-paste.ini\": " +
+					"Only the following keys are valid: policy.yaml",
+			),
+		)
+	})
+
 })