fix(olm): fix ConversionWebhook spec.conversion lifecycle during CSV upgrades

pkg/controller/install/deployment.go

@@ -123,6 +123,14 @@ func (i *StrategyDeploymentInstaller) createOrUpdateCertResourcesForDeployment()
 				return err
 			}
 		case *webhookDescriptionWithCAPEM:
+			// ConversionWebhook CRD patching is deferred to the post-Install readiness
+			// check (areWebhooksAvailable) so that spec.conversion is only written once
+			// the new deployment's pods are actually serving /convert. Writing it here
+			// during Install() — before any pod is ready — causes a window where the
+			// apiserver routes conversion calls to pods that return HTTP 404.
+			if d.webhookDescription.Type == v1alpha1.ConversionWebhook {
+				continue
+			}
 			err := i.createOrUpdateWebhook(d.caPEM, d.webhookDescription)
 			if err != nil {
 				return err
@@ -134,6 +142,22 @@ func (i *StrategyDeploymentInstaller) createOrUpdateCertResourcesForDeployment()
 	return nil
 }
 
+// EnsureConversionWebhooks writes spec.conversion on CRDs for any ConversionWebhook
+// entries in the cert resources. Called after the deployment is confirmed ready so that
+// the conversion endpoint is only activated when the new pods are serving /convert.
+func (i *StrategyDeploymentInstaller) EnsureConversionWebhooks() error {
+	for _, desc := range i.getCertResources() {
+		d, ok := desc.(*webhookDescriptionWithCAPEM)
+		if !ok || d.webhookDescription.Type != v1alpha1.ConversionWebhook {
+			continue
+		}
+		if err := i.createOrUpdateConversionWebhook(d.caPEM, d.webhookDescription); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (i *StrategyDeploymentInstaller) deploymentForSpec(name string, spec appsv1.DeploymentSpec, specLabels k8slabels.Set) (deployment *appsv1.Deployment, hash string, err error) {
 	dep := &appsv1.Deployment{Spec: spec}
 	dep.SetName(name)

pkg/controller/operators/olm/apiservices.go

@@ -426,6 +426,19 @@ func (a *Operator) getWebhookCABundle(csv *v1alpha1.ClusterServiceVersion, desc
 
 			return crd.Spec.Conversion.Webhook.ClientConfig.CABundle, nil
 		}
+
+		// Conversion webhook configuration is deferred until the deployment is ready.
+		// Use the OLM-managed CA while calculating the expected deployment spec before
+		// the CRD has been configured.
+		if desc.DeploymentName != "" {
+			secretName := install.SecretName(install.ServiceName(desc.DeploymentName))
+			secret, err := a.lister.CoreV1().SecretLister().Secrets(csv.GetNamespace()).Get(secretName)
+			if err == nil {
+				if caBundle, ok := secret.Data[install.OLMCAPEMKey]; ok && len(caBundle) > 0 {
+					return caBundle, nil
+				}
+			}
+		}
 	}
 
 	return nil, fmt.Errorf("unable to find CA")
@@ -562,7 +575,11 @@ func (a *Operator) cleanUpRemovedWebhooks(csv *v1alpha1.ClusterServiceVersion) e
 	return nil
 }
 
-func (a *Operator) areWebhooksAvailable(csv *v1alpha1.ClusterServiceVersion) (bool, error) {
+// areWebhooksAvailable checks that all webhook resources declared in the CSV exist and
+// are correctly configured. For ConversionWebhook entries it also writes spec.conversion
+// on the target CRDs using the provided installer, ensuring conversion is only activated
+// once the new deployment's pods are ready to serve /convert.
+func (a *Operator) areWebhooksAvailable(csv *v1alpha1.ClusterServiceVersion, installer install.StrategyInstaller) (bool, error) {
 	err := a.cleanUpRemovedWebhooks(csv)
 	if err != nil {
 		return false, err
@@ -593,6 +610,16 @@ func (a *Operator) areWebhooksAvailable(csv *v1alpha1.ClusterServiceVersion) (bo
 			}
 			webhookCount = len(webhookList.Items)
 		case v1alpha1.ConversionWebhook:
+			// Write spec.conversion on each target CRD now that the deployment is confirmed
+			// ready. This is deferred from Install() to prevent routing conversion calls to
+			// pods that are not yet serving /convert.
+			sdi, ok := installer.(*install.StrategyDeploymentInstaller)
+			if !ok {
+				return false, fmt.Errorf("conversionWebhook requires a StrategyDeploymentInstaller, got %T", installer)
+			}
+			if err := sdi.EnsureConversionWebhooks(); err != nil {
+				return false, fmt.Errorf("conversionWebhook not ready: %w", err)
+			}
 			for _, conversionCRD := range desc.ConversionCRDs {
 				// check if CRD exists on cluster
 				crd, err := a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), conversionCRD, metav1.GetOptions{})

pkg/controller/operators/olm/operator.go

@@ -1302,11 +1302,27 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) {
 	// webhook from the CRD definition.
 	csvs, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(clusterServiceVersion.GetNamespace()).List(labels.Everything())
 	if err != nil {
-		logger.Errorf("error listing csvs: %v\n", err)
+		// Without a complete CSV list we cannot safely determine which CRDs are still
+		// covered by a replacement CSV. Bail out to avoid incorrectly clearing
+		// spec.conversion on CRDs that a replacement still owns.
+		logger.Errorf("error listing csvs, skipping conversion webhook cleanup: %v\n", err)
+		return
 	}
+
+	// Build the set of CRDs whose ConversionWebhook is still covered by the replacement CSV.
+	// If the replacement dropped the ConversionWebhook for a given CRD, spec.conversion on
+	// that CRD must be reset — otherwise it keeps pointing at the now-deleted service and
+	// all CR requests against that CRD will fail.
+	coveredCRDs := map[string]bool{}
 	for _, csv := range csvs {
 		if csv.Spec.Replaces == clusterServiceVersion.GetName() {
-			return
+			for _, desc := range csv.Spec.WebhookDefinitions {
+				if desc.Type == v1alpha1.ConversionWebhook {
+					for _, crdName := range desc.ConversionCRDs {
+						coveredCRDs[crdName] = true
+					}
+				}
+			}
 		}
 	}
 
@@ -1316,18 +1332,26 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) {
 		}
 
 		for i, crdName := range desc.ConversionCRDs {
+			if coveredCRDs[crdName] {
+				// Replacement CSV still has a ConversionWebhook for this CRD; leave
+				// spec.conversion intact so in-flight conversion calls keep working.
+				continue
+			}
+
 			crd, err := a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), crdName, metav1.GetOptions{})
 			if err != nil {
 				logger.Errorf("error getting CRD %v which was defined in CSVs spec.WebhookDefinition[%d]: %v\n", crdName, i, err)
 				continue
 			}
 
 			copy := crd.DeepCopy()
-			copy.Spec.Conversion.Strategy = apiextensionsv1.NoneConverter
-			copy.Spec.Conversion.Webhook = nil
+			if copy.Spec.Conversion != nil {
+				copy.Spec.Conversion.Strategy = apiextensionsv1.NoneConverter
+				copy.Spec.Conversion.Webhook = nil
 
-			if _, err = a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), copy, metav1.UpdateOptions{}); err != nil {
-				logger.Errorf("error updating conversion strategy for CRD %v: %v\n", crdName, err)
+				if _, err = a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), copy, metav1.UpdateOptions{}); err != nil {
+					logger.Errorf("error updating conversion strategy for CRD %v: %v\n", crdName, err)
+				}
 			}
 		}
 	}
@@ -2617,7 +2641,16 @@ func (a *Operator) updateInstallStatus(csv *v1alpha1.ClusterServiceVersion, inst
 	}
 
 	apiServicesInstalled, apiServiceErr := a.areAPIServicesAvailable(csv)
-	webhooksInstalled, webhookErr := a.areWebhooksAvailable(csv)
+	// Only attempt to write spec.conversion once the deployment is confirmed ready.
+	// areWebhooksAvailable calls EnsureConversionWebhooks, so calling it when
+	// strategyInstalled is false would recreate the upgrade race we are fixing.
+	// Note: CheckInstalled never returns (true, non-nil error), so strategyInstalled
+	// is sufficient — no need to also gate on strategyErr.
+	webhooksInstalled := false
+	var webhookErr error
+	if strategyInstalled {
+		webhooksInstalled, webhookErr = a.areWebhooksAvailable(csv, installer)
+	}
 
 	if strategyInstalled && apiServicesInstalled && webhooksInstalled {
 		// if there's no error, we're successfully running

pkg/controller/operators/olm/operator_test.go

@@ -3702,6 +3702,52 @@ func TestWebhookCABundleRetrieval(t *testing.T) {
 				err:      missingCAError,
 			},
 		},
+		{
+			name: "RetrieveCAFromConversionWebhookSecretBeforeCRDIsConfigured",
+			initial: initial{
+				csvs: []*v1alpha1.ClusterServiceVersion{
+					csvWithConversionWebhook(csv("csv1",
+						namespace,
+						"0.0.0",
+						"",
+						installStrategy("csv1-dep1",
+							nil,
+							[]v1alpha1.StrategyDeploymentPermissions{},
+						),
+						[]*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")},
+						[]*apiextensionsv1.CustomResourceDefinition{},
+						v1alpha1.CSVPhaseInstalling,
+					), "csv1-dep1", []string{"c1.g1"}),
+				},
+				crds: []runtime.Object{
+					crdWithConversionWebhook(crd("c1", "v1", "g1"), nil),
+				},
+				objs: []runtime.Object{
+					&corev1.Secret{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      install.SecretName(install.ServiceName("csv1-dep1")),
+							Namespace: namespace,
+							Labels: map[string]string{
+								install.OLMManagedLabelKey: install.OLMManagedLabelValue,
+							},
+						},
+						Data: map[string][]byte{
+							install.OLMCAPEMKey: caBundle,
+						},
+					},
+				},
+				desc: v1alpha1.WebhookDescription{
+					DeploymentName: "csv1-dep1",
+					GenerateName:   "webhook",
+					Type:           v1alpha1.ConversionWebhook,
+					ConversionCRDs: []string{"c1.g1"},
+				},
+			},
+			expected: expected{
+				caBundle: caBundle,
+				err:      nil,
+			},
+		},
 		{
 			name: "RetrieveFromValidatingAdmissionWebhook",
 			initial: initial{