Skip to content

Commit

Permalink
Add missing logic for DefaultConfigOverwrite
Browse files Browse the repository at this point in the history
We only want to support the policy.yaml file passthrough for
PlacementAPI

The kolla config files are modified to move the supported files in place.

Implements: OSPRH-2503
  • Loading branch information
gibizer committed Feb 8, 2024
1 parent 482655c commit a7e7d12
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 20 deletions.
6 changes: 2 additions & 4 deletions api/bases/placement.openstack.org_placementapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions api/v1beta1/placementapi_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 50 additions & 2 deletions api/v1beta1/placementapi_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
6 changes: 2 additions & 4 deletions config/crd/bases/placement.openstack.org_placementapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions templates/placementapi/config/placement-api-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
3 changes: 3 additions & 0 deletions templates/placementapi/config/placement.conf
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@ www_authenticate_uri = {{ .KeystonePublicURL }}
auth_url = {{ .KeystoneInternalURL }}
auth_type = password
interface = internal

[oslo_policy]
policy_file=/etc/placement/policy.yaml
28 changes: 21 additions & 7 deletions tests/functional/placementapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
37 changes: 37 additions & 0 deletions tests/functional/placementapi_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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",
),
)
})

})

0 comments on commit a7e7d12

Please sign in to comment.