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

@@ -562,7 +562,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 +597,14 @@ 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.
+			if sdi, ok := installer.(*install.StrategyDeploymentInstaller); ok {
+				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)
+				}
 			}
 		}
 	}
@@ -2614,7 +2638,14 @@ 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.
+	webhooksInstalled := false
+	var webhookErr error
+	if strategyInstalled && strategyErr == nil {
+		webhooksInstalled, webhookErr = a.areWebhooksAvailable(csv, installer)
+	}
 
 	if strategyInstalled && apiServicesInstalled && webhooksInstalled {
 		// if there's no error, we're successfully running