-
Notifications
You must be signed in to change notification settings - Fork 4.8k
OPRUN-3863: Add olmv1 webhook support origin tests #29825
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 all 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,316 @@ | ||
| package operators | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "k8s.io/apimachinery/pkg/runtime/schema" | ||
| "k8s.io/client-go/dynamic" | ||
| "k8s.io/client-go/tools/clientcmd" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "time" | ||
|
|
||
| g "github.com/onsi/ginkgo/v2" | ||
| o "github.com/onsi/gomega" | ||
| exutil "github.com/openshift/origin/test/extended/util" | ||
| "k8s.io/apimachinery/pkg/api/meta" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/util/wait" | ||
| ) | ||
|
|
||
| const ( | ||
| conditionTypeServing = "Serving" | ||
| conditionTypeInstalled = "Installed" | ||
|
|
||
| openshiftServiceCANamespace = "openshift-service-ca" | ||
| openshiftServiceCASigningKeySecretName = "signing-key" | ||
| ) | ||
|
|
||
| var ( | ||
| webhookTestBaseDir = exutil.FixturePath("testdata", "olmv1", "webhook-support") | ||
| webhookOperatorInstallNamespace = "webhook-operator" | ||
| ) | ||
|
|
||
| var _ = g.Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServiceCA][Skipped:Disconnected][Serial] OLMv1 operator with webhooks", g.Ordered, g.Serial, func() { | ||
|
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. I wonder if there is some way to test certificate rotation and check that the service is available after a rotation occurs? I feel like that has been problematic in OLMv0's implementation.
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. That's a great question, and I'm really not sure how much we can control openshift-serviceca's rotation period. Let me poke at it.
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. I've updated the two tests:
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. For the latter case, I think my expectation would be:
2 and 3 wouldn't happen atomically at the same time, so I think it would be safe to assume some downtime, between catalogd and operator-controller, but then for that to correct itself on its own after both (2) and (3) occur. Is that your understanding of what should happen?
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. I think that's reasonable. I was wondering if it would be a controlled hand-off to the new pod by the deployment.
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. The service-ca validity period is 26 months, a time period where users are expected to have upgraded, such that the certificate is rotated as part of an upgrade. |
||
| defer g.GinkgoRecover() | ||
| oc := exutil.NewCLIWithoutNamespace("openshift-operator-controller-webhooks") | ||
|
|
||
| g.BeforeAll(func(ctx g.SpecContext) { | ||
|
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. You cannot use beforeall like this in origin, it's not true ginkgo. Every test has to be completely independent because they are run in their own process -- so BeforeAll effectively becomes BeforeEach.. If you need a cluster configuration the job would be dedicated to this purpose, configure it the right way and then run your suite. OTE has designs for tests tied to configs, but it's not implemented yet. https://github.com/openshift/enhancements/blob/master/enhancements/testing/openshift-tests-extension.md |
||
| g.By("checking olmv1 capability is enabled") | ||
| checkFeatureCapability(oc) | ||
|
|
||
| g.By("creating the webhook-operator catalog") | ||
| manifest, err := os.ReadFile(filepath.Join(webhookTestBaseDir, "webhook-operator-catalog.yaml")) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| cleanup, err := applyRenderedManifests(oc, string(manifest)) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| g.DeferCleanup(cleanup) | ||
|
|
||
| g.By("waiting for the webhook-operator catalog to be serving") | ||
| var lastReason string | ||
| err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, | ||
| func(ctx context.Context) (bool, error) { | ||
| isDone, err, status := checkCatalogServing(oc, "webhook-operator-catalog") | ||
| if lastReason != status { | ||
| g.GinkgoLogr.Info(fmt.Sprintf("waitForWebhookOperatorCatalogServing: %q", status)) | ||
| lastReason = status | ||
| } | ||
| return isDone, err | ||
| }) | ||
| o.Expect(lastReason).To(o.BeEmpty()) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| g.By("installing the webhook operator") | ||
| extManifests, err := getWebhookClusterExtensionInstallManifests(webhookOperatorInstallNamespace) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| cleanup, err = applyRenderedManifests(oc, extManifests) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| g.DeferCleanup(cleanup) | ||
|
|
||
| g.By("waiting for the webhook operator to be installed") | ||
| lastReason = "" | ||
| err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, | ||
| func(ctx context.Context) (bool, error) { | ||
| isDone, err, status := checkOperatorInstalled(oc, "webhook-operator") | ||
| if lastReason != status { | ||
| g.GinkgoLogr.Info(fmt.Sprintf("waitForWebhookOperatorInstalled: %q", status)) | ||
| lastReason = status | ||
| } | ||
| return isDone, err | ||
| }) | ||
| o.Expect(lastReason).To(o.BeEmpty()) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| g.By("waiting for the webhook operator deployment to be Available") | ||
| err = oc.AsAdmin().WithoutNamespace().Run("wait").Args("--for=condition=Available", "-n", webhookOperatorInstallNamespace, "deployments/webhook-operator-webhook").Execute() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| }) | ||
|
|
||
| g.AfterEach(func() { | ||
| if g.CurrentSpecReport().Failed() { | ||
| exutil.DumpPodLogsStartingWith("", oc) | ||
| } | ||
| }) | ||
|
|
||
| g.It("should have a working validating webhook", func(ctx g.SpecContext) { | ||
| g.By("creating a webhook test resource that will be rejected by the validating webhook") | ||
| resource := getWebhookOperatorResource("validating-webhook-test", webhookOperatorInstallNamespace, false) | ||
|
|
||
| // even if the resource gets created, it will be cleaned up when test gets torn down and the | ||
| // webhook-operator namespace gets deleted | ||
| o.Eventually(func(cg o.Gomega) { | ||
| _, err := applyRenderedManifests(oc, resource) | ||
| cg.Expect(err).To(o.HaveOccurred()) | ||
| cg.Expect(err.Error()).To(o.ContainSubstring("Invalid value: false: Spec.Valid must be true")) | ||
| }).WithTimeout(10 * time.Second).WithPolling(250 * time.Millisecond).Should(o.Succeed()) | ||
| }) | ||
|
|
||
| g.It("should have a working mutating webhook", func(ctx g.SpecContext) { | ||
| g.By("creating a valid webhook test resource") | ||
| var ( | ||
| mutatingWebhookResourceName = "mutating-webhook-test" | ||
| resource = getWebhookOperatorResource(mutatingWebhookResourceName, webhookOperatorInstallNamespace, true) | ||
| ) | ||
|
|
||
| o.Eventually(func(cg o.Gomega) { | ||
| cleanup, err := applyRenderedManifests(oc, resource) | ||
| cg.Expect(err).NotTo(o.HaveOccurred()) | ||
| // clean up fn will only be scheduled if there are no errors | ||
| g.DeferCleanup(cleanup) | ||
| }).WithTimeout(10 * time.Second).WithPolling(250 * time.Millisecond).Should(o.Succeed()) | ||
|
|
||
| cfg, err := clientcmd.BuildConfigFromFlags("", exutil.KubeConfigPath()) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
| c, err := dynamic.NewForConfig(cfg) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
|
|
||
| v1WebhookTestClient := c.Resource(schema.GroupVersionResource{ | ||
| Group: "webhook.operators.coreos.io", | ||
| Version: "v1", | ||
| Resource: "webhooktests", | ||
| }).Namespace(webhookOperatorInstallNamespace) | ||
|
|
||
| g.By("getting the created resource in v1 schema") | ||
| obj, err := v1WebhookTestClient.Get(ctx, mutatingWebhookResourceName, metav1.GetOptions{}) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
| o.Expect(obj).ToNot(o.BeNil()) | ||
|
|
||
| g.By("validating the resource spec") | ||
| spec := obj.Object["spec"].(map[string]interface{}) | ||
| o.Expect(spec).To(o.Equal(map[string]interface{}{ | ||
| "valid": true, | ||
| "mutate": true, | ||
| })) | ||
| }) | ||
|
|
||
| g.It("should have a working conversion webhook", func(ctx g.SpecContext) { | ||
| var ( | ||
| conversionWebhookResourceName = "conversion-webhook-test" | ||
| resource = getWebhookOperatorResource(conversionWebhookResourceName, webhookOperatorInstallNamespace, true) | ||
| ) | ||
|
|
||
| o.Eventually(func(cg o.Gomega) { | ||
| cleanup, err := applyRenderedManifests(oc, resource) | ||
| cg.Expect(err).NotTo(o.HaveOccurred()) | ||
| // clean up fn will only be scheduled if there are no errors | ||
| g.DeferCleanup(cleanup) | ||
| }).WithTimeout(10 * time.Second).WithPolling(250 * time.Millisecond).Should(o.Succeed()) | ||
|
|
||
| cfg, err := clientcmd.BuildConfigFromFlags("", exutil.KubeConfigPath()) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
| c, err := dynamic.NewForConfig(cfg) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
|
|
||
| v2WebhookTestClient := c.Resource(schema.GroupVersionResource{ | ||
| Group: "webhook.operators.coreos.io", | ||
| Version: "v2", | ||
| Resource: "webhooktests", | ||
| }).Namespace(webhookOperatorInstallNamespace) | ||
|
|
||
| g.By("getting the created resource in v2 schema") | ||
| obj, err := v2WebhookTestClient.Get(ctx, conversionWebhookResourceName, metav1.GetOptions{}) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
| o.Expect(obj).ToNot(o.BeNil()) | ||
|
|
||
| g.By("validating the resource spec") | ||
| spec := obj.Object["spec"].(map[string]interface{}) | ||
| o.Expect(spec).To(o.Equal(map[string]interface{}{ | ||
| "conversion": map[string]interface{}{ | ||
| "valid": true, | ||
| "mutate": true, | ||
| }, | ||
| })) | ||
| }) | ||
|
|
||
| g.It("should be tolerant to openshift-service-ca certificate rotation", func(ctx g.SpecContext) { | ||
| resource := getWebhookOperatorResource("some-resource", webhookOperatorInstallNamespace, true) | ||
|
|
||
| // delete tls secret | ||
| g.By("deleting the openshift-service-ca signing-key secret") | ||
| err := oc.AsAdmin().WithoutNamespace().Run("delete").Args("secret", openshiftServiceCASigningKeySecretName, "--namespace", openshiftServiceCANamespace).Execute() | ||
|
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. I wonder if deleting the signing key, even though we're in a serial test, will be disruptive for other workloads? I'm guessing yes. But also, maybe those other workloads should be tolerating and self-correcting when the signing key is deleted.
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. I was wondering the same thing. Let's try it out. If it turns out to be too disruptive we can switch it off and notify the org in case it's something that should be rectified across the board.
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. This feels dangerous, and given the validity period of the service-ca, doesn't happen very often. I do wonder what side effects there will be. |
||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
|
|
||
| // even with the secret deleted the webhook should still be responsive | ||
| g.By("checking webhook is responsive through cert rotation") | ||
| o.Eventually(func(g o.Gomega) { | ||
| cleanup, err := applyRenderedManifests(oc, resource) | ||
| g.Expect(err).ToNot(o.HaveOccurred()) | ||
| cleanup() | ||
| }).WithTimeout(30 * time.Second).WithPolling(500 * time.Millisecond).Should(o.Succeed()) | ||
| }) | ||
|
|
||
| g.It("should be tolerant to tls secret deletion", func(ctx g.SpecContext) { | ||
| resource := getWebhookOperatorResource("some-resource", webhookOperatorInstallNamespace, true) | ||
| certificateSecretName := "webhook-operator-webhook-service-cert" | ||
|
|
||
| g.By("ensuring secret exists") | ||
| o.Eventually(func(g o.Gomega) { | ||
| err := oc.AsAdmin().WithoutNamespace().Run("get").Args("secret", certificateSecretName, "-n", webhookOperatorInstallNamespace).Execute() | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
| }).Should(o.Succeed()) | ||
|
|
||
| // delete tls secret | ||
| g.By("checking webhook is responsive through secret recreation") | ||
| err := oc.AsAdmin().WithoutNamespace().Run("delete").Args("secret", certificateSecretName, "-n", webhookOperatorInstallNamespace).Execute() | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
|
|
||
| // even with the secret deleted the webhook should still be responsive | ||
| o.Eventually(func(g o.Gomega) { | ||
| cleanup, err := applyRenderedManifests(oc, resource) | ||
| g.Expect(err).ToNot(o.HaveOccurred()) | ||
| cleanup() | ||
| }).WithTimeout(10 * time.Second).WithPolling(100 * time.Millisecond).Should(o.Succeed()) | ||
| }) | ||
| }) | ||
|
|
||
| func checkCatalogServing(oc *exutil.CLI, catalogName string) (bool, error, string) { | ||
| cmdArgs := []string{ | ||
| "clustercatalogs.olm.operatorframework.io", | ||
| catalogName, | ||
| "-o=jsonpath={.status.conditions}", | ||
| } | ||
|
|
||
| output, err := oc.AsAdmin().WithoutNamespace().Run("get").Args(cmdArgs...).Output() | ||
| if err != nil { | ||
| return false, err, "" | ||
| } | ||
| // no data yet, so try again | ||
| if output == "" { | ||
| return false, nil, "no output" | ||
| } | ||
|
|
||
| var conditions []metav1.Condition | ||
| if err := json.Unmarshal([]byte(output), &conditions); err != nil { | ||
| return false, fmt.Errorf("error in json.Unmarshal(%v): %v", output, err), "" | ||
| } | ||
| c := meta.FindStatusCondition(conditions, conditionTypeServing) | ||
| if c == nil { | ||
| return false, nil, fmt.Sprintf("condition not present: %q", conditionTypeServing) | ||
| } | ||
| if c.Status != metav1.ConditionTrue { | ||
| return false, nil, fmt.Sprintf("expected status to be %q: %+v", metav1.ConditionTrue, c) | ||
| } | ||
| return true, nil, "" | ||
| } | ||
|
|
||
| func checkOperatorInstalled(oc *exutil.CLI, extName string) (bool, error, string) { | ||
| cmdArgs := []string{ | ||
| "clusterextensions.olm.operatorframework.io", | ||
| extName, | ||
| "-o=jsonpath={.status.conditions}", | ||
| } | ||
|
|
||
| output, err := oc.AsAdmin().WithoutNamespace().Run("get").Args(cmdArgs...).Output() | ||
| if err != nil { | ||
| return false, err, "" | ||
| } | ||
| // no data yet, so try again | ||
| if output == "" { | ||
| return false, nil, "no output" | ||
| } | ||
|
|
||
| var conditions []metav1.Condition | ||
| if err := json.Unmarshal([]byte(output), &conditions); err != nil { | ||
| return false, fmt.Errorf("error in json.Unmarshal(%v): %v", output, err), "" | ||
| } | ||
| c := meta.FindStatusCondition(conditions, conditionTypeInstalled) | ||
| if c == nil { | ||
| return false, nil, fmt.Sprintf("condition not present: %q", conditionTypeInstalled) | ||
| } | ||
| if c.Status != metav1.ConditionTrue { | ||
| return false, nil, fmt.Sprintf("expected status to be %q: %+v", metav1.ConditionTrue, c) | ||
| } | ||
| return true, nil, "" | ||
| } | ||
|
|
||
| func applyRenderedManifests(oc *exutil.CLI, manifests string) (func(), error) { | ||
| g.By(fmt.Sprintf("applying manifests %q", manifests)) | ||
| err := oc.AsAdmin().WithoutNamespace().Run("apply").Args("-f", "-").InputString(manifests).Execute() | ||
| return func() { | ||
| err := oc.AsAdmin().WithoutNamespace().Run("delete").Args("-f", "-").InputString(manifests).Execute() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| }, err | ||
| } | ||
|
|
||
| func getWebhookClusterExtensionInstallManifests(installNamespace string) (string, error) { | ||
| bytes, err := os.ReadFile(filepath.Join(webhookTestBaseDir, "webhook-operator.yaml")) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return strings.ReplaceAll(string(bytes), "{NAMESPACE}", installNamespace), nil | ||
| } | ||
|
|
||
| func getWebhookOperatorResource(name string, namespace string, valid bool) string { | ||
| return fmt.Sprintf(`apiVersion: webhook.operators.coreos.io/v1 | ||
| kind: WebhookTest | ||
| metadata: | ||
| name: %s | ||
| namespace: %s | ||
| spec: | ||
| valid: %t | ||
| `, name, namespace, valid) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.