[release-v1.42] fix(apiserver): prevent calico-apiserver deadlock on direct 3.21-to-3.23 upgrade

pkg/controller/apiserver/apiserver_controller.go

@@ -473,6 +473,19 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re
 		}
 	}
 
+	// Detect the allow-tigera -> calico-system apiserver migration window. Gate on the Tier
+	// watch being ready, matching the includeV3NetworkPolicy read above: before the watch is
+	// ready the aggregated v3 API may be unavailable, and we must not degrade/return on that.
+	// During a real migration the old apiserver is serving, so the watch is ready.
+	migrationInProgress := false
+	if !r.opts.UseV3CRDs && r.tierWatchReady.IsReady() {
+		migrationInProgress, err = utils.APIServerMigrationInProgress(ctx, r.client)
+		if err != nil {
+			r.status.SetDegraded(operatorv1.ResourceReadError, "Error checking apiserver migration state", err, reqLogger)
+			return reconcile.Result{}, err
+		}
+	}
+
 	err = utils.PopulateK8sServiceEndPoint(r.client)
 	if err != nil {
 		r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading services endpoint configmap", err, reqLogger)
@@ -577,6 +590,12 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re
 		return reconcile.Result{}, err
 	}
 
+	// The bridge must precede the workload component so the apply loop's fail-fast
+	// behavior requeues before any cutover if a bridge write does not land.
+	if migrationInProgress {
+		components = append(components, render.APIServerMigrationBridge(&apiServerCfg))
+	}
+
 	components = append(components,
 		component,
 		rcertificatemanagement.CertificateManagement(&rcertificatemanagement.Config{
@@ -594,7 +613,10 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re
 	// dependency on the API server deployment.
 	//
 	// We do this last to avoid transient errors with policy preventing progression of the controller.
-	if r.opts.UseV3CRDs || includeV3NetworkPolicy {
+	// While migrating, the bridge owns calico-system.apiserver-access and the transitional
+	// allow-tigera.apiserver-access must stay; skipping this component avoids the double-write
+	// and suppresses its deletion of allow-tigera.apiserver-access until the apiserver is stable.
+	if (r.opts.UseV3CRDs || includeV3NetworkPolicy) && !migrationInProgress {
 		components = append(components, render.APIServerPolicy(&apiServerCfg))
 	}
 

pkg/controller/apiserver/apiserver_controller_test.go

@@ -32,7 +32,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	apiregv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	v3 "github.com/tigera/api/pkg/apis/projectcalico/v3"
@@ -1030,4 +1032,106 @@ var _ = Describe("apiserver controller tests", func() {
 			Expect(err.Error()).To(ContainSubstring("CalicoWebhooksDeployment"))
 		})
 	})
+
+	Context("apiserver migration bridge", func() {
+		// Reconciler wired as in the other tests, with both watches ready.
+		newReconciler := func(cli client.Client) ReconcileAPIServer {
+			return ReconcileAPIServer{
+				client:              cli,
+				scheme:              scheme,
+				status:              mockStatus,
+				tierWatchReady:      ready,
+				migrationWatchReady: ready,
+				opts: options.ControllerOptions{
+					EnterpriseCRDExists: true,
+					DetectedProvider:    operatorv1.ProviderNone,
+					ClusterDomain:       dns.DefaultClusterDomain,
+				},
+			}
+		}
+
+		seedMigrationState := func(cli client.Client) {
+			// Deprecated tier still present; apiserver not yet Ready in calico-system.
+			Expect(cli.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"}})).NotTo(HaveOccurred())
+			Expect(cli.Create(ctx, &v3.NetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera.default-deny", Namespace: "calico-system"},
+				Spec:       v3.NetworkPolicySpec{Tier: "allow-tigera", Selector: "all()"},
+			})).NotTo(HaveOccurred())
+			Expect(cli.Create(ctx, installation)).To(BeNil())
+		}
+
+		It("writes the bridge and does not tear down allow-tigera.apiserver-access while migrating", func() {
+			seedMigrationState(cli)
+			r := newReconciler(cli)
+
+			_, err := r.Reconcile(ctx, reconcile.Request{})
+			Expect(err).ShouldNot(HaveOccurred())
+
+			// default-deny rewritten to exclude the apiserver.
+			dd := &v3.NetworkPolicy{}
+			Expect(cli.Get(ctx, client.ObjectKey{Name: "allow-tigera.default-deny", Namespace: "calico-system"}, dd)).NotTo(HaveOccurred())
+			Expect(dd.Spec.Selector).To(Equal("k8s-app != 'calico-apiserver'"))
+
+			// order-1 transitional allow created in the deprecated tier and NOT deleted.
+			at := &v3.NetworkPolicy{}
+			Expect(cli.Get(ctx, client.ObjectKey{Name: "allow-tigera.apiserver-access", Namespace: "calico-system"}, at)).NotTo(HaveOccurred())
+			Expect(at.Spec.Tier).To(Equal("allow-tigera"))
+
+			// steady-state allow created by the bridge.
+			cs := &v3.NetworkPolicy{}
+			Expect(cli.Get(ctx, client.ObjectKey{Name: "calico-system.apiserver-access", Namespace: "calico-system"}, cs)).NotTo(HaveOccurred())
+		})
+
+		It("requeues without moving when a bridge write fails", func() {
+			// A bridge write failure surfaces through SetDegraded(ResourceUpdateError, ...),
+			// which the shared mock (set up for other error codes only) does not expect.
+			mockStatus.On("SetDegraded", operatorv1.ResourceUpdateError, mock.Anything, mock.Anything, mock.Anything).Return().Maybe()
+
+			failing := ctrlrfake.DefaultFakeClientBuilder(scheme).WithInterceptorFuncs(interceptor.Funcs{
+				Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+					if np, ok := obj.(*v3.NetworkPolicy); ok && np.GetName() == "allow-tigera.apiserver-access" {
+						return kerror.NewServiceUnavailable("the server is currently unable to handle the request")
+					}
+					return c.Create(ctx, obj, opts...)
+				},
+			}).Build()
+
+			// Re-seed prerequisites on the failing client (mirrors BeforeEach essentials): the
+			// operator CA (so certificatemanager.Create, which is not called with
+			// AllowCACreation from within Reconcile, does not fail first), and the APIServer CR
+			// (without it Reconcile short-circuits on "not found" before ever reaching the
+			// bridge). Authentication/dex/oidc are not required: GetAuthentication's NotFound is
+			// tolerated and the keyValidatorConfig lookup is skipped.
+			certificateManager, err := certificatemanager.Create(failing, nil, "cluster.local", common.OperatorNamespace(), certificatemanager.AllowCACreation())
+			Expect(err).NotTo(HaveOccurred())
+			Expect(failing.Create(ctx, certificateManager.KeyPair().Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
+			Expect(failing.Create(ctx, &operatorv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"}})).NotTo(HaveOccurred())
+			Expect(failing.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "calico-system"}})).NotTo(HaveOccurred())
+			seedMigrationState(failing)
+
+			r := newReconciler(failing)
+			_, err = r.Reconcile(ctx, reconcile.Request{})
+			Expect(err).To(HaveOccurred())
+
+			// The workload cutover must NOT have happened. The bridge is ordered before the
+			// workload component and the apply loop is fail-fast, so render.APIServer never runs
+			// and the v3.projectcalico.org APIService is never created this reconcile.
+			apiService := &apiregv1.APIService{}
+			getErr := failing.Get(ctx, client.ObjectKey{Name: "v3.projectcalico.org"}, apiService)
+			Expect(kerror.IsNotFound(getErr)).To(BeTrue(), "workload cutover must not run when a bridge write fails")
+		})
+
+		It("lets the tail create calico-system.apiserver-access when no bridge is needed", func() {
+			// No allow-tigera tier => not migrating; calico-system tier already seeded by BeforeEach.
+			Expect(cli.Create(ctx, installation)).To(BeNil())
+			r := newReconciler(cli)
+
+			_, err := r.Reconcile(ctx, reconcile.Request{})
+			Expect(err).ShouldNot(HaveOccurred())
+
+			cs := &v3.NetworkPolicy{}
+			Expect(cli.Get(ctx, client.ObjectKey{Name: "calico-system.apiserver-access", Namespace: "calico-system"}, cs)).NotTo(HaveOccurred())
+			Expect(cs.Spec.Tier).To(Equal("calico-system"))
+		})
+	})
 })

