Skip to content

Commit

Permalink
Check for valid service override endpoint type on create and update
Browse files Browse the repository at this point in the history
  • Loading branch information
stuggi committed May 8, 2024
1 parent 67bdcd6 commit b9fff2d
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 8 deletions.
2 changes: 2 additions & 0 deletions api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,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
3 changes: 1 addition & 2 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g=
github.com/onsi/gomega v1.33.0 h1:snPCflnZrpMsy94p4lXVEkHo12lmPnc3vY5XBbreexE=
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.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand All @@ -85,6 +83,7 @@ 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/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=
Expand Down
23 changes: 19 additions & 4 deletions api/v1beta1/placementapi_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package v1beta1
import (
"fmt"

"github.com/openstack-k8s-operators/lib-common/modules/common/service"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -126,19 +127,33 @@ func (r *PlacementAPI) ValidateDelete() (admission.Warnings, error) {
}

func (r PlacementAPISpec) ValidateCreate(basePath *field.Path) field.ErrorList {
return ValidateDefaultConfigOverwrite(basePath, r.DefaultConfigOverwrite)
return r.PlacementAPISpecCore.ValidateCreate(basePath)
}

func (r PlacementAPISpec) ValidateUpdate(old PlacementAPISpec, basePath *field.Path) field.ErrorList {
return ValidateDefaultConfigOverwrite(basePath, r.DefaultConfigOverwrite)
return r.PlacementAPISpecCore.ValidateCreate(basePath)
}

func (r PlacementAPISpecCore) ValidateCreate(basePath *field.Path) field.ErrorList {
return ValidateDefaultConfigOverwrite(basePath, r.DefaultConfigOverwrite)
var allErrs field.ErrorList

// validate the service override key is valid
allErrs = append(allErrs, service.ValidateRoutedOverrides(basePath.Child("override").Child("service"), r.Override.Service)...)

allErrs = append(allErrs, ValidateDefaultConfigOverwrite(basePath, r.DefaultConfigOverwrite)...)

return allErrs
}

func (r PlacementAPISpecCore) ValidateUpdate(old PlacementAPISpecCore, basePath *field.Path) field.ErrorList {
return ValidateDefaultConfigOverwrite(basePath, r.DefaultConfigOverwrite)
var allErrs field.ErrorList

// validate the service override key is valid
allErrs = append(allErrs, service.ValidateRoutedOverrides(basePath.Child("override").Child("service"), r.Override.Service)...)

allErrs = append(allErrs, ValidateDefaultConfigOverwrite(basePath, r.DefaultConfigOverwrite)...)

return allErrs
}

func ValidateDefaultConfigOverwrite(
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,5 @@ replace github.com/openstack-k8s-operators/placement-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
3 changes: 1 addition & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxC
github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
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=
Expand All @@ -102,6 +100,7 @@ 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/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=
Expand Down
85 changes: 85 additions & 0 deletions tests/functional/placementapi_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,18 @@ import (

. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
. "github.com/onsi/gomega" //revive:disable:dot-imports

//revive:disable-next-line:dot-imports
. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"

corev1 "k8s.io/api/core/v1"
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"

condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
placementv1 "github.com/openstack-k8s-operators/placement-operator/api/v1beta1"
)

Expand Down Expand Up @@ -105,4 +112,82 @@ var _ = Describe("PlacementAPI Webhook", func() {
)
})

It("rejects with wrong service override endpoint type", func() {
spec := GetDefaultPlacementAPISpec()
spec["override"] = map[string]interface{}{
"service": map[string]interface{}{
"internal": map[string]interface{}{},
"wrooong": map[string]interface{}{},
},
}

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(
th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(
ContainSubstring(
"invalid: spec.override.service[wrooong]: " +
"Invalid value: \"wrooong\": invalid endpoint type: wrooong"),
)
})

When("A PlacementAPI instance is updated with wrong service override endpoint", func() {
BeforeEach(func() {
DeferCleanup(k8sClient.Delete, ctx, CreatePlacementAPISecret(namespace, SecretName))
DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(namespace))

placementAPI := CreatePlacementAPI(names.PlacementAPIName, GetDefaultPlacementAPISpec())
DeferCleanup(
mariadb.DeleteDBService,
mariadb.CreateDBService(
namespace,
GetPlacementAPI(names.PlacementAPIName).Spec.DatabaseInstance,
corev1.ServiceSpec{
Ports: []corev1.ServicePort{{Port: 3306}},
},
),
)

mariadb.SimulateMariaDBDatabaseCompleted(names.MariaDBDatabaseName)
mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount)
th.SimulateJobSuccess(names.DBSyncJobName)
th.SimulateDeploymentReplicaReady(names.DeploymentName)
keystone.SimulateKeystoneServiceReady(names.KeystoneServiceName)
keystone.SimulateKeystoneEndpointReady(names.KeystoneEndpointName)
DeferCleanup(th.DeleteInstance, placementAPI)

th.ExpectCondition(
names.PlacementAPIName,
ConditionGetterFunc(PlacementConditionGetter),
condition.ReadyCondition,
corev1.ConditionTrue,
)
})
It("rejects update with wrong service override endpoint type", func() {
PlacementAPI := GetPlacementAPI(names.PlacementAPIName)
Expect(PlacementAPI).NotTo(BeNil())
if PlacementAPI.Spec.Override.Service == nil {
PlacementAPI.Spec.Override.Service = map[service.Endpoint]service.RoutedOverrideSpec{}
}
PlacementAPI.Spec.Override.Service["wrooong"] = service.RoutedOverrideSpec{}
err := k8sClient.Update(ctx, PlacementAPI)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(
ContainSubstring(
"invalid: spec.override.service[wrooong]: " +
"Invalid value: \"wrooong\": invalid endpoint type: wrooong"),
)
})
})
})

0 comments on commit b9fff2d

Please sign in to comment.