diff --git a/pkg/operator/resource/resourceapply/admissionregistration.go b/pkg/operator/resource/resourceapply/admissionregistration.go new file mode 100644 index 0000000000..75d9d82a93 --- /dev/null +++ b/pkg/operator/resource/resourceapply/admissionregistration.go @@ -0,0 +1,140 @@ +package resourceapply + +import ( + "context" + "fmt" + + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + admissionregistrationclientv1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1" + "k8s.io/klog/v2" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// ApplyMutatingWebhookConfiguration ensures the form of the specified +// mutatingwebhookconfiguration is present in the API. If it does not exist, +// it will be created. If it does exist, the metadata of the required +// mutatingwebhookconfiguration will be merged with the existing mutatingwebhookconfiguration +// and an update performed if the mutatingwebhookconfiguration spec and metadata differ from +// the previously required spec and metadata based on generation change. +func ApplyMutatingWebhookConfiguration(client admissionregistrationclientv1.MutatingWebhookConfigurationsGetter, recorder events.Recorder, + requiredOriginal *admissionregistrationv1.MutatingWebhookConfiguration, expectedGeneration int64) (*admissionregistrationv1.MutatingWebhookConfiguration, bool, error) { + + if requiredOriginal == nil { + return nil, false, fmt.Errorf("Unexpected nil instead of an object") + } + required := requiredOriginal.DeepCopy() + + existing, err := client.MutatingWebhookConfigurations().Get(context.TODO(), required.GetName(), metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + actual, err := client.MutatingWebhookConfigurations().Create(context.TODO(), required, metav1.CreateOptions{}) + reportCreateEvent(recorder, required, err) + if err != nil { + return nil, false, err + } + return actual, true, nil + } else if err != nil { + return nil, false, err + } + + modified := resourcemerge.BoolPtr(false) + existingCopy := existing.DeepCopy() + + resourcemerge.EnsureObjectMeta(modified, &existingCopy.ObjectMeta, required.ObjectMeta) + if !*modified && existingCopy.GetGeneration() == expectedGeneration { + return existingCopy, false, nil + } + // at this point we know that we're going to perform a write. We're just trying to get the object correct + toWrite := existingCopy // shallow copy so the code reads easier + copyMutatingWebhookCABundle(existing, required) + toWrite.Webhooks = required.Webhooks + + klog.V(4).Infof("MutatingWebhookConfiguration %q changes: %v", required.GetNamespace()+"/"+required.GetName(), JSONPatchNoError(existing, toWrite)) + + actual, err := client.MutatingWebhookConfigurations().Update(context.TODO(), toWrite, metav1.UpdateOptions{}) + reportUpdateEvent(recorder, required, err) + if err != nil { + return nil, false, err + } + return actual, *modified || actual.GetGeneration() > existingCopy.GetGeneration(), nil +} + +// copyMutatingWebhookCABundle populates webhooks[].clientConfig.caBundle fields from existing resource if it was set before +// and is not set in present. This provides upgrade compatibility with service-ca-bundle operator. +func copyMutatingWebhookCABundle(from, to *admissionregistrationv1.MutatingWebhookConfiguration) { + fromMap := make(map[string]admissionregistrationv1.MutatingWebhook, len(from.Webhooks)) + for _, webhook := range from.Webhooks { + fromMap[webhook.Name] = webhook + } + + for i, wh := range to.Webhooks { + if existing, ok := fromMap[wh.Name]; ok && wh.ClientConfig.CABundle == nil { + to.Webhooks[i].ClientConfig.CABundle = existing.ClientConfig.CABundle + } + } +} + +// ApplyValidatingWebhookConfiguration ensures the form of the specified +// validatingwebhookconfiguration is present in the API. If it does not exist, +// it will be created. If it does exist, the metadata of the required +// validatingwebhookconfiguration will be merged with the existing validatingwebhookconfiguration +// and an update performed if the validatingwebhookconfiguration spec and metadata differ from +// the previously required spec and metadata based on generation change. +func ApplyValidatingWebhookConfiguration(client admissionregistrationclientv1.ValidatingWebhookConfigurationsGetter, recorder events.Recorder, + requiredOriginal *admissionregistrationv1.ValidatingWebhookConfiguration, expectedGeneration int64) (*admissionregistrationv1.ValidatingWebhookConfiguration, bool, error) { + if requiredOriginal == nil { + return nil, false, fmt.Errorf("Unexpected nil instead of an object") + } + required := requiredOriginal.DeepCopy() + + existing, err := client.ValidatingWebhookConfigurations().Get(context.TODO(), required.GetName(), metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + actual, err := client.ValidatingWebhookConfigurations().Create(context.TODO(), required, metav1.CreateOptions{}) + reportCreateEvent(recorder, required, err) + if err != nil { + return nil, false, err + } + return actual, true, nil + } else if err != nil { + return nil, false, err + } + + modified := resourcemerge.BoolPtr(false) + existingCopy := existing.DeepCopy() + + resourcemerge.EnsureObjectMeta(modified, &existingCopy.ObjectMeta, required.ObjectMeta) + if !*modified && existingCopy.GetGeneration() == expectedGeneration { + return existingCopy, false, nil + } + // at this point we know that we're going to perform a write. We're just trying to get the object correct + toWrite := existingCopy // shallow copy so the code reads easier + copyValidatingWebhookCABundle(existing, required) + toWrite.Webhooks = required.Webhooks + + klog.V(4).Infof("ValidatingWebhookConfiguration %q changes: %v", required.GetNamespace()+"/"+required.GetName(), JSONPatchNoError(existing, toWrite)) + + actual, err := client.ValidatingWebhookConfigurations().Update(context.TODO(), toWrite, metav1.UpdateOptions{}) + reportUpdateEvent(recorder, required, err) + if err != nil { + return nil, false, err + } + return actual, *modified || actual.GetGeneration() > existingCopy.GetGeneration(), nil +} + +// copyValidatingWebhookCABundle populates webhooks[].clientConfig.caBundle fields from existing resource if it was set before +// and is not set in present. This provides upgrade compatibility with service-ca-bundle operator. +func copyValidatingWebhookCABundle(from, to *admissionregistrationv1.ValidatingWebhookConfiguration) { + fromMap := make(map[string]admissionregistrationv1.ValidatingWebhook, len(from.Webhooks)) + for _, webhook := range from.Webhooks { + fromMap[webhook.Name] = webhook + } + + for i, wh := range to.Webhooks { + if existing, ok := fromMap[wh.Name]; ok && wh.ClientConfig.CABundle == nil { + to.Webhooks[i].ClientConfig.CABundle = existing.ClientConfig.CABundle + } + } +} diff --git a/pkg/operator/resource/resourceapply/admissionregistration_test.go b/pkg/operator/resource/resourceapply/admissionregistration_test.go new file mode 100644 index 0000000000..ca4f929e2c --- /dev/null +++ b/pkg/operator/resource/resourceapply/admissionregistration_test.go @@ -0,0 +1,495 @@ +package resourceapply + +import ( + "fmt" + "strings" + "testing" + + ktesting "k8s.io/client-go/testing" + + "github.com/openshift/client-go/config/clientset/versioned/scheme" + "github.com/openshift/library-go/pkg/operator/events" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes/fake" + + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" +) + +func init() { + utilruntime.Must(admissionregistrationv1.AddToScheme(scheme.Scheme)) +} + +func TestApplyMutatingConfiguration(t *testing.T) { + defaultHook := &admissionregistrationv1.MutatingWebhookConfiguration{} + defaultHook.SetName("test") + createEvent := "MutatingWebhookConfigurationCreated" + updateEvent := "MutatingWebhookConfigurationUpdated" + + injectGeneration := func(generation int64) ktesting.ReactionFunc { + return func(action ktesting.Action) (bool, runtime.Object, error) { + actual, _ := action.(ktesting.CreateAction) + webhookConfiguration, _ := actual.GetObject().(*admissionregistrationv1.MutatingWebhookConfiguration) + webhookConfiguration.SetGeneration(generation) + return false, webhookConfiguration, nil + } + } + + tests := []struct { + name string + expectModified bool + // Simulate server-side generation increase on update + disableGeneration bool + observedGeneration int64 + expectedGeneration int64 + existing func() *admissionregistrationv1.MutatingWebhookConfiguration + input func() *admissionregistrationv1.MutatingWebhookConfiguration + checkUpdated func(*admissionregistrationv1.MutatingWebhookConfiguration) error + expectedEvents []string + }{ + { + name: "Should successfully create webhook", + expectModified: true, + expectedGeneration: 1, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + expectedEvents: []string{createEvent}, + }, + { + name: "Should update webhook when changed", + expectModified: true, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + hook.Webhooks = append(hook.Webhooks, admissionregistrationv1.MutatingWebhook{ + Name: "test", + }) + return hook + }, + observedGeneration: 1, + expectedGeneration: 3, + existing: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook has changed externally since last observation + hook.SetGeneration(2) + return hook + }, + expectedEvents: []string{updateEvent}, + }, + { + name: "Should not update webhook when is unchanged", + expectModified: false, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + observedGeneration: 1, + expectedGeneration: 1, + existing: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook is unchanged generation-wise + hook.SetGeneration(1) + return hook + }, + }, + { + name: "Should create webhook and attempt an update when generation check is disabled, but report changes only once", + expectModified: true, + disableGeneration: true, + expectedGeneration: 1, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + expectedEvents: []string{createEvent, updateEvent}, + }, + { + name: "Should attempt to update resource twice when generation check is disabled but report changes only once", + expectModified: true, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + // Generation check is disabled, or this is the first apply + disableGeneration: true, + observedGeneration: 0, + expectedGeneration: 2, + existing: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook is unchanged generation-wise + hook.SetGeneration(1) + return hook + }, + expectedEvents: []string{updateEvent, updateEvent}, + }, + { + name: "Should update webhook, but preserve caBundle field if it is not set", + expectModified: true, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + hook.Webhooks = append(hook.Webhooks, + admissionregistrationv1.MutatingWebhook{Name: "test"}, + admissionregistrationv1.MutatingWebhook{Name: "second"}) + return hook + }, + observedGeneration: 1, + expectedGeneration: 3, + existing: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook has changed externally since last observation + hook.SetGeneration(2) + hook.Webhooks = append(hook.Webhooks, admissionregistrationv1.MutatingWebhook{ + Name: "test", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: []byte("test"), + }, + AdmissionReviewVersions: []string{"v1beta1"}, + }) + return hook + }, + checkUpdated: func(hook *admissionregistrationv1.MutatingWebhookConfiguration) error { + if len(hook.Webhooks) != 2 { + return fmt.Errorf("Expected to find both webhooks, got: %+v", hook.Webhooks) + } + for _, webhook := range hook.Webhooks { + if string(webhook.ClientConfig.CABundle) == "test" { + return nil + } + } + return fmt.Errorf("Expected to find webhook with unchanged clientConfig.caBundle injection == 'test', got: %#v", hook) + }, + expectedEvents: []string{updateEvent}, + }, + { + name: "Should update webhook, and force caBundle field if is set", + expectModified: true, + input: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + hook.Webhooks = append(hook.Webhooks, + admissionregistrationv1.MutatingWebhook{ + Name: "test", + ClientConfig: admissionregistrationv1.WebhookClientConfig{CABundle: []byte("test")}, + }, + admissionregistrationv1.MutatingWebhook{Name: "second"}) + return hook + }, + observedGeneration: 1, + expectedGeneration: 3, + existing: func() *admissionregistrationv1.MutatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook has changed externally since last observation + hook.SetGeneration(2) + hook.Webhooks = append(hook.Webhooks, admissionregistrationv1.MutatingWebhook{ + Name: "test", + AdmissionReviewVersions: []string{"v1beta1"}, + }) + return hook + }, + checkUpdated: func(hook *admissionregistrationv1.MutatingWebhookConfiguration) error { + if len(hook.Webhooks) != 2 { + return fmt.Errorf("Expected to find both webhooks, got: %+v", hook.Webhooks) + } + for _, webhook := range hook.Webhooks { + if string(webhook.ClientConfig.CABundle) == "test" { + return nil + } + } + return fmt.Errorf("Expected to find webhook with unchanged clientConfig.caBundle injection == 'test', got: %#v", hook) + }, + expectedEvents: []string{updateEvent}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + existingHooks := []runtime.Object{} + if test.existing != nil { + existingHooks = append(existingHooks, test.existing()) + } + client := fake.NewSimpleClientset(existingHooks...) + + // Simulate server-side generation increase + client.PrependReactor("create", "*", injectGeneration(1)) + if test.existing != nil { + client.PrependReactor("update", "*", injectGeneration(test.existing().GetGeneration()+1)) + } + recorder := events.NewInMemoryRecorder("test") + + testApply := func(expectedGeneration int64, expectModify bool) { + updatedHook, modified, err := ApplyMutatingWebhookConfiguration( + client.AdmissionregistrationV1(), + recorder, test.input(), expectedGeneration) + if err != nil { + t.Fatal(err) + } + if expectModify != modified { + t.Errorf("expected modified to be equal %v, got %v: %#v", expectModify, modified, updatedHook) + } + if expectedGeneration != 0 && expectedGeneration != updatedHook.GetGeneration() { + t.Errorf("expected generation to be %v, got %v, %#v", expectedGeneration, updatedHook.GetGeneration(), updatedHook) + } + + if test.checkUpdated != nil { + if err = test.checkUpdated(updatedHook); err != nil { + t.Errorf("Expected modification: %v", err) + } + } + } + + testApply(test.expectedGeneration, test.expectModified) + + // Second modification with generation tracking + testApply(test.expectedGeneration, false) + + // Disabled generation tracking + if test.disableGeneration { + testApply(0, false) + } + + assertEvents(t, test.name, test.expectedEvents, recorder.Events()) + }) + } +} + +func TestApplyValidatingConfiguration(t *testing.T) { + defaultHook := &admissionregistrationv1.ValidatingWebhookConfiguration{} + defaultHook.SetName("test") + createEvent := "ValidatingWebhookConfigurationCreated" + updateEvent := "ValidatingWebhookConfigurationUpdated" + + injectGeneration := func(generation int64) ktesting.ReactionFunc { + return func(action ktesting.Action) (bool, runtime.Object, error) { + actual, _ := action.(ktesting.CreateAction) + webhookConfiguration, _ := actual.GetObject().(*admissionregistrationv1.ValidatingWebhookConfiguration) + webhookConfiguration.SetGeneration(generation) + return false, webhookConfiguration, nil + } + } + + tests := []struct { + name string + expectModified bool + // Simulate server-side generation increase on update + disableGeneration bool + observedGeneration int64 + expectedGeneration int64 + existing func() *admissionregistrationv1.ValidatingWebhookConfiguration + input func() *admissionregistrationv1.ValidatingWebhookConfiguration + checkUpdated func(*admissionregistrationv1.ValidatingWebhookConfiguration) error + expectedEvents []string + }{ + { + name: "Should successfully create webhook", + expectModified: true, + expectedGeneration: 1, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + expectedEvents: []string{createEvent}, + }, + { + name: "Should update webhook when changed", + expectModified: true, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + hook.Webhooks = append(hook.Webhooks, admissionregistrationv1.ValidatingWebhook{ + Name: "test", + }) + return hook + }, + observedGeneration: 1, + expectedGeneration: 3, + existing: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook has changed externally since last observation + hook.SetGeneration(2) + return hook + }, + expectedEvents: []string{updateEvent}, + }, + { + name: "Should not update webhook when is unchanged", + expectModified: false, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + observedGeneration: 1, + expectedGeneration: 1, + existing: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook is unchanged generation-wise + hook.SetGeneration(1) + return hook + }, + }, + { + name: "Should create webhook and attempt an update when generation check is disabled, but report changes only once", + expectModified: true, + disableGeneration: true, + expectedGeneration: 1, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + expectedEvents: []string{createEvent, updateEvent}, + }, + { + name: "Should attempt to update resource twice when generation check is disabled but report changes only once", + expectModified: true, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + return defaultHook.DeepCopy() + }, + // Generation check is disabled, or this is the first apply + disableGeneration: true, + observedGeneration: 0, + expectedGeneration: 2, + existing: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook is unchanged generation-wise + hook.SetGeneration(1) + return hook + }, + expectedEvents: []string{updateEvent, updateEvent}, + }, + { + name: "Should update webhook, but preserve caBundle field if it is not set", + expectModified: true, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + hook.Webhooks = append(hook.Webhooks, + admissionregistrationv1.ValidatingWebhook{Name: "test"}, + admissionregistrationv1.ValidatingWebhook{Name: "second"}) + return hook + }, + observedGeneration: 1, + expectedGeneration: 3, + existing: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook has changed externally since last observation + hook.SetGeneration(2) + hook.Webhooks = append(hook.Webhooks, admissionregistrationv1.ValidatingWebhook{ + Name: "test", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: []byte("test"), + }, + AdmissionReviewVersions: []string{"v1beta1"}, + }) + return hook + }, + checkUpdated: func(hook *admissionregistrationv1.ValidatingWebhookConfiguration) error { + if len(hook.Webhooks) != 2 { + return fmt.Errorf("Expected to find both webhooks, got: %+v", hook.Webhooks) + } + for _, webhook := range hook.Webhooks { + if string(webhook.ClientConfig.CABundle) == "test" { + return nil + } + } + return fmt.Errorf("Expected to find webhook with unchanged clientConfig.caBundle injection == 'test', got: %#v", hook) + }, + expectedEvents: []string{updateEvent}, + }, + { + name: "Should update webhook, and force caBundle field if is set", + expectModified: true, + input: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + hook.Webhooks = append(hook.Webhooks, + admissionregistrationv1.ValidatingWebhook{ + Name: "test", + ClientConfig: admissionregistrationv1.WebhookClientConfig{CABundle: []byte("test")}, + }, + admissionregistrationv1.ValidatingWebhook{Name: "second"}) + return hook + }, + observedGeneration: 1, + expectedGeneration: 3, + existing: func() *admissionregistrationv1.ValidatingWebhookConfiguration { + hook := defaultHook.DeepCopy() + // Webhook has changed externally since last observation + hook.SetGeneration(2) + hook.Webhooks = append(hook.Webhooks, admissionregistrationv1.ValidatingWebhook{ + Name: "test", + AdmissionReviewVersions: []string{"v1beta1"}, + }) + return hook + }, + checkUpdated: func(hook *admissionregistrationv1.ValidatingWebhookConfiguration) error { + if len(hook.Webhooks) != 2 { + return fmt.Errorf("Expected to find both webhooks, got: %+v", hook.Webhooks) + } + for _, webhook := range hook.Webhooks { + if string(webhook.ClientConfig.CABundle) == "test" { + return nil + } + } + return fmt.Errorf("Expected to find webhook with unchanged clientConfig.caBundle injection == 'test', got: %#v", hook) + }, + expectedEvents: []string{updateEvent}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + existingHooks := []runtime.Object{} + if test.existing != nil { + existingHooks = append(existingHooks, test.existing()) + } + client := fake.NewSimpleClientset(existingHooks...) + + // Simulate server-side generation increase + client.PrependReactor("create", "*", injectGeneration(1)) + if test.existing != nil { + client.PrependReactor("update", "*", injectGeneration(test.existing().GetGeneration()+1)) + } + recorder := events.NewInMemoryRecorder("test") + + testApply := func(expectedGeneration int64, expectModify bool) { + updatedHook, modified, err := ApplyValidatingWebhookConfiguration( + client.AdmissionregistrationV1(), + recorder, test.input(), expectedGeneration) + if err != nil { + t.Fatal(err) + } + if expectModify != modified { + t.Errorf("expected modified to be equal %v, got %v: %#v", expectModify, modified, updatedHook) + } + if expectedGeneration != 0 && expectedGeneration != updatedHook.GetGeneration() { + t.Errorf("expected generation to be %v, got %v, %#v", expectedGeneration, updatedHook.GetGeneration(), updatedHook) + } + + if test.checkUpdated != nil { + if err = test.checkUpdated(updatedHook); err != nil { + t.Errorf("Expected modification: %v", err) + } + } + } + + testApply(test.expectedGeneration, test.expectModified) + + // Second modification with generation tracking + testApply(test.expectedGeneration, false) + + // Disabled generation tracking + if test.disableGeneration { + testApply(0, false) + } + + assertEvents(t, test.name, test.expectedEvents, recorder.Events()) + }) + } +} + +func assertEvents(t *testing.T, testCase string, expectedReasons []string, events []*corev1.Event) { + if len(expectedReasons) != len(events) { + t.Errorf( + "Test case: %s. Number of expected events (%v) differs from number of real events (%v)", + testCase, + len(expectedReasons), + len(events), + ) + } else { + for i, event := range expectedReasons { + if !strings.EqualFold(event, events[i].Reason) { + t.Errorf("Test case: %s. Expected %v event, got: %v", testCase, event, events[i].Reason) + } + } + } +} diff --git a/pkg/operator/resource/resourceapply/admissions.go b/pkg/operator/resource/resourceapply/admissions.go deleted file mode 100644 index 0edc443491..0000000000 --- a/pkg/operator/resource/resourceapply/admissions.go +++ /dev/null @@ -1,77 +0,0 @@ -package resourceapply - -import ( - "context" - - admissionv1 "k8s.io/api/admissionregistration/v1" - "k8s.io/apimachinery/pkg/api/equality" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - admissionclientv1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1" - "k8s.io/klog/v2" - - "github.com/openshift/library-go/pkg/operator/events" - "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" -) - -// ApplyValidatingWebhookConfiguration merges objectmeta, update webhooks. -func ApplyValidatingWebhookConfiguration(client admissionclientv1.ValidatingWebhookConfigurationsGetter, recorder events.Recorder, required *admissionv1.ValidatingWebhookConfiguration) (*admissionv1.ValidatingWebhookConfiguration, bool, error) { - existing, err := client.ValidatingWebhookConfigurations().Get(context.TODO(), required.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - actual, err := client.ValidatingWebhookConfigurations(). - Create(context.TODO(), required, metav1.CreateOptions{}) - reportCreateEvent(recorder, required, err) - return actual, true, err - } - if err != nil { - return nil, false, err - } - - modified := resourcemerge.BoolPtr(false) - existingCopy := existing.DeepCopy() - - resourcemerge.EnsureObjectMeta(modified, &existingCopy.ObjectMeta, required.ObjectMeta) - contentSame := equality.Semantic.DeepEqual(existingCopy.Webhooks, required.Webhooks) - if contentSame && !*modified { - return existingCopy, false, nil - } - - if klog.V(4).Enabled() { - klog.Infof("ValidatingWebhookConfiguration %q changes: %v", required.Name, JSONPatchNoError(existing, required)) - } - - actual, err := client.ValidatingWebhookConfigurations().Update(context.TODO(), required, metav1.UpdateOptions{}) - reportUpdateEvent(recorder, required, err) - return actual, true, err -} - -// ApplyMutatingWebhookConfiguration merges objectmeta, update webhooks. -func ApplyMutatingWebhookConfiguration(client admissionclientv1.MutatingWebhookConfigurationsGetter, recorder events.Recorder, required *admissionv1.MutatingWebhookConfiguration) (*admissionv1.MutatingWebhookConfiguration, bool, error) { - existing, err := client.MutatingWebhookConfigurations().Get(context.TODO(), required.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - actual, err := client.MutatingWebhookConfigurations(). - Create(context.TODO(), required, metav1.CreateOptions{}) - reportCreateEvent(recorder, required, err) - return actual, true, err - } - if err != nil { - return nil, false, err - } - - modified := resourcemerge.BoolPtr(false) - existingCopy := existing.DeepCopy() - - resourcemerge.EnsureObjectMeta(modified, &existingCopy.ObjectMeta, required.ObjectMeta) - contentSame := equality.Semantic.DeepEqual(existingCopy.Webhooks, required.Webhooks) - if contentSame && !*modified { - return existingCopy, false, nil - } - - if klog.V(4).Enabled() { - klog.Infof("ValidatingWebhookConfiguration %q changes: %v", required.Name, JSONPatchNoError(existing, required)) - } - - actual, err := client.MutatingWebhookConfigurations().Update(context.TODO(), required, metav1.UpdateOptions{}) - reportUpdateEvent(recorder, required, err) - return actual, true, err -} diff --git a/pkg/operator/resource/resourceapply/admissions_test.go b/pkg/operator/resource/resourceapply/admissions_test.go deleted file mode 100644 index 261399645f..0000000000 --- a/pkg/operator/resource/resourceapply/admissions_test.go +++ /dev/null @@ -1,282 +0,0 @@ -package resourceapply - -import ( - "testing" - - "github.com/davecgh/go-spew/spew" - - admissionv1 "k8s.io/api/admissionregistration/v1" - "k8s.io/apimachinery/pkg/api/equality" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/kubernetes/fake" - clienttesting "k8s.io/client-go/testing" - - "github.com/openshift/library-go/pkg/operator/events" -) - -func TestApplyValidatingWebhookConfiguration(t *testing.T) { - tests := []struct { - name string - existing []runtime.Object - input *admissionv1.ValidatingWebhookConfiguration - - expectedModified bool - verifyActions func(actions []clienttesting.Action, t *testing.T) - }{ - { - name: "create", - input: &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - }, - - expectedModified: true, - verifyActions: func(actions []clienttesting.Action, t *testing.T) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("get", "validatingwebhookconfigurations") || actions[0].(clienttesting.GetAction).GetName() != "foo" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("create", "validatingwebhookconfigurations") { - t.Error(spew.Sdump(actions)) - } - expected := &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - } - actual := actions[1].(clienttesting.CreateAction).GetObject().(*admissionv1.ValidatingWebhookConfiguration) - if !equality.Semantic.DeepEqual(expected, actual) { - t.Error(JSONPatchNoError(expected, actual)) - } - }, - }, - { - name: "update webhooks", - input: &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.ValidatingWebhook{ - { - Name: "webhook1", - }, - }, - }, - existing: []runtime.Object{ - &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.ValidatingWebhook{ - { - Name: "webhook2", - }, - }, - }, - }, - expectedModified: true, - verifyActions: func(actions []clienttesting.Action, t *testing.T) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("get", "validatingwebhookconfigurations") || actions[0].(clienttesting.GetAction).GetName() != "foo" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("update", "validatingwebhookconfigurations") { - t.Error(spew.Sdump(actions)) - } - expected := &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.ValidatingWebhook{ - { - Name: "webhook1", - }, - }, - } - actual := actions[1].(clienttesting.UpdateActionImpl).GetObject().(*admissionv1.ValidatingWebhookConfiguration) - if !equality.Semantic.DeepEqual(expected, actual) { - t.Error(JSONPatchNoError(expected, actual)) - } - }, - }, - { - name: "no update", - input: &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.ValidatingWebhook{ - { - Name: "webhook1", - }, - { - Name: "webhook2", - }, - }, - }, - existing: []runtime.Object{ - &admissionv1.ValidatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.ValidatingWebhook{ - { - Name: "webhook1", - }, - { - Name: "webhook2", - }, - }, - }, - }, - expectedModified: false, - verifyActions: func(actions []clienttesting.Action, t *testing.T) { - if len(actions) != 1 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("get", "validatingwebhookconfigurations") || actions[0].(clienttesting.GetAction).GetName() != "foo" { - t.Error(spew.Sdump(actions)) - } - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - client := fake.NewSimpleClientset(test.existing...) - _, actualModified, err := ApplyValidatingWebhookConfiguration(client.AdmissionregistrationV1(), events.NewInMemoryRecorder("test"), test.input) - if err != nil { - t.Fatal(err) - } - if test.expectedModified != actualModified { - t.Errorf("expected %v, got %v", test.expectedModified, actualModified) - } - test.verifyActions(client.Actions(), t) - }) - } -} - -func TestApplyMutatingWebhookConfiguration(t *testing.T) { - tests := []struct { - name string - existing []runtime.Object - input *admissionv1.MutatingWebhookConfiguration - - expectedModified bool - verifyActions func(actions []clienttesting.Action, t *testing.T) - }{ - { - name: "create", - input: &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - }, - - expectedModified: true, - verifyActions: func(actions []clienttesting.Action, t *testing.T) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("get", "mutatingwebhookconfigurations") || actions[0].(clienttesting.GetAction).GetName() != "foo" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("create", "mutatingwebhookconfigurations") { - t.Error(spew.Sdump(actions)) - } - expected := &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - } - actual := actions[1].(clienttesting.CreateAction).GetObject().(*admissionv1.MutatingWebhookConfiguration) - if !equality.Semantic.DeepEqual(expected, actual) { - t.Error(JSONPatchNoError(expected, actual)) - } - }, - }, - { - name: "update webhooks", - input: &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.MutatingWebhook{ - { - Name: "webhook1", - }, - }, - }, - existing: []runtime.Object{ - &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.MutatingWebhook{ - { - Name: "webhook2", - }, - }, - }, - }, - expectedModified: true, - verifyActions: func(actions []clienttesting.Action, t *testing.T) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("get", "mutatingwebhookconfigurations") || actions[0].(clienttesting.GetAction).GetName() != "foo" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("update", "mutatingwebhookconfigurations") { - t.Error(spew.Sdump(actions)) - } - expected := &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.MutatingWebhook{ - { - Name: "webhook1", - }, - }, - } - actual := actions[1].(clienttesting.UpdateActionImpl).GetObject().(*admissionv1.MutatingWebhookConfiguration) - if !equality.Semantic.DeepEqual(expected, actual) { - t.Error(JSONPatchNoError(expected, actual)) - } - }, - }, - { - name: "no update", - input: &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.MutatingWebhook{ - { - Name: "webhook1", - }, - { - Name: "webhook2", - }, - }, - }, - existing: []runtime.Object{ - &admissionv1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Webhooks: []admissionv1.MutatingWebhook{ - { - Name: "webhook1", - }, - { - Name: "webhook2", - }, - }, - }, - }, - expectedModified: false, - verifyActions: func(actions []clienttesting.Action, t *testing.T) { - if len(actions) != 1 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("get", "mutatingwebhookconfigurations") || actions[0].(clienttesting.GetAction).GetName() != "foo" { - t.Error(spew.Sdump(actions)) - } - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - client := fake.NewSimpleClientset(test.existing...) - _, actualModified, err := ApplyMutatingWebhookConfiguration(client.AdmissionregistrationV1(), events.NewInMemoryRecorder("test"), test.input) - if err != nil { - t.Fatal(err) - } - if test.expectedModified != actualModified { - t.Errorf("expected %v, got %v", test.expectedModified, actualModified) - } - test.verifyActions(client.Actions(), t) - }) - } -} diff --git a/pkg/operator/resource/resourceapply/generic.go b/pkg/operator/resource/resourceapply/generic.go index 43a4ce30f8..7f62f87abe 100644 --- a/pkg/operator/resource/resourceapply/generic.go +++ b/pkg/operator/resource/resourceapply/generic.go @@ -9,7 +9,6 @@ import ( "github.com/openshift/api" "github.com/openshift/library-go/pkg/operator/events" - admissionv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" storagev1 "k8s.io/api/storage/v1" @@ -166,16 +165,6 @@ func ApplyDirectly(clients *ClientHolder, recorder events.Recorder, manifests As result.Error = fmt.Errorf("missing kubeClient") } result.Result, result.Changed, result.Error = ApplyCSIDriver(clients.kubeClient.StorageV1(), recorder, t) - case *admissionv1.ValidatingWebhookConfiguration: - if clients.kubeClient == nil { - result.Error = fmt.Errorf("missing kubeClient") - } - result.Result, result.Changed, result.Error = ApplyValidatingWebhookConfiguration(clients.kubeClient.AdmissionregistrationV1(), recorder, t) - case *admissionv1.MutatingWebhookConfiguration: - if clients.kubeClient == nil { - result.Error = fmt.Errorf("missing kubeClient") - } - result.Result, result.Changed, result.Error = ApplyMutatingWebhookConfiguration(clients.kubeClient.AdmissionregistrationV1(), recorder, t) default: result.Error = fmt.Errorf("unhandled type %T", requiredObj) } diff --git a/pkg/operator/resource/resourcemerge/admissionregistration.go b/pkg/operator/resource/resourcemerge/admissionregistration.go new file mode 100644 index 0000000000..2fcfd13949 --- /dev/null +++ b/pkg/operator/resource/resourcemerge/admissionregistration.go @@ -0,0 +1,51 @@ +package resourcemerge + +import ( + operatorsv1 "github.com/openshift/api/operator/v1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// ExpectedMutatingWebhooksConfiguration returns last applied generation for MutatingWebhookConfiguration resource registered in operator +func ExpectedMutatingWebhooksConfiguration(name string, previousGenerations []operatorsv1.GenerationStatus) int64 { + generation := GenerationFor(previousGenerations, schema.GroupResource{Group: admissionregistrationv1.SchemeGroupVersion.Group, Resource: "mutatingwebhookconfigurations"}, "", name) + if generation != nil { + return generation.LastGeneration + } + return -1 +} + +// SetMutatingWebhooksConfigurationGeneration updates operator generation status list with last applied generation for provided MutatingWebhookConfiguration resource +func SetMutatingWebhooksConfigurationGeneration(generations *[]operatorsv1.GenerationStatus, actual *admissionregistrationv1.MutatingWebhookConfiguration) { + if actual == nil { + return + } + SetGeneration(generations, operatorsv1.GenerationStatus{ + Group: admissionregistrationv1.SchemeGroupVersion.Group, + Resource: "mutatingwebhookconfigurations", + Name: actual.Name, + LastGeneration: actual.ObjectMeta.Generation, + }) +} + +// ExpectedValidatingWebhooksConfiguration returns last applied generation for ValidatingWebhookConfiguration resource registered in operator +func ExpectedValidatingWebhooksConfiguration(name string, previousGenerations []operatorsv1.GenerationStatus) int64 { + generation := GenerationFor(previousGenerations, schema.GroupResource{Group: admissionregistrationv1.SchemeGroupVersion.Group, Resource: "validatingwebhookconfigurations"}, "", name) + if generation != nil { + return generation.LastGeneration + } + return -1 +} + +// SetValidatingWebhooksConfigurationGeneration updates operator generation status list with last applied generation for provided ValidatingWebhookConfiguration resource +func SetValidatingWebhooksConfigurationGeneration(generations *[]operatorsv1.GenerationStatus, actual *admissionregistrationv1.ValidatingWebhookConfiguration) { + if actual == nil { + return + } + SetGeneration(generations, operatorsv1.GenerationStatus{ + Group: admissionregistrationv1.SchemeGroupVersion.Group, + Resource: "validatingwebhookconfigurations", + Name: actual.Name, + LastGeneration: actual.ObjectMeta.Generation, + }) +}