pkg/controller/installation/core_controller.go

@@ -1629,8 +1629,13 @@ func (r *ReconcileInstallation) Reconcile(ctx context.Context, request reconcile
 		if nonclusterhost != nil {
 			components = append(components, render.NewTyphaNonClusterHostPolicy(&typhaCfg))
 		}
+		migrationInProgress, err := utils.APIServerMigrationInProgress(ctx, r.client)
+		if err != nil {
+			r.status.SetDegraded(operatorv1.ResourceReadError, "Error checking apiserver migration state", err, reqLogger)
+			return reconcile.Result{}, err
+		}
 		components = append(components,
-			kubecontrollers.NewCalicoKubeControllersPolicy(&kubeControllersCfg, calicoSystemDefaultDenyForCalicoSystem()),
+			kubecontrollers.NewCalicoKubeControllersPolicy(&kubeControllersCfg, calicoSystemDefaultDenyForCalicoSystem(), migrationInProgress),
 		)
 	}
 

pkg/controller/tiers/tiers_controller.go

@@ -96,6 +96,14 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error {
 		return fmt.Errorf("tiers-controller failed to watch node-local-dns daemonset: %v", err)
 	}
 
+	// Perform periodic reconciliation. This acts as a backstop to catch reconcile issues, and also
+	// makes sure we spot when things change that might not trigger a reconciliation - notably, deleting
+	// the deprecated allow-tigera tier once the transitional apiserver-migration policies within it have
+	// been cleaned up (that cleanup is driven by other controllers and does not enqueue this one).
+	if err := utils.AddPeriodicReconcile(c, utils.PeriodicReconcileTime, &handler.EnqueueRequestForObject{}); err != nil {
+		return fmt.Errorf("tiers-controller failed to create periodic reconcile watch: %w", err)
+	}
+
 	return nil
 }
 
