fix: restore kubeconfig Secret cache invalidation

controllers/delete_checks.go

@@ -66,7 +66,7 @@ func validateDeleteChecks(ctx context.Context, clusterSummary *configv1beta1.Clu
 
 	adminNamespace, adminName := getClusterSummaryAdmin(clusterSummary)
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(),
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(),
 		clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, adminNamespace,
 		adminName, clusterSummary.Spec.ClusterType, logger)
 	if err != nil {

controllers/drift_detection_upgrade.go

@@ -303,7 +303,7 @@ func skipUpgrading(ctx context.Context, c client.Client, cluster client.Object,
 	// Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the
 	// direct client rather than c.
 	cacheMgr := clustercache.GetManager()
-	managedClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterDirectClient(), cluster.GetNamespace(), cluster.GetName(),
+	managedClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterClient(), cluster.GetNamespace(), cluster.GetName(),
 		"", "", clusterproxy.GetClusterType(clusterRef), logger)
 	if err != nil {
 		logger.V(logs.LogDebug).Error(err, "failed to get managed client")

controllers/export_test.go

@@ -287,11 +287,11 @@ var (
 )
 
 // SetManagementClusterConfigForTest overrides the cached management cluster rest.Config,
-// leaving managementClusterClient/managementClusterDirectClient untouched, and returns the
-// previous value so a test can restore it. Unlike SetManagementClusterAccess, this lets a
-// test break only the config the "local" (management cluster) undeploy pass builds its
-// discovery/dynamic clients from, without also breaking the direct client other code paths
-// (e.g. kubeconfig Secret reads for the "remote" pass) rely on.
+// leaving managementClusterClient untouched, and returns the previous value so a test can
+// restore it. Unlike SetManagementClusterAccess, this lets a test break only the config the
+// "local" (management cluster) undeploy pass builds its discovery/dynamic clients from,
+// without also breaking other code paths (e.g. kubeconfig Secret reads for the "remote" pass)
+// that go through managementClusterClient directly.
 func SetManagementClusterConfigForTest(config *rest.Config) *rest.Config {
 	old := managementClusterConfig
 	managementClusterConfig = config

controllers/handlers_helm.go

@@ -160,7 +160,7 @@ func deployHelmCharts(ctx context.Context, c client.Client,
 		adminNamespace, adminName := getClusterSummaryAdmin(clusterSummary)
 		// Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the
 		// direct client rather than c.
-		remoteRestConfig, restErr := clustercache.GetManager().GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(),
+		remoteRestConfig, restErr := clustercache.GetManager().GetKubernetesRestConfig(ctx, getManagementClusterClient(),
 			clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName,
 			adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger)
 		if restErr != nil {
@@ -280,7 +280,7 @@ func postProcessDeployedHelmCharts(ctx context.Context, clusterSummary *configv1
 	// Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the
 	// direct client rather than c.
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName,
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterNamespace, clusterName,
 		adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return err
@@ -367,7 +367,7 @@ func undeployHelmCharts(ctx context.Context, c client.Client,
 
 	// Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the
 	// direct client rather than c.
-	remoteRestConfig, err := clustercache.GetManager().GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(),
+	remoteRestConfig, err := clustercache.GetManager().GetKubernetesRestConfig(ctx, getManagementClusterClient(),
 		clusterNamespace, clusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return err
@@ -1746,10 +1746,8 @@ func createRegistryClientOptions(ctx context.Context, clusterSummary *configv1be
 			return nil, err
 		}
 
-		// Not a ClusterProfileSecretType Secret, so it is never in the (possibly scoped) cache;
-		// use the direct client.
 		secret := &corev1.Secret{}
-		err = getManagementClusterDirectClient().Get(ctx,
+		err = getManagementClusterClient().Get(ctx,
 			types.NamespacedName{
 				Namespace: credentialSecretNamespace,
 				Name:      currentChart.RegistryCredentialsConfig.CredentialsSecretRef.Name,
@@ -2845,7 +2843,7 @@ func recoverRelease(ctx context.Context, clusterSummary *configv1beta1.ClusterSu
 				requestedChart.ReleaseNamespace, secretName))
 
 		cacheMgr := clustercache.GetManager()
-		remoteClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterDirectClient(),
+		remoteClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterClient(),
 			clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName,
 			"", "", clusterSummary.Spec.ClusterType, logger)
 		if err != nil {
@@ -4894,10 +4892,8 @@ func createFileWithCredentials(ctx context.Context, c client.Client, clusterSumm
 		return "", err
 	}
 
-	// Not a ClusterProfileSecretType Secret, so it is never in the (possibly scoped) cache;
-	// use the direct client rather than c.
 	secret := &corev1.Secret{}
-	err = getManagementClusterDirectClient().Get(ctx,
+	err = getManagementClusterClient().Get(ctx,
 		types.NamespacedName{
 			Namespace: namespace,
 			Name:      credSecretRef.Name,
@@ -4952,10 +4948,8 @@ func createFileWithCA(ctx context.Context, c client.Client, clusterSummary *conf
 		return "", err
 	}
 
-	// Not a ClusterProfileSecretType Secret, so it is never in the (possibly scoped) cache;
-	// use the direct client rather than c.
 	secret := &corev1.Secret{}
-	err = getManagementClusterDirectClient().Get(ctx,
+	err = getManagementClusterClient().Get(ctx,
 		types.NamespacedName{
 			Namespace: namespace,
 			Name:      requestedChart.RegistryCredentialsConfig.CASecretRef.Name,

controllers/handlers_helm_test.go

@@ -1825,7 +1825,7 @@ resources:
 		credentialsBytes, err := json.Marshal(credentials)
 		Expect(err).To(BeNil())
 
-		// getCredentialsAndCAFiles reads these Secrets through getManagementClusterDirectClient(),
+		// getCredentialsAndCAFiles reads these Secrets through getManagementClusterClient(),
 		// which is backed by the shared envtest API server (see controllers_suite_test.go), not by
 		// the local fake client c built below. So they must be created via testEnv, not just added
 		// to initObjects.

controllers/handlers_kustomize.go

@@ -272,15 +272,15 @@ func cleanStaleKustomizeResources(ctx context.Context, clusterSummary *configv1b
 
 	// Only resources previously deployed by ClusterSummary are removed here. Even if profile is created by serviceAccount
 	// use cluster-admin account to do the removal
-	remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(),
+	remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterClient(),
 		clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType,
 		logger)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace,
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterSummary.Spec.ClusterNamespace,
 		clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return nil, nil, err

controllers/handlers_resources.go

@@ -229,15 +229,15 @@ func cleanStaleResources(ctx context.Context, clusterSummary *configv1beta1.Clus
 
 	// Only resources previously deployed by ClusterSummary are removed here. Even if profile is created by serviceAccount
 	// use cluster-admin account to do the removal
-	remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(),
+	remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterClient(),
 		clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType,
 		logger)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace,
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterSummary.Spec.ClusterNamespace,
 		clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return nil, nil, err
@@ -450,17 +450,14 @@ func pushModeUndeployResources(ctx context.Context, c client.Client, clusterSumm
 
 	// Only resources previously deployed by ClusterSummary are removed here. Even if profile is created by serviceAccount
 	// use cluster-admin account to do the removal
-	// Kubeconfig Secret reads must bypass any Secret-cache scoping (see
-	// getManagementClusterDirectClient), so these two calls deliberately use the
-	// direct client rather than the c passed into this function.
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName,
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterNamespace, clusterName,
 		"", "", clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return err
 	}
 
-	remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName,
+	remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterClient(), clusterNamespace, clusterName,
 		"", "", clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return err

controllers/handlers_resources_test.go

@@ -299,11 +299,10 @@ var _ = Describe("HandlersResource", func() {
 		}, timeout, pollingInterval).Should(BeTrue())
 
 		// Break only the rest.Config the local (management cluster) cleanup pass builds its
-		// discovery/dynamic client from. managementClusterClient/managementClusterDirectClient
-		// are left untouched, so the remote pass - which resolves its own client independently,
-		// from the kubeconfig Secret created in BeforeEach - keeps working. That is what lets
-		// this test tell the two passes apart: the local pass must fail while the remote pass
-		// succeeds.
+		// discovery/dynamic client from. managementClusterClient is left untouched, so the
+		// remote pass - which resolves its own client independently, from the kubeconfig Secret
+		// created in BeforeEach - keeps working. That is what lets this test tell the two passes
+		// apart: the local pass must fail while the remote pass succeeds.
 		brokenConfig := rest.CopyConfig(testEnv.Config)
 		brokenConfig.Host = "https://127.0.0.1:1"
 		brokenConfig.Timeout = 2 * time.Second

controllers/handlers_utils.go

@@ -743,7 +743,7 @@ func getClusterSummaryAndClusterClient(ctx context.Context, clusterNamespace, cl
 	adminNamespace, adminName := getClusterSummaryAdmin(clusterSummary)
 	// Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the
 	// direct client rather than the c passed into this function.
-	clusterClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace,
+	clusterClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterClient(), clusterSummary.Spec.ClusterNamespace,
 		clusterSummary.Spec.ClusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return nil, nil, err
@@ -1422,7 +1422,7 @@ func getRestConfig(ctx context.Context, clusterSummary *configv1beta1.ClusterSum
 	// Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the
 	// direct client rather than c.
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName,
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterNamespace, clusterName,
 		adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger)
 	if err != nil {
 		return nil, logger, err
@@ -1732,12 +1732,12 @@ func prepareSetters(ctx context.Context, clusterSummary *configv1beta1.ClusterSu
 		pullmode.WithDeployedGVKs(gvks))
 
 	if includeDeployChecks {
-		preDeployCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(),
+		preDeployCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterClient(),
 			clusterSummary, clusterSummary.Spec.ClusterProfileSpec.PreDeployChecks, logger)
 		if err != nil {
 			return nil, err
 		}
-		validateHealthJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(),
+		validateHealthJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterClient(),
 			clusterSummary, clusterSummary.Spec.ClusterProfileSpec.ValidateHealths, logger)
 		if err != nil {
 			return nil, err
@@ -1751,12 +1751,12 @@ func prepareSetters(ctx context.Context, clusterSummary *configv1beta1.ClusterSu
 	}
 
 	if includeDeleteChecks {
-		preDeleteCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(),
+		preDeleteCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterClient(),
 			clusterSummary, clusterSummary.Spec.ClusterProfileSpec.PreDeleteChecks, logger)
 		if err != nil {
 			return nil, err
 		}
-		postDeleteCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(),
+		postDeleteCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterClient(),
 			clusterSummary, clusterSummary.Spec.ClusterProfileSpec.PostDeleteChecks, logger)
 		if err != nil {
 			return nil, err
@@ -1897,7 +1897,7 @@ func getReloaderClient(ctx context.Context, clusterNamespace, clusterName string
 	// ResourceSummary is a Sveltos resource created in managed clusters.
 	// Sveltos resources are always created using cluster-admin so that admin does not need to be
 	// given such permissions.
-	return clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(),
+	return clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterClient(),
 		clusterNamespace, clusterName, "", "", clusterType, logger)
 }
 
@@ -2004,7 +2004,7 @@ func validatePreDeployChecks(ctx context.Context, c client.Client, clusterSummar
 	// direct client rather than c.
 	cacheMgr := clustercache.GetManager()
 
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName,
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterNamespace, clusterName,
 		adminNamespace, adminName, clusterType, logger)
 	if err != nil {
 		return err

controllers/helmchart_version_source.go

@@ -64,10 +64,8 @@ func resolveChartCredentials(ctx context.Context, secretRef *corev1.SecretRefere
 		return "", "", nil
 	}
 
-	// Not a ClusterProfileSecretType Secret, so it is never in the (possibly scoped) cache;
-	// use the direct client rather than c.
 	secret := &corev1.Secret{}
-	if getErr := getManagementClusterDirectClient().Get(ctx,
+	if getErr := getManagementClusterClient().Get(ctx,
 		types.NamespacedName{Namespace: secretRef.Namespace, Name: secretRef.Name}, secret); getErr != nil {
 		return "", "", getErr
 	}

controllers/management_cluster.go

@@ -30,8 +30,7 @@ import (
 )
 
 var (
-	managementClusterClient       client.Client
-	managementClusterDirectClient client.Client
+	managementClusterClient client.Client
 
 	managementClusterConfig          *rest.Config
 	managementClusterMapper          *restmapper.DeferredDiscoveryRESTMapper
@@ -53,20 +52,6 @@ func SetManagementClusterAccess(c client.Client, config *rest.Config, dc *discov
 
 	managementClusterCachedDiscovery = memory.NewMemCacheClient(dc)
 	managementClusterMapper = restmapper.NewDeferredDiscoveryRESTMapper(managementClusterCachedDiscovery)
-
-	// Uncached client for reads that must never be served - or silently hidden as NotFound -
-	// by a scoped cache. Concretely: kubeconfig Secrets for SveltosCluster/CAPI managed
-	// clusters are not of type ClusterProfileSecretType, so they fall outside the Secret cache
-	// scope --disable-secret-caching applies; reading them through the (possibly scoped)
-	// managementClusterClient would break cluster connectivity the moment that scoping is
-	// narrower than "every Secret". Built once here rather than per-call; on the rare
-	// construction failure, fall back to c so callers always get a usable client.
-	directClient, err := client.New(config, client.Options{Scheme: c.Scheme(), Mapper: managementClusterMapper})
-	if err != nil {
-		managementClusterDirectClient = c
-	} else {
-		managementClusterDirectClient = directClient
-	}
 }
 
 func SetDriftdetectionConfigMap(name string) {
@@ -109,14 +94,6 @@ func getManagementClusterClient() client.Client {
 	return managementClusterClient
 }
 
-// getManagementClusterDirectClient returns an uncached client to the management cluster.
-// Use this specifically for reads that must see every object of their type - kubeconfig
-// Secret lookups above all - regardless of any ByObject/field-selector scoping applied to
-// the (possibly cached) client getManagementClusterClient returns.
-func getManagementClusterDirectClient() client.Client {
-	return managementClusterDirectClient
-}
-
 func getManagementClusterMapper() *restmapper.DeferredDiscoveryRESTMapper {
 	return managementClusterMapper
 }

controllers/resourcesummary.go

@@ -208,7 +208,7 @@ func deployDriftDetectionCRDs(ctx context.Context, clusterNamespace, clusterName
 
 	var err error
 	cacheMgr := clustercache.GetManager()
-	remoteConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace,
+	remoteConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterNamespace,
 		clusterName, "", "", clusterType, logger)
 	if err != nil {
 		logger.V(logs.LogInfo).Error(err, "failed to get cluster rest config")
@@ -326,7 +326,7 @@ func deployDriftDetectionManagerInManagedCluster(ctx context.Context,
 
 	// Sveltos resources are deployed using cluster-admin role.
 	cacheMgr := clustercache.GetManager()
-	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(),
+	remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(),
 		clusterNamespace, clusterName, "", "", clusterType, logger)
 	if err != nil {
 		logger.V(logs.LogInfo).Error(err, "failed to get cluster rest config")
@@ -918,7 +918,7 @@ func getResourceSummaryClient(ctx context.Context, clusterNamespace, clusterName
 	// ResourceSummary is a Sveltos resource created in managed clusters.
 	// Sveltos resources are always created using cluster-admin so that admin does not need to be
 	// given such permissions.
-	return clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(),
+	return clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterClient(),
 		clusterNamespace, clusterName, "", "", clusterType, logger)
 }