Skip to content

Commit

Permalink
Allow webhook changes to the bastion enabled flag
Browse files Browse the repository at this point in the history
Signed-off-by: Tobias Giese <tobias.giese@daimler.com>
  • Loading branch information
tobiasgiese committed Dec 4, 2021
1 parent 244d31b commit aea3643
Show file tree
Hide file tree
Showing 8 changed files with 520 additions and 56 deletions.
40 changes: 13 additions & 27 deletions api/v1alpha4/openstackcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ package v1alpha4
import (
"reflect"

"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/builder"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"
Expand Down Expand Up @@ -65,40 +64,27 @@ func (r *OpenStackCluster) ValidateCreate() error {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (r *OpenStackCluster) ValidateUpdate(old runtime.Object) error {
func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) error {
var allErrs field.ErrorList

newOpenStackCluster, err := runtime.DefaultUnstructuredConverter.ToUnstructured(r)
if err != nil {
return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackCluster").GroupKind(), r.Name, field.ErrorList{
field.InternalError(nil, errors.Wrap(err, "failed to convert new OpenStackCluster to unstructured object")),
})
}
oldOpenStackCluster, err := runtime.DefaultUnstructuredConverter.ToUnstructured(old)
if err != nil {
return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackCluster").GroupKind(), r.Name, field.ErrorList{
field.InternalError(nil, errors.Wrap(err, "failed to convert old OpenStackCluster to unstructured object")),
})
}
old := oldRaw.(*OpenStackCluster)

if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
}

newOpenStackClusterSpec := newOpenStackCluster["spec"].(map[string]interface{})
oldOpenStackClusterSpec := oldOpenStackCluster["spec"].(map[string]interface{})

// get controlPlaneEndpoint, something like {"host":"", "port":""}
cpe := oldOpenStackClusterSpec["controlPlaneEndpoint"].(map[string]interface{})