@@ -136,13 +144,21 @@ func (r *ReconcileTiers) Reconcile(ctx context.Context, request reconcile.Reques
 		return reconcile.Result{}, err
 	}
 
-	// Try to delete allow-tigera deprecated tier
-	err = componentHandler.CreateOrUpdateOrDelete(ctx, render.NewDeletionPassthrough(&v3.Tier{
-		TypeMeta:   metav1.TypeMeta{Kind: "Tier", APIVersion: "projectcalico.org/v3"},
-		ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"},
-	}), nil)
-	if err != nil {
-		log.V(1).Info("Unable to delete deprecated allow-tigera tier at this time", "error", err)
+	// Delete the deprecated allow-tigera tier, but not while the apiserver is still migrating
+	// out of it: the migration bridge (apiserver controller) writes transitional policies into
+	// this tier, so removing it mid-migration would fight the bridge. Resume once the apiserver
+	// is stable in calico-system.
+	migrationInProgress, migErr := utils.APIServerMigrationInProgress(ctx, r.client)
+	if migErr != nil {
+		log.V(1).Info("Unable to determine apiserver migration state; deferring allow-tigera tier deletion", "error", migErr)
+	} else if !migrationInProgress {
+		err = componentHandler.CreateOrUpdateOrDelete(ctx, render.NewDeletionPassthrough(&v3.Tier{
+			TypeMeta:   metav1.TypeMeta{Kind: "Tier", APIVersion: "projectcalico.org/v3"},
+			ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"},
+		}), nil)
+		if err != nil {
+			log.V(1).Info("Unable to delete deprecated allow-tigera tier at this time", "error", err)
+		}
 	}
 
 	r.status.ReadyToMonitor()

pkg/controller/tiers/tiers_controller_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/mock"
 
 	appsv1 "k8s.io/api/apps/v1"
+	kerror "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -204,4 +205,43 @@ var _ = Describe("tier controller tests", func() {
 		Expect(err).ShouldNot(HaveOccurred())
 		mockStatus.AssertExpectations(GinkgoT())
 	})
