From 2143d77b876fe3ed4833be9a2acc95d0dad6293c Mon Sep 17 00:00:00 2001 From: Kirsten Laskoski Date: Sun, 24 Nov 2024 09:59:20 -0500 Subject: [PATCH] webhook: add validating unit tests --- pkg/webhook/validatingwebhook.go | 57 ++++-- pkg/webhook/validatingwebhook_test.go | 280 ++++++++++++++++++++++++++ 2 files changed, 322 insertions(+), 15 deletions(-) create mode 100644 pkg/webhook/validatingwebhook_test.go diff --git a/pkg/webhook/validatingwebhook.go b/pkg/webhook/validatingwebhook.go index 82be43857..2d6d26e75 100644 --- a/pkg/webhook/validatingwebhook.go +++ b/pkg/webhook/validatingwebhook.go @@ -5,6 +5,7 @@ import ( "github.com/golang/glog" "github.com/openshift-kni/eco-goinfra/pkg/clients" + "github.com/openshift-kni/eco-goinfra/pkg/msg" "golang.org/x/net/context" admregv1 "k8s.io/api/admissionregistration/v1" @@ -24,16 +25,28 @@ type ValidatingConfigurationBuilder struct { // errorMsg is processed before ValidatingWebhookConfiguration object is created. errorMsg string // apiClient opens api connection to the cluster. - apiClient *clients.Settings + apiClient goclient.Client } // PullValidatingConfiguration pulls existing ValidatingWebhookConfiguration from cluster. -func PullValidatingConfiguration(apiClient *clients.Settings, name string) ( - *ValidatingConfigurationBuilder, error) { +func PullValidatingConfiguration(apiClient *clients.Settings, name string) (*ValidatingConfigurationBuilder, error) { glog.V(100).Infof("Pulling existing ValidatingWebhookConfiguration name %s from cluster", name) + if apiClient == nil { + glog.V(100).Infof("The ValidatingWebhookConfiguration apiClient is nil") + + return nil, fmt.Errorf("validatingWebhookConfiguration 'apiClient' cannot be nil") + } + + err := apiClient.AttachScheme(admregv1.AddToScheme) + if err != nil { + glog.V(100).Infof("Failed to add admissionregistration v1 scheme to client schemes") + + return nil, err + } + builder := &ValidatingConfigurationBuilder{ - apiClient: apiClient, + apiClient: apiClient.Client, Definition: &admregv1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -44,11 +57,11 @@ func PullValidatingConfiguration(apiClient *clients.Settings, name string) ( if name == "" { glog.V(100).Infof("The name of the ValidatingWebhookConfiguration is empty") - return nil, fmt.Errorf("ValidatingWebhookConfiguration 'name' cannot be empty") + return nil, fmt.Errorf("validatingWebhookConfiguration 'name' cannot be empty") } if !builder.Exists() { - return nil, fmt.Errorf("ValidatingWebhookConfiguration object %s does not exist", name) + return nil, fmt.Errorf("validatingWebhookConfiguration object %s does not exist", name) } builder.Definition = builder.Object @@ -71,7 +84,7 @@ func (builder *ValidatingConfigurationBuilder) Exists() bool { // Get returns ValidatingWebhookConfiguration object. func (builder *ValidatingConfigurationBuilder) Get() (*admregv1.ValidatingWebhookConfiguration, error) { if valid, err := builder.validate(); !valid { - return &admregv1.ValidatingWebhookConfiguration{}, err + return nil, err } validatingWebhookConfiguration := &admregv1.ValidatingWebhookConfiguration{} @@ -82,7 +95,7 @@ func (builder *ValidatingConfigurationBuilder) Get() (*admregv1.ValidatingWebhoo if err != nil { glog.V(100).Infof("Failed to get ValidatingWebhookConfiguration %s: %v", builder.Definition.Name, err) - return &admregv1.ValidatingWebhookConfiguration{}, err + return nil, err } return validatingWebhookConfiguration, nil @@ -97,8 +110,7 @@ func (builder *ValidatingConfigurationBuilder) Delete() (*ValidatingConfiguratio glog.V(100).Infof("Deleting the ValidatingWebhookConfiguration %s", builder.Definition.Name) if !builder.Exists() { - glog.V(100).Infof("ValidatingWebhookConfiguration %s cannot be deleted"+ - " because it does not exist", + glog.V(100).Infof("ValidatingWebhookConfiguration %s cannot be deleted because it does not exist", builder.Definition.Name) builder.Object = nil @@ -107,9 +119,8 @@ func (builder *ValidatingConfigurationBuilder) Delete() (*ValidatingConfiguratio } err := builder.apiClient.Delete(context.TODO(), builder.Definition) - if err != nil { - return builder, fmt.Errorf("can not delete ValidatingWebhookConfiguration: %w", err) + return builder, err } builder.Object = nil @@ -126,18 +137,28 @@ func (builder *ValidatingConfigurationBuilder) Update() (*ValidatingConfiguratio glog.V(100).Infof("Updating ValidatingWebhookConfiguration %s", builder.Definition.Name) + if !builder.Exists() { + glog.V(100).Infof("Cannot update ValidatingWebhookConfiguration %s as it does not exist", builder.Definition.Name) + + return builder, fmt.Errorf("cannot update non-existent validatingWebhookConfiguration") + } + + builder.Definition.ResourceVersion = builder.Object.ResourceVersion err := builder.apiClient.Update(context.TODO(), builder.Definition) - if err == nil { - builder.Object = builder.Definition + + if err != nil { + return builder, err } + builder.Object = builder.Definition + return builder, err } // validate will check that the builder and builder definition are properly initialized before // accessing any member fields. func (builder *ValidatingConfigurationBuilder) validate() (bool, error) { - resourceCRD := "ValidatingWebhookConfiguration" + resourceCRD := "validatingWebhookConfiguration" if builder == nil { glog.V(100).Infof("The %s builder is uninitialized", resourceCRD) @@ -145,6 +166,12 @@ func (builder *ValidatingConfigurationBuilder) validate() (bool, error) { return false, fmt.Errorf("error: received nil %s builder", resourceCRD) } + if builder.Definition == nil { + glog.V(100).Infof("The %s is undefined", resourceCRD) + + return false, fmt.Errorf(msg.UndefinedCrdObjectErrString(resourceCRD)) + } + if builder.apiClient == nil { glog.V(100).Infof("The %s builder apiclient is nil", resourceCRD) diff --git a/pkg/webhook/validatingwebhook_test.go b/pkg/webhook/validatingwebhook_test.go new file mode 100644 index 000000000..f3704ff62 --- /dev/null +++ b/pkg/webhook/validatingwebhook_test.go @@ -0,0 +1,280 @@ +package webhook + +import ( + "fmt" + "testing" + + "github.com/openshift-kni/eco-goinfra/pkg/clients" + "github.com/stretchr/testify/assert" + admregv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +const defaultValidatingName = "test-validating-webhook-configuration" + +func TestPullValidatingConfiguration(t *testing.T) { + testCases := []struct { + name string + addToRuntimeObjects bool + client bool + expectedError error + }{ + { + name: defaultValidatingName, + addToRuntimeObjects: true, + client: true, + expectedError: nil, + }, + { + name: "", + addToRuntimeObjects: true, + client: true, + expectedError: fmt.Errorf("validatingWebhookConfiguration 'name' cannot be empty"), + }, + { + name: defaultValidatingName, + addToRuntimeObjects: false, + client: true, + expectedError: fmt.Errorf("validatingWebhookConfiguration object %s does not exist", defaultValidatingName), + }, + { + name: defaultValidatingName, + addToRuntimeObjects: true, + client: false, + expectedError: fmt.Errorf("validatingWebhookConfiguration 'apiClient' cannot be nil"), + }, + } + + for _, testCase := range testCases { + var ( + runtimeObjects []runtime.Object + testSettings *clients.Settings + ) + + if testCase.addToRuntimeObjects { + runtimeObjects = append(runtimeObjects, buildDummyValidatingConfiguration(defaultValidatingName)) + } + + if testCase.client { + testSettings = clients.GetTestClients(clients.TestClientParams{ + K8sMockObjects: runtimeObjects, + SchemeAttachers: testSchemes, + }) + } + + testBuilder, err := PullValidatingConfiguration(testSettings, testCase.name) + assert.Equal(t, testCase.expectedError, err) + + if testCase.expectedError == nil { + assert.Equal(t, testCase.name, testBuilder.Definition.Name) + } + } +} + +func TestValidatingConfigurationExists(t *testing.T) { + testCases := []struct { + testBuilder *ValidatingConfigurationBuilder + exists bool + }{ + { + testBuilder: newValidatingConfigurationBuilder(buildTestClientWithDummyValidatingConfiguration()), + exists: true, + }, + { + testBuilder: newValidatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + exists: false, + }, + } + + for _, testCase := range testCases { + exists := testCase.testBuilder.Exists() + assert.Equal(t, testCase.exists, exists) + } +} + +func TestValidatingConfigurationGet(t *testing.T) { + testCases := []struct { + testBuilder *ValidatingConfigurationBuilder + expectedError string + }{ + { + testBuilder: newValidatingConfigurationBuilder(buildTestClientWithDummyValidatingConfiguration()), + expectedError: "", + }, + { + testBuilder: newValidatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + expectedError: fmt.Sprintf("validatingwebhookconfigurations.admissionregistration.k8s.io \"%s\" not found", + defaultValidatingName), + }, + } + + for _, testCase := range testCases { + validatingWebhookConfiguration, err := testCase.testBuilder.Get() + + if testCase.expectedError == "" { + assert.Nil(t, err) + assert.Equal(t, defaultValidatingName, validatingWebhookConfiguration.Name) + } else { + assert.EqualError(t, err, testCase.expectedError) + } + } +} + +func TestValidatingConfigurationDelete(t *testing.T) { + testCases := []struct { + testBuilder *ValidatingConfigurationBuilder + expectedErr error + }{ + { + testBuilder: newValidatingConfigurationBuilder(buildTestClientWithDummyValidatingConfiguration()), + expectedErr: nil, + }, + { + testBuilder: newValidatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + expectedErr: nil, + }, + } + + for _, testCase := range testCases { + testBuilder, err := testCase.testBuilder.Delete() + assert.Equal(t, testCase.expectedErr, err) + + if testCase.expectedErr == nil { + assert.Equal(t, defaultValidatingName, testBuilder.Definition.Name) + } + } +} + +func TestValidatingConfigurationUpdate(t *testing.T) { + testCases := []struct { + testBuilder *ValidatingConfigurationBuilder + expectedErr error + }{ + { + testBuilder: newValidatingConfigurationBuilder(buildTestClientWithDummyValidatingConfiguration()), + expectedErr: nil, + }, + { + testBuilder: newValidatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + expectedErr: fmt.Errorf("cannot update non-existent validatingWebhookConfiguration"), + }, + } + + for _, testCase := range testCases { + assert.Empty(t, testCase.testBuilder.Definition.Webhooks) + + testCase.testBuilder.Definition.Webhooks = []admregv1.ValidatingWebhook{{}} + + testBuilder, err := testCase.testBuilder.Update() + assert.Equal(t, testCase.expectedErr, err) + + if testCase.expectedErr == nil { + assert.Len(t, testBuilder.Object.Webhooks, 1) + } + } +} + +func TestValidatingConfigurationValidate(t *testing.T) { + testCases := []struct { + builderNil bool + definitionNil bool + apiClientNil bool + errorMsg string + expectedError error + }{ + { + builderNil: false, + definitionNil: false, + apiClientNil: false, + errorMsg: "", + expectedError: nil, + }, + { + builderNil: true, + definitionNil: false, + apiClientNil: false, + errorMsg: "", + expectedError: fmt.Errorf("error: received nil validatingWebhookConfiguration builder"), + }, + { + builderNil: false, + definitionNil: true, + apiClientNil: false, + errorMsg: "", + expectedError: fmt.Errorf("can not redefine the undefined validatingWebhookConfiguration"), + }, + { + builderNil: false, + definitionNil: false, + apiClientNil: true, + errorMsg: "", + expectedError: fmt.Errorf("validatingWebhookConfiguration builder cannot have nil apiClient"), + }, + { + builderNil: false, + definitionNil: false, + apiClientNil: false, + errorMsg: "test error", + expectedError: fmt.Errorf("test error"), + }, + } + + for _, testCase := range testCases { + testBuilder := newValidatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})) + + if testCase.builderNil { + testBuilder = nil + } + + if testCase.definitionNil { + testBuilder.Definition = nil + } + + if testCase.apiClientNil { + testBuilder.apiClient = nil + } + + if testCase.errorMsg != "" { + testBuilder.errorMsg = testCase.errorMsg + } + + valid, err := testBuilder.validate() + + assert.Equal(t, testCase.expectedError, err) + assert.Equal(t, testCase.expectedError == nil, valid) + } +} + +// buildDummyValidatingConfiguration returns a new ValidatingWebhookConfiguration with the given name. +func buildDummyValidatingConfiguration(name string) *admregv1.ValidatingWebhookConfiguration { + return &admregv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} + +// buildTestClientWithDummyValidatingConfiguration returns a new client with a mock ValidatingWebhookConfiguration +// object, using the default name. +func buildTestClientWithDummyValidatingConfiguration() *clients.Settings { + return clients.GetTestClients(clients.TestClientParams{ + K8sMockObjects: []runtime.Object{ + buildDummyValidatingConfiguration(defaultValidatingName), + }, + SchemeAttachers: testSchemes, + }) +} + +// newValidatingConfigurationBuilder returns a ValidatingConfigurationBuilder with the default name and the provided +// client. +func newValidatingConfigurationBuilder(apiClient *clients.Settings) *ValidatingConfigurationBuilder { + return &ValidatingConfigurationBuilder{ + apiClient: apiClient.Client, + Definition: &admregv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultValidatingName, + }, + }, + } +}