// allow change only for the first time
host, ok := cpe["host"].(string)
if ok && len(host) == 0 {
delete(oldOpenStackClusterSpec, "controlPlaneEndpoint")
delete(newOpenStackClusterSpec, "controlPlaneEndpoint")
if old.Spec.ControlPlaneEndpoint.Host != "" {
old.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
r.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
}

// Allow changes to the bastion host enabled flag.
if r.Spec.Bastion != nil && old.Spec.Bastion != nil {
r.Spec.Bastion.Enabled = true
old.Spec.Bastion.Enabled = true
}

if !reflect.DeepEqual(oldOpenStackClusterSpec, newOpenStackClusterSpec) {
if !reflect.DeepEqual(old.Spec, r.Spec) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified"))
}

Expand Down
132 changes: 132 additions & 0 deletions api/v1alpha4/openstackcluster_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
Copyright 2021 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha4

import (
"testing"

. "github.com/onsi/gomega"
)

func TestOpenStackCluster_ValidateUpdate(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
oldTemplate *OpenStackCluster
newTemplate *OpenStackCluster
wantErr bool
}{
{
name: "OpenStackCluster.Spec.Bastion with immutable spec",
oldTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Instance: OpenStackMachineSpec{
CloudName: "foobar",
},
Enabled: false,
},
},
},
newTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Instance: OpenStackMachineSpec{
CloudName: "foobarbaz",
},
Enabled: true,
},
},
},
wantErr: true,
},
{
name: "OpenStackCluster.Spec.Bastion with mutable spec",
oldTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Enabled: false,
},
},
},
newTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Enabled: true,
},
},
},
wantErr: false,
},
{
name: "OpenStackCluster.Spec.IdentityRef with immutable spec",
oldTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
IdentityRef: &OpenStackIdentityReference{
Kind: "Secret",
},
CloudName: "foobar",
},
},
newTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
IdentityRef: &OpenStackIdentityReference{
Kind: "foobar",
},
},
},
wantErr: true,
},
{
name: "OpenStackCluster.Spec.IdentityRef with mutable spec",
oldTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
IdentityRef: &OpenStackIdentityReference{
Kind: "Secret",
},
CloudName: "foobar",
},
},
newTemplate: &OpenStackCluster{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
IdentityRef: &OpenStackIdentityReference{
Kind: "Secret",
},
},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := tt.newTemplate.ValidateUpdate(tt.oldTemplate)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(err).NotTo(HaveOccurred())
}
})
}
}
8 changes: 7 additions & 1 deletion api/v1alpha4/openstackclustertemplate_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,14 @@ func (r *OpenStackClusterTemplate) ValidateCreate() error {
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (r *OpenStackClusterTemplate) ValidateUpdate(oldRaw runtime.Object) error {
var allErrs field.ErrorList

old := oldRaw.(*OpenStackClusterTemplate)

// Allow changes to the bastion host enabled flag.
if r.Spec.Template.Spec.Bastion != nil && old.Spec.Template.Spec.Bastion != nil {
r.Spec.Template.Spec.Bastion.Enabled = true
old.Spec.Template.Spec.Bastion.Enabled = true
}

if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("OpenStackClusterTemplate", "spec", "template", "spec"), r, openStackClusterTemplateImmutableMsg),
Expand Down
108 changes: 108 additions & 0 deletions api/v1alpha4/openstackclustertemplate_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
Copyright 2021 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha4

import (
"testing"

. "github.com/onsi/gomega"
)

func TestOpenStackClusterTemplate_ValidateUpdate(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
oldTemplate *OpenStackClusterTemplate
newTemplate *OpenStackClusterTemplate
wantErr bool
}{
{
name: "OpenStackClusterTemplate with immutable spec",
oldTemplate: &OpenStackClusterTemplate{
Spec: OpenStackClusterTemplateSpec{
Template: OpenStackClusterTemplateResource{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Instance: OpenStackMachineSpec{
CloudName: "foobar",
},
Enabled: false,
},
},
},
},
},
newTemplate: &OpenStackClusterTemplate{
Spec: OpenStackClusterTemplateSpec{
Template: OpenStackClusterTemplateResource{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Instance: OpenStackMachineSpec{
CloudName: "foobarbaz",
},
Enabled: true,
},
},
},
},
},
wantErr: true,
},
{
name: "OpenStackClusterTemplate with mutable spec",
oldTemplate: &OpenStackClusterTemplate{
Spec: OpenStackClusterTemplateSpec{
Template: OpenStackClusterTemplateResource{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Enabled: false,
},
},
},
},
},
newTemplate: &OpenStackClusterTemplate{
Spec: OpenStackClusterTemplateSpec{
Template: OpenStackClusterTemplateResource{
Spec: OpenStackClusterSpec{
CloudName: "foobar",
Bastion: &Bastion{
Enabled: true,
},
},
},
},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := tt.newTemplate.ValidateUpdate(tt.oldTemplate)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(err).NotTo(HaveOccurred())
}
})
}
}
40 changes: 13 additions & 27 deletions api/v1beta1/openstackcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ package v1beta1
import (
"reflect"

"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/builder"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"
Expand Down Expand Up @@ -65,40 +64,27 @@ func (r *OpenStackCluster) ValidateCreate() error {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (r *OpenStackCluster) ValidateUpdate(old runtime.Object) error {
func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) error {
var allErrs field.ErrorList

newOpenStackCluster, err := runtime.DefaultUnstructuredConverter.ToUnstructured(r)
if err != nil {
return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackCluster").GroupKind(), r.Name, field.ErrorList{
field.InternalError(nil, errors.Wrap(err, "failed to convert new OpenStackCluster to unstructured object")),
})
}
oldOpenStackCluster, err := runtime.DefaultUnstructuredConverter.ToUnstructured(old)
if err != nil {
return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackCluster").GroupKind(), r.Name, field.ErrorList{
field.InternalError(nil, errors.Wrap(err, "failed to convert old OpenStackCluster to unstructured object")),
})
}
old := oldRaw.(*OpenStackCluster)

if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
}

newOpenStackClusterSpec := newOpenStackCluster["spec"].(map[string]interface{})
oldOpenStackClusterSpec := oldOpenStackCluster["spec"].(map[string]interface{})

// get controlPlaneEndpoint, something like {"host":"", "port":""}
cpe := oldOpenStackClusterSpec["controlPlaneEndpoint"].(map[string]interface{})

// allow change only for the first time
host, ok := cpe["host"].(string)
if ok && len(host) == 0 {
delete(oldOpenStackClusterSpec, "controlPlaneEndpoint")
delete(newOpenStackClusterSpec, "controlPlaneEndpoint")
if old.Spec.ControlPlaneEndpoint.Host != "" {
old.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
r.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
}

// Allow changes to the bastion host enabled flag.
if r.Spec.Bastion != nil && old.Spec.Bastion != nil {
r.Spec.Bastion.Enabled = true
old.Spec.Bastion.Enabled = true
}

if !reflect.DeepEqual(oldOpenStackClusterSpec, newOpenStackClusterSpec) {
if !reflect.DeepEqual(old.Spec, r.Spec) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified"))
}

Expand Down
Loading

0 comments on commit aea3643

Please sign in to comment.