+
+	It("does not delete the allow-tigera tier while the apiserver migration is in progress", func() {
+		mockStatus.On("ReadyToMonitor")
+		mockStatus.On("ClearDegraded")
+
+		// Migration in progress: deprecated tier present, calico-apiserver not Ready.
+		Expect(c.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"}})).NotTo(HaveOccurred())
+		Expect(c.Create(ctx, &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{Name: "calico-apiserver", Namespace: "calico-system"},
+			Status:     appsv1.DeploymentStatus{ReadyReplicas: 0},
+		})).NotTo(HaveOccurred())
+
+		_, err := r.Reconcile(ctx, reconcile.Request{})
+		Expect(err).ShouldNot(HaveOccurred())
+
+		tier := &v3.Tier{}
+		Expect(c.Get(ctx, client.ObjectKey{Name: "allow-tigera"}, tier)).NotTo(HaveOccurred())
+		mockStatus.AssertExpectations(GinkgoT())
+	})
+
+	It("deletes the allow-tigera tier once the apiserver is stable in calico-system", func() {
+		mockStatus.On("ReadyToMonitor")
+		mockStatus.On("ClearDegraded")
+
+		// Migration complete: deprecated tier present, calico-apiserver Ready.
+		Expect(c.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"}})).NotTo(HaveOccurred())
+		Expect(c.Create(ctx, &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{Name: "calico-apiserver", Namespace: "calico-system"},
+			Status:     appsv1.DeploymentStatus{ReadyReplicas: 1},
+		})).NotTo(HaveOccurred())
+
+		_, err := r.Reconcile(ctx, reconcile.Request{})
+		Expect(err).ShouldNot(HaveOccurred())
+
+		tier := &v3.Tier{}
+		err = c.Get(ctx, client.ObjectKey{Name: "allow-tigera"}, tier)
+		Expect(kerror.IsNotFound(err)).To(BeTrue())
+		mockStatus.AssertExpectations(GinkgoT())
+	})
 })

pkg/controller/utils/apiserver_migration.go

@@ -0,0 +1,62 @@
+// Copyright (c) 2026 Tigera, Inc. All rights reserved.
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"context"
+
+	appsv1 "k8s.io/api/apps/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	v3 "github.com/tigera/api/pkg/apis/projectcalico/v3"
+
+	"github.com/tigera/operator/pkg/render"
+)
+
+// APIServerMigrationInProgress reports whether the cluster is mid-way through the
+// allow-tigera -> calico-system apiserver migration performed on a direct
+// Calico Enterprise 3.21.x -> 3.23.x upgrade: the deprecated "allow-tigera" Tier
+// still exists AND the calico-apiserver Deployment is not yet Ready in
+// calico-system. Controllers use it to sequence the migration bridge and to
+// suppress teardown of the transitional policies until the apiserver is stable.
+//
+// It reads the "allow-tigera" Tier and the calico-apiserver Deployment through the
+// caller's cached client; every controller that calls it already watches those
+// types.
+func APIServerMigrationInProgress(ctx context.Context, c client.Client) (bool, error) {
+	tier := &v3.Tier{}
+	if err := c.Get(ctx, client.ObjectKey{Name: render.DeprecatedAPIServerTierName}, tier); err != nil {
+		if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) {
+			// No deprecated tier: fresh install or already-migrated cluster.
+			return false, nil
+		}
+		return false, err
+	}
+
+	// The deprecated tier exists. Migration is in progress until calico-apiserver
+	// is Ready in calico-system.
+	d := &appsv1.Deployment{}
+	err := c.Get(ctx, client.ObjectKey{Name: render.APIServerName, Namespace: render.APIServerNamespace}, d)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			// Not created (or moved) yet.
+			return true, nil
+		}
+		return false, err
+	}
+	return d.Status.ReadyReplicas == 0, nil
+}