diff --git a/pkg/webhook/mutatingwebhook.go b/pkg/webhook/mutatingwebhook.go index 9894d3197..afc2ff2d5 100644 --- a/pkg/webhook/mutatingwebhook.go +++ b/pkg/webhook/mutatingwebhook.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,15 +25,28 @@ type MutatingConfigurationBuilder struct { // errorMsg is processed before MutatingWebhookConfiguration object is created. errorMsg string // apiClient opens api connection to the cluster. - apiClient *clients.Settings + apiClient goclient.Client } // PullMutatingConfiguration pulls existing MutatingWebhookConfiguration from cluster. func PullMutatingConfiguration(apiClient *clients.Settings, name string) (*MutatingConfigurationBuilder, error) { glog.V(100).Infof("Pulling existing MutatingWebhookConfiguration name %s from cluster", name) - builder := MutatingConfigurationBuilder{ - apiClient: apiClient, + if apiClient == nil { + glog.V(100).Infof("The MutatingWebhookConfiguration apiClient is nil") + + return nil, fmt.Errorf("mutatingWebhookConfiguration '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 := &MutatingConfigurationBuilder{ + apiClient: apiClient.Client, Definition: &admregv1.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -43,16 +57,16 @@ func PullMutatingConfiguration(apiClient *clients.Settings, name string) (*Mutat if name == "" { glog.V(100).Infof("The name of the MutatingWebhookConfiguration is empty") - builder.errorMsg = "MutatingWebhookConfiguration 'name' cannot be empty" + return nil, fmt.Errorf("mutatingWebhookConfiguration 'name' cannot be empty") } if !builder.Exists() { - return nil, fmt.Errorf("MutatingWebhook object %s does not exist", name) + return nil, fmt.Errorf("mutatingWebhookConfiguration object %s does not exist", name) } builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Exists checks whether the given MutatingWebhookConfiguration object exists in the cluster. @@ -70,7 +84,7 @@ func (builder *MutatingConfigurationBuilder) Exists() bool { // Get returns MutatingWebhookConfiguration object. func (builder *MutatingConfigurationBuilder) Get() (*admregv1.MutatingWebhookConfiguration, error) { if valid, err := builder.validate(); !valid { - return &admregv1.MutatingWebhookConfiguration{}, err + return nil, err } mutatingWebhookConfiguration := &admregv1.MutatingWebhookConfiguration{} @@ -79,9 +93,9 @@ func (builder *MutatingConfigurationBuilder) Get() (*admregv1.MutatingWebhookCon }, mutatingWebhookConfiguration) if err != nil { - glog.V(100).Infof("Failed to get MutatingConfigurationBuilder %s: %v", builder.Definition.Name, err) + glog.V(100).Infof("Failed to get MutatingWebhookConfiguration %s: %v", builder.Definition.Name, err) - return &admregv1.MutatingWebhookConfiguration{}, err + return nil, err } return mutatingWebhookConfiguration, err @@ -105,9 +119,8 @@ func (builder *MutatingConfigurationBuilder) Delete() (*MutatingConfigurationBui } err := builder.apiClient.Delete(context.TODO(), builder.Definition) - if err != nil { - return builder, fmt.Errorf("can not delete MutatingWebhookConfiguration: %w", err) + return builder, err } builder.Object = nil @@ -124,18 +137,28 @@ func (builder *MutatingConfigurationBuilder) Update() (*MutatingConfigurationBui glog.V(100).Infof("Updating MutatingWebhookConfiguration %s", builder.Definition.Name) + if !builder.Exists() { + glog.V(100).Infof("Cannot update MutatingWebhookConfiguration %s as it does not exist", builder.Definition.Name) + + return builder, fmt.Errorf("cannot update non-existent mutatingWebhookConfiguration") + } + + 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 *MutatingConfigurationBuilder) validate() (bool, error) { - resourceCRD := "MutatingWebhookConfiguration" + resourceCRD := "mutatingWebhookConfiguration" if builder == nil { glog.V(100).Infof("The %s builder is uninitialized", resourceCRD) @@ -143,10 +166,16 @@ func (builder *MutatingConfigurationBuilder) 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) - builder.errorMsg = fmt.Sprintf("%s builder cannot have nil apiClient", resourceCRD) + return false, fmt.Errorf("%s builder cannot have nil apiClient", resourceCRD) } if builder.errorMsg != "" { diff --git a/pkg/webhook/mutatingwebhook_test.go b/pkg/webhook/mutatingwebhook_test.go new file mode 100644 index 000000000..a8192b8f3 --- /dev/null +++ b/pkg/webhook/mutatingwebhook_test.go @@ -0,0 +1,267 @@ +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 defaultMutatingName = "test-mutating-webhook-configuration" + +var testSchemes = []clients.SchemeAttacher{ + admregv1.AddToScheme, +} + +func TestPullMutatingConfiguration(t *testing.T) { + testCases := []struct { + name string + addToRuntimeObjects bool + client bool + expectedError error + }{ + { + name: defaultMutatingName, + addToRuntimeObjects: true, + client: true, + expectedError: nil, + }, + { + name: "", + addToRuntimeObjects: true, + client: true, + expectedError: fmt.Errorf("mutatingWebhookConfiguration 'name' cannot be empty"), + }, + { + name: defaultMutatingName, + addToRuntimeObjects: false, + client: true, + expectedError: fmt.Errorf("mutatingWebhookConfiguration object %s does not exist", defaultMutatingName), + }, + { + name: defaultMutatingName, + addToRuntimeObjects: true, + client: false, + expectedError: fmt.Errorf("mutatingWebhookConfiguration 'apiClient' cannot be nil"), + }, + } + + for _, testCase := range testCases { + var ( + runtimeObjects []runtime.Object + testSettings *clients.Settings + ) + + if testCase.addToRuntimeObjects { + runtimeObjects = append(runtimeObjects, buildDummyMutatingConfiguration(defaultMutatingName)) + } + + if testCase.client { + testSettings = clients.GetTestClients(clients.TestClientParams{ + K8sMockObjects: runtimeObjects, + SchemeAttachers: testSchemes, + }) + } + + testBuilder, err := PullMutatingConfiguration(testSettings, testCase.name) + assert.Equal(t, testCase.expectedError, err) + + if testCase.expectedError == nil { + assert.Equal(t, testCase.name, testBuilder.Definition.Name) + } + } +} + +func TestMutatingConfigurationExists(t *testing.T) { + testCases := []struct { + testBuilder *MutatingConfigurationBuilder + exists bool + }{ + { + testBuilder: newMutatingConfigurationBuilder(buildTestClientWithDummyMutatingConfiguration()), + exists: true, + }, + { + testBuilder: newMutatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + exists: false, + }, + } + + for _, testCase := range testCases { + exists := testCase.testBuilder.Exists() + assert.Equal(t, testCase.exists, exists) + } +} + +func TestMutatingConfigurationGet(t *testing.T) { + testCases := []struct { + testBuilder *MutatingConfigurationBuilder + expectedError string + }{ + { + testBuilder: newMutatingConfigurationBuilder(buildTestClientWithDummyMutatingConfiguration()), + expectedError: "", + }, + { + testBuilder: newMutatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + expectedError: fmt.Sprintf("mutatingwebhookconfigurations.admissionregistration.k8s.io \"%s\" not found", + defaultMutatingName), + }, + } + + for _, testCase := range testCases { + mutatingWebhookConfiguration, err := testCase.testBuilder.Get() + + if testCase.expectedError == "" { + assert.Nil(t, err) + assert.Equal(t, defaultMutatingName, mutatingWebhookConfiguration.Name) + } else { + assert.EqualError(t, err, testCase.expectedError) + } + } +} + +func TestMutatingConfigurationDelete(t *testing.T) { + testCases := []struct { + testBuilder *MutatingConfigurationBuilder + expectedErr error + }{ + { + testBuilder: newMutatingConfigurationBuilder(buildTestClientWithDummyMutatingConfiguration()), + expectedErr: nil, + }, + { + testBuilder: newMutatingConfigurationBuilder(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, defaultMutatingName, testBuilder.Definition.Name) + } + } +} + +func TestMutatingConfigurationUpdate(t *testing.T) { + testCases := []struct { + testBuilder *MutatingConfigurationBuilder + expectedErr error + }{ + { + testBuilder: newMutatingConfigurationBuilder(buildTestClientWithDummyMutatingConfiguration()), + expectedErr: nil, + }, + { + testBuilder: newMutatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})), + expectedErr: fmt.Errorf("cannot update non-existent mutatingWebhookConfiguration"), + }, + } + + for _, testCase := range testCases { + assert.Empty(t, testCase.testBuilder.Definition.Webhooks) + + testCase.testBuilder.Definition.Webhooks = []admregv1.MutatingWebhook{{}} + + testBuilder, err := testCase.testBuilder.Update() + assert.Equal(t, testCase.expectedErr, err) + + if testCase.expectedErr == nil { + assert.Len(t, testBuilder.Object.Webhooks, 1) + } + } +} + +func TestMutatingConfigurationValidate(t *testing.T) { + testCases := []struct { + builderNil bool + definitionNil bool + apiClientNil bool + expectedError error + }{ + { + builderNil: false, + definitionNil: false, + apiClientNil: false, + expectedError: nil, + }, + { + builderNil: true, + definitionNil: false, + apiClientNil: false, + expectedError: fmt.Errorf("error: received nil mutatingWebhookConfiguration builder"), + }, + { + builderNil: false, + definitionNil: true, + apiClientNil: false, + expectedError: fmt.Errorf("can not redefine the undefined mutatingWebhookConfiguration"), + }, + { + builderNil: false, + definitionNil: false, + apiClientNil: true, + expectedError: fmt.Errorf("mutatingWebhookConfiguration builder cannot have nil apiClient"), + }, + } + + for _, testCase := range testCases { + testBuilder := newMutatingConfigurationBuilder(clients.GetTestClients(clients.TestClientParams{})) + + if testCase.builderNil { + testBuilder = nil + } + + if testCase.definitionNil { + testBuilder.Definition = nil + } + + if testCase.apiClientNil { + testBuilder.apiClient = nil + } + + valid, err := testBuilder.validate() + + assert.Equal(t, testCase.expectedError, err) + assert.Equal(t, testCase.expectedError == nil, valid) + } +} + +// buildDummyMutatingConfiguration returns a new MutatingWebhookConfiguration with the given name. +func buildDummyMutatingConfiguration(name string) *admregv1.MutatingWebhookConfiguration { + return &admregv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} + +// buildTestClientWithDummyMutatingConfiguration returns a new client with a mock MutatingWebhookConfiguration object, +// using the default name. +func buildTestClientWithDummyMutatingConfiguration() *clients.Settings { + return clients.GetTestClients(clients.TestClientParams{ + K8sMockObjects: []runtime.Object{ + buildDummyMutatingConfiguration(defaultMutatingName), + }, + SchemeAttachers: testSchemes, + }) +} + +// newMutatingConfigurationBuilder returns a MutatingConfigurationBuilder with the default name and the provided client. +func newMutatingConfigurationBuilder(apiClient *clients.Settings) *MutatingConfigurationBuilder { + return &MutatingConfigurationBuilder{ + apiClient: apiClient.Client, + Definition: &admregv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultMutatingName, + }, + }, + } +} diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go deleted file mode 100644 index d770c2c07..000000000 --- a/pkg/webhook/webhook_test.go +++ /dev/null @@ -1 +0,0 @@ -package webhook