-
Notifications
You must be signed in to change notification settings - Fork 55
OCPCLOUD-913 - Tests for webhook configurations provisioning by MAO #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| package framework | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| mapiv1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" | ||
| admissionregistrationv1 "k8s.io/api/admissionregistration/v1" | ||
| "k8s.io/apimachinery/pkg/api/equality" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| "k8s.io/apimachinery/pkg/util/wait" | ||
| "k8s.io/klog" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| // DefaultValidatingWebhookConfiguration is a default validating webhook configuration resource provided by MAO | ||
| var DefaultValidatingWebhookConfiguration = mapiv1.NewValidatingWebhookConfiguration() | ||
|
|
||
| // DefaultMutatingWebhookConfiguration is a default mutating webhook configuration resource provided by MAO | ||
| var DefaultMutatingWebhookConfiguration = mapiv1.NewMutatingWebhookConfiguration() | ||
|
|
||
| // GetMutatingWebhookConfiguration gets MutatingWebhookConfiguration object by name | ||
| func GetMutatingWebhookConfiguration(c client.Client, name string) (*admissionregistrationv1.MutatingWebhookConfiguration, error) { | ||
| key := client.ObjectKey{Name: name} | ||
| existing := &admissionregistrationv1.MutatingWebhookConfiguration{} | ||
|
|
||
| if err := wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { | ||
| if err := c.Get(context.TODO(), key, existing); err != nil { | ||
| klog.Errorf("Error querying api for MutatingWebhookConfiguration object %q: %v, retrying...", name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }); err != nil { | ||
| return nil, fmt.Errorf("error getting MutatingWebhookConfiguration %q: %v", name, err) | ||
| } | ||
| return existing, nil | ||
| } | ||
|
|
||
| // GetValidatingWebhookConfiguration gets ValidatingWebhookConfiguration object by name | ||
| func GetValidatingWebhookConfiguration(c client.Client, name string) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) { | ||
| key := client.ObjectKey{Name: name} | ||
| existing := &admissionregistrationv1.ValidatingWebhookConfiguration{} | ||
|
|
||
| if err := wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { | ||
| if err := c.Get(context.TODO(), key, existing); err != nil { | ||
| klog.Errorf("Error querying api for ValidatingWebhookConfiguration object %q: %v, retrying...", name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }); err != nil { | ||
| return nil, fmt.Errorf("error getting ValidatingWebhookConfiguration %q: %v", name, err) | ||
| } | ||
| return existing, nil | ||
| } | ||
|
|
||
| // DeleteValidatingWebhookConfiguration deletes the specified ValidatingWebhookConfiguration object | ||
| func DeleteValidatingWebhookConfiguration(c client.Client, webhookConfiguraiton *admissionregistrationv1.ValidatingWebhookConfiguration) error { | ||
| return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { | ||
| if err := c.Delete(context.TODO(), webhookConfiguraiton); apierrors.IsNotFound(err) { | ||
| return true, nil | ||
| } else if err != nil { | ||
| klog.Errorf("error querying api for ValidatingWebhookConfiguration object %q: %v, retrying...", webhookConfiguraiton.Name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| } | ||
|
|
||
| // DeleteMutatingWebhookConfiguration deletes the specified MutatingWebhookConfiguration object | ||
| func DeleteMutatingWebhookConfiguration(c client.Client, webhookConfiguraiton *admissionregistrationv1.MutatingWebhookConfiguration) error { | ||
| return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { | ||
| if err := c.Delete(context.TODO(), webhookConfiguraiton); apierrors.IsNotFound(err) { | ||
| return true, nil | ||
| } else if err != nil { | ||
| klog.Errorf("error querying api for MutatingWebhookConfiguration object %q: %v, retrying...", webhookConfiguraiton.Name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| } | ||
|
|
||
| // UpdateMutatingWebhookConfiguration updates the specified mutating webhook configuration | ||
| func UpdateMutatingWebhookConfiguration(c client.Client, updated *admissionregistrationv1.MutatingWebhookConfiguration) error { | ||
| return wait.PollImmediate(RetryShort, WaitMedium, func() (bool, error) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why I think the only purpose of using
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me. Webhooks are ready to react as soon as they are updated in the cluster. I could see potential to use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @enxebre I decreased both wait periods to be at |
||
| existing, err := GetMutatingWebhookConfiguration(c, updated.Name) | ||
| if err != nil { | ||
| klog.Errorf("Error getting MutatingWebhookConfiguration: %v", err) | ||
| return false, nil | ||
| } | ||
| if err := c.Patch(context.TODO(), existing, client.MergeFrom(updated)); err != nil { | ||
| klog.Errorf("error patching MutatingWebhookConfiguration object %q: %v, retrying...", updated.Name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| } | ||
|
|
||
| // UpdateValidatingWebhookConfiguration updates the specified mutating webhook configuration | ||
| func UpdateValidatingWebhookConfiguration(c client.Client, updated *admissionregistrationv1.ValidatingWebhookConfiguration) error { | ||
| return wait.PollImmediate(RetryShort, WaitMedium, func() (bool, error) { | ||
| existing, err := GetValidatingWebhookConfiguration(c, updated.Name) | ||
| if err != nil { | ||
| klog.Errorf("Error getting ValidatingWebhookConfiguration: %v", err) | ||
| return false, nil | ||
| } | ||
| if err := c.Patch(context.TODO(), existing, client.MergeFrom(updated)); err != nil { | ||
| klog.Errorf("error patching ValidatingWebhookConfiguration object %q: %v, retrying...", updated.Name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| } | ||
|
|
||
| // IsMutatingWebhookConfigurationSynced expects a matching MutatingWebhookConfiguration to be present in the cluster | ||
| func IsMutatingWebhookConfigurationSynced(c client.Client) bool { | ||
| if err := wait.PollImmediate(RetryShort, WaitLong, func() (bool, error) { | ||
| existing, err := GetMutatingWebhookConfiguration(c, DefaultMutatingWebhookConfiguration.Name) | ||
| if err != nil { | ||
| klog.Errorf("Error getting MutatingWebhookConfiguration: %v", err) | ||
| return false, nil | ||
| } | ||
|
|
||
| // Due to caBundle injection by service-ca-operator, we have to use DeepDerivative, | ||
| // which will ignore change in spec.webhooks[x].serviceReference.caBundle in comparison | ||
| // to empty value, as the default webhook configuration does not have this field set | ||
| equal := equality.Semantic.DeepDerivative(DefaultMutatingWebhookConfiguration.Webhooks, existing.Webhooks) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth a note that this will should the CABundle field that has been set by the injector and that's why we are using |
||
| if !equal { | ||
| klog.Infof("MutatingWebhookConfiguration is not yet equal, retrying...") | ||
| } | ||
| return equal, nil | ||
| }); err != nil { | ||
| klog.Errorf("Error waiting for match with expected MutatingWebhookConfigurationMatched: %v", err) | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // IsValidatingWebhookConfigurationSynced expects a matching MutatingWebhookConfiguration to be present in the cluster | ||
| func IsValidatingWebhookConfigurationSynced(c client.Client) bool { | ||
| if err := wait.PollImmediate(RetryShort, WaitLong, func() (bool, error) { | ||
| existing, err := GetValidatingWebhookConfiguration(c, DefaultValidatingWebhookConfiguration.Name) | ||
| if err != nil { | ||
| klog.Errorf("Error getting MutatingWebhookConfiguration: %v", err) | ||
| return false, nil | ||
| } | ||
|
|
||
| // Due to caBundle injection by service-ca-operator, we have to use DeepDerivative, | ||
| // which will ignore change in spec.webhooks[x].serviceReference.caBundle in comparison | ||
| // to empty value, as the default webhook configuration does not have this field set | ||
| equal := equality.Semantic.DeepDerivative(DefaultValidatingWebhookConfiguration.Webhooks, existing.Webhooks) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth a note that this will should the CABundle field that has been set by the injector and that's why we are using |
||
| if !equal { | ||
| klog.Infof("ValidatingWebhookConfiguration is not yet equal, retrying...") | ||
| } | ||
| return equal, nil | ||
| }); err != nil { | ||
| klog.Errorf("Error waiting for match with expected ValidatingWebhookConfigurationMatched: %v", err) | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about trying to delete a resource that doesn't exist? Should we account for that?