diff --git a/api/bases/placement.openstack.org_placementapis.yaml b/api/bases/placement.openstack.org_placementapis.yaml index e3616705..15867089 100644 --- a/api/bases/placement.openstack.org_placementapis.yaml +++ b/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/ - . TODO: -> implement' + description: DefaultConfigOverwrite - interface to overwrite default + config files like policy.yaml. type: object networkAttachments: description: NetworkAttachments is a list of NetworkAttachment resource diff --git a/api/v1beta1/placementapi_types.go b/api/v1beta1/placementapi_types.go index c848b59b..bc257edf 100644 --- a/api/v1beta1/placementapi_types.go +++ b/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/ . - // TODO: -> implement + // DefaultConfigOverwrite - interface to overwrite default config files like policy.yaml. DefaultConfigOverwrite map[string]string `json:"defaultConfigOverwrite,omitempty"` // +kubebuilder:validation:Optional diff --git a/api/v1beta1/placementapi_webhook.go b/api/v1beta1/placementapi_webhook.go index eac2f3f5..88159201 100644 --- a/api/v1beta1/placementapi_webhook.go +++ b/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 +} diff --git a/config/crd/bases/placement.openstack.org_placementapis.yaml b/config/crd/bases/placement.openstack.org_placementapis.yaml index e3616705..15867089 100644 --- a/config/crd/bases/placement.openstack.org_placementapis.yaml +++ b/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/ - . TODO: -> implement' + description: DefaultConfigOverwrite - interface to overwrite default + config files like policy.yaml. type: object networkAttachments: description: NetworkAttachments is a list of NetworkAttachment resource diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index 2f379c0d..cbc1c34f 100644 --- a/controllers/placementapi_controller.go +++ b/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//.conf.d // all other files get placed into /etc/ 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 diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index 210cc117..b815cee2 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/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": [ diff --git a/templates/placementapi/config/placement.conf b/templates/placementapi/config/placement.conf index aa69f5d0..5ac4783f 100644 --- a/templates/placementapi/config/placement.conf +++ b/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 diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index a3a5f008..a31dab44 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/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() { diff --git a/tests/functional/placementapi_webhook_test.go b/tests/functional/placementapi_webhook_test.go index 10abd848..5851be7d 100644 --- a/tests/functional/placementapi_webhook_test.go +++ b/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", + ), + ) + }) + })