[release-4.11] OCPBUGS-16075: fix dynamic conversion webhook

Makefile

@@ -122,7 +122,7 @@ e2e/operator-registry: ## Run e2e registry tests
 	$(MAKE) e2e WHAT=operator-registry
 
 e2e/olm: ## Run e2e olm tests
-	$(MAKE) e2e WHAT=operator-lifecycle-manager E2E_INSTALL_NS=openshift-operator-lifecycle-manager E2E_TEST_NS=openshift-operators E2E_TIMEOUT=120m KUBECTL=oc
+	$(MAKE) e2e WHAT=operator-lifecycle-manager E2E_CATALOG_NS=openshift-marketplace E2E_INSTALL_NS=openshift-operator-lifecycle-manager E2E_TEST_NS=openshift-operators E2E_TIMEOUT=120m KUBECTL=oc
 
 .PHONY: vendor
 vendor:

staging/operator-lifecycle-manager/Makefile

@@ -134,10 +134,11 @@ TEST := $(shell go run ./test/e2e/split/... -chunks $(E2E_TEST_NUM_CHUNKS) -prin
 endif
 E2E_OPTS ?= $(if $(E2E_SEED),-seed '$(E2E_SEED)') $(if $(SKIP), -skip '$(SKIP)') $(if $(TEST),-focus '$(TEST)') -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress
 E2E_INSTALL_NS ?= operator-lifecycle-manager
+E2E_CATALOG_NS ?= $(E2E_INSTALL_NS)
 E2E_TEST_NS ?= operators
 
 e2e:
-	$(GINKGO) $(E2E_OPTS) $(or $(run), ./test/e2e) $< -- -namespace=$(E2E_TEST_NS) -olmNamespace=$(E2E_INSTALL_NS) -dummyImage=bitnami/nginx:latest $(or $(extra_args), -kubeconfig=${KUBECONFIG})
+	$(GINKGO) $(E2E_OPTS) $(or $(run), ./test/e2e) $< -- -namespace=$(E2E_TEST_NS) -olmNamespace=$(E2E_INSTALL_NS) -catalogNamespace=$(E2E_CATALOG_NS) -dummyImage=bitnami/nginx:latest $(or $(extra_args), -kubeconfig=${KUBECONFIG})
 
 # See workflows/e2e-tests.yml See test/e2e/README.md for details.
 .PHONY: e2e-local

staging/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go

@@ -295,7 +295,7 @@ func (a *Operator) getWebhookCABundle(csv *v1alpha1.ClusterServiceVersion, desc
 			if err != nil {
 				continue
 			}
-			if crd.Spec.Conversion == nil || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
+			if crd.Spec.Conversion == nil || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil || crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
 				continue
 			}
 
@@ -472,7 +472,7 @@ func (a *Operator) areWebhooksAvailable(csv *v1alpha1.ClusterServiceVersion) (bo
 					return false, err
 				}
 
-				if crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != "Webhook" || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
+				if crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != "Webhook" || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil || crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
 					return false, fmt.Errorf("conversionWebhook not ready")
 				}
 				webhookCount++

staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go

@@ -2268,7 +2268,12 @@ func (a *Operator) updateInstallStatus(csv *v1alpha1.ClusterServiceVersion, inst
 		return fmt.Errorf(msg)
 	}
 
-	if !webhooksInstalled || webhookErr != nil {
+	if webhookErr != nil {
+		csv.SetPhaseWithEventIfChanged(v1alpha1.CSVPhaseInstallReady, requeueConditionReason, fmt.Sprintf("Webhook install failed: %s", webhookErr), now, a.recorder)
+		return webhookErr
+	}
+
+	if !webhooksInstalled {
 		msg := "webhooks not installed"
 		csv.SetPhaseWithEventIfChanged(requeuePhase, requeueConditionReason, msg, now, a.recorder)
 		if err := a.csvQueueSet.Requeue(csv.GetNamespace(), csv.GetName()); err != nil {

staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator_test.go

@@ -3513,7 +3513,7 @@ func TestWebhookCABundleRetrieval(t *testing.T) {
 					), "csv1-dep1", []string{"c1.g1"}),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1", "g1"),
+					crdWithConversionWebhook(crd("c1", "v1", "g1"), nil),
 				},
 				desc: v1alpha1.WebhookDescription{
 					GenerateName:   "webhook",

staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go

@@ -131,17 +131,6 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 	})
 
 	It("global update triggers subscription sync", func() {
-		globalNS := operatorNamespace
-
-		// Determine which namespace is global. Should be `openshift-marketplace` for OCP 4.2+.
-		// Locally it is `olm`
-		namespaces, _ := c.KubernetesInterface().CoreV1().Namespaces().List(context.Background(), metav1.ListOptions{})
-		for _, ns := range namespaces.Items {
-			if ns.GetName() == "openshift-marketplace" {
-				globalNS = "openshift-marketplace"
-			}
-		}
-
 		mainPackageName := genName("nginx-")
 
 		mainPackageStable := fmt.Sprintf("%s-stable", mainPackageName)
@@ -179,7 +168,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 		}
 
 		// Create the initial catalog source
-		cs, cleanup := createInternalCatalogSource(c, crc, mainCatalogName, globalNS, mainManifests, []apiextensions.CustomResourceDefinition{mainCRD}, []v1alpha1.ClusterServiceVersion{mainCSV})
+		cs, cleanup := createInternalCatalogSource(c, crc, mainCatalogName, globalCatalogNamespace, mainManifests, []apiextensions.CustomResourceDefinition{mainCRD}, []v1alpha1.ClusterServiceVersion{mainCSV})
 		defer cleanup()
 
 		// Attempt to get the catalog source before creating install plan

staging/operator-lifecycle-manager/test/e2e/catalog_exclusion_test.go

@@ -49,7 +49,7 @@ var _ = Describe("Global Catalog Exclusion", func() {
 		globalCatalog := &v1alpha1.CatalogSource{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      genName("bad-global-catalog-"),
-				Namespace: operatorNamespace,
+				Namespace: globalCatalogNamespace,
 			},
 			Spec: v1alpha1.CatalogSourceSpec{
 				DisplayName: "Broken Global Catalog Source",
@@ -105,7 +105,7 @@ var _ = Describe("Global Catalog Exclusion", func() {
 		When("the operator group is annotated with olm.operatorframework.io/exclude-global-namespace-resolution=true", func() {
 			// Note: this test flakes on openshift CI but passes running on a local openshift cluster
 			// For some reason, the subscription does not transition to a failed state in time
-			It("[FLAKE] the broken subscription should resolve and have state AtLatest", func() {
+			It("the broken subscription should resolve and have state AtLatest", func() {
 				By("checking that the subscription is not resolving and has a condition with type ResolutionFailed")
 				EventuallyResource(subscription, 5*time.Minute).Should(ContainSubscriptionConditionOfType(v1alpha1.SubscriptionResolutionFailed))
 

staging/operator-lifecycle-manager/test/e2e/e2e_test.go

@@ -30,6 +30,9 @@ var (
 	olmNamespace = flag.String(
 		"olmNamespace", "", "namespace where olm is running")
 
+	catalogNamespace = flag.String(
+		"catalogNamespace", "", "namespace where the global catalog content is stored")
+
 	communityOperators = flag.String(
 		"communityOperators",
 		"quay.io/operator-framework/upstream-community-operators@sha256:098457dc5e0b6ca9599bd0e7a67809f8eca397907ca4d93597380511db478fec",
@@ -58,6 +61,7 @@ var (
 	testNamespace           = ""
 	operatorNamespace       = ""
 	communityOperatorsImage = ""
+	globalCatalogNamespace  = ""
 	junitDir                = "junit"
 )
 
@@ -95,6 +99,7 @@ var _ = BeforeSuite(func() {
 	testNamespace = *namespace
 	operatorNamespace = *olmNamespace
 	communityOperatorsImage = *communityOperators
+	globalCatalogNamespace = *catalogNamespace
 	testdataDir = *testdataPath
 	deprovision = ctx.MustProvision(ctx.Ctx())
 	ctx.MustInstall(ctx.Ctx())