From 82b9a90e29413dd72f66294886b9a4eb7a27b165 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Thu, 10 Sep 2026 10:07:59 +0200 Subject: [PATCH] fix: restore kubeconfig Secret cache invalidation addon-controller never picks up a kubeconfig Secret's content changing until the pod restarts. This was reproduced: point a SveltosCluster's kubeconfig Secret at a new cluster's API server and delete the old cluster. Addon-controller keeps dialing the old, now-defunct IP indefinitely (dial tcp :6443: i/o timeout). PR #1920 scoped the manager's Secret cache to type == addons.projectsveltos.io/cluster-profile (policyRef Secrets only), made unconditional. The ClusterSummaryReconciler's Secret watch is built on that same scoped informer, so a kubeconfig Secret update for a Secret of a different type was never seen. The watch handler (requeueClusterSummaryForReference) already calls clustercache.RemoveSecret correctly on every Secret event it sees. The fix consist in flipping the scoping from an allowlist to a denylist. Cache every Secret except Helm's own release-history storage (type == helm.sh/release.v1), Fixes #1954 --- controllers/delete_checks.go | 2 +- controllers/drift_detection_upgrade.go | 2 +- controllers/export_test.go | 10 ++++----- controllers/handlers_helm.go | 20 +++++++----------- controllers/handlers_helm_test.go | 2 +- controllers/handlers_kustomize.go | 4 ++-- controllers/handlers_resources.go | 11 ++++------ controllers/handlers_resources_test.go | 9 ++++---- controllers/handlers_utils.go | 16 +++++++-------- controllers/helmchart_version_source.go | 4 +--- controllers/management_cluster.go | 25 +---------------------- controllers/resourcesummary.go | 6 +++--- controllers/resourcesummary_collection.go | 7 +------ controllers/source_integrity.go | 8 ++------ controllers/utils.go | 2 +- pkg/app/app.go | 10 ++------- 16 files changed, 44 insertions(+), 94 deletions(-) diff --git a/controllers/delete_checks.go b/controllers/delete_checks.go index 417ea4e1..ac811938 100644 --- a/controllers/delete_checks.go +++ b/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 { diff --git a/controllers/drift_detection_upgrade.go b/controllers/drift_detection_upgrade.go index 37d5d252..6458d15b 100644 --- a/controllers/drift_detection_upgrade.go +++ b/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") diff --git a/controllers/export_test.go b/controllers/export_test.go index ae941249..6adf2b97 100644 --- a/controllers/export_test.go +++ b/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 diff --git a/controllers/handlers_helm.go b/controllers/handlers_helm.go index f61ddb93..b15a4cf2 100644 --- a/controllers/handlers_helm.go +++ b/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, diff --git a/controllers/handlers_helm_test.go b/controllers/handlers_helm_test.go index 3b95aec7..a56a54e6 100644 --- a/controllers/handlers_helm_test.go +++ b/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. diff --git a/controllers/handlers_kustomize.go b/controllers/handlers_kustomize.go index d188e964..f86aa87e 100644 --- a/controllers/handlers_kustomize.go +++ b/controllers/handlers_kustomize.go @@ -272,7 +272,7 @@ 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 { @@ -280,7 +280,7 @@ func cleanStaleKustomizeResources(ctx context.Context, clusterSummary *configv1b } 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 diff --git a/controllers/handlers_resources.go b/controllers/handlers_resources.go index 71b35d5d..1e20966a 100644 --- a/controllers/handlers_resources.go +++ b/controllers/handlers_resources.go @@ -229,7 +229,7 @@ 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 { @@ -237,7 +237,7 @@ func cleanStaleResources(ctx context.Context, clusterSummary *configv1beta1.Clus } 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 diff --git a/controllers/handlers_resources_test.go b/controllers/handlers_resources_test.go index 489af729..17e5556f 100644 --- a/controllers/handlers_resources_test.go +++ b/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 diff --git a/controllers/handlers_utils.go b/controllers/handlers_utils.go index cef4032c..9cfb2c25 100644 --- a/controllers/handlers_utils.go +++ b/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 diff --git a/controllers/helmchart_version_source.go b/controllers/helmchart_version_source.go index c1f8a68d..1956cc12 100644 --- a/controllers/helmchart_version_source.go +++ b/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 } diff --git a/controllers/management_cluster.go b/controllers/management_cluster.go index 5e155133..88db3bbe 100644 --- a/controllers/management_cluster.go +++ b/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 } diff --git a/controllers/resourcesummary.go b/controllers/resourcesummary.go index 660a2280..39a80642 100644 --- a/controllers/resourcesummary.go +++ b/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) } diff --git a/controllers/resourcesummary_collection.go b/controllers/resourcesummary_collection.go index 6ece024d..9346443e 100644 --- a/controllers/resourcesummary_collection.go +++ b/controllers/resourcesummary_collection.go @@ -327,12 +327,7 @@ func collectResourceSummariesFromCluster(ctx context.Context, c client.Client, c return nil } - // When !isAgentInMgmtMode, IsDriftDetectionVersionCompatible internally calls - // clusterproxy.GetKubernetesClient to build a client for the managed cluster, which - // reads its kubeconfig Secret (e.g. the CAPI type=cluster.x-k8s.io/secret Secret) - not - // a ClusterProfileSecretType Secret, so it must never be read through the (possibly - // scoped) cached client. - if !sveltos_upgrade.IsDriftDetectionVersionCompatible(ctx, getManagementClusterDirectClient(), getSveltosNamespace(), version, + if !sveltos_upgrade.IsDriftDetectionVersionCompatible(ctx, getManagementClusterClient(), getSveltosNamespace(), version, cluster.Namespace, cluster.Name, clusterproxy.GetClusterType(clusterRef), getAgentInMgmtCluster(), logger) { msg := "compatibility checks failed" diff --git a/controllers/source_integrity.go b/controllers/source_integrity.go index 5a9de47b..5542f4e7 100644 --- a/controllers/source_integrity.go +++ b/controllers/source_integrity.go @@ -94,10 +94,8 @@ func verifyCosignSignature(ctx context.Context, requestedChart *configv1beta1.He if ns == "" { ns = secretNamespace } - // Not a ClusterProfileSecretType Secret, so it is never in the (possibly scoped) cache; - // use the direct client. secret := &corev1.Secret{} - if err := getManagementClusterDirectClient().Get(ctx, + if err := getManagementClusterClient().Get(ctx, types.NamespacedName{Namespace: ns, Name: sv.SecretRef.Name}, secret); err != nil { return fmt.Errorf("cosign: failed to get public key secret %s/%s: %w", @@ -224,10 +222,8 @@ func createFileWithKeyring(ctx context.Context, clusterSummary *configv1beta1.Cl if ns == "" { ns = clusterSummary.Namespace } - // Not a ClusterProfileSecretType Secret, so it is never in the (possibly scoped) cache; - // use the direct client. secret := &corev1.Secret{} - if err := getManagementClusterDirectClient().Get(ctx, + if err := getManagementClusterClient().Get(ctx, types.NamespacedName{Namespace: ns, Name: secretRef.Name}, secret); err != nil { return "", fmt.Errorf("failed to get keyring secret %s/%s: %w", diff --git a/controllers/utils.go b/controllers/utils.go index a6611ecd..42338eaf 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -224,7 +224,7 @@ func isNamespaced(ctx context.Context, r *unstructured.Unstructured, clusterName mapper = getManagementClusterMapper() } else { cacheMgr := clustercache.GetManager() - mapper, err = cacheMgr.GetMapper(ctx, getManagementClusterDirectClient(), clusterNamespace, + mapper, err = cacheMgr.GetMapper(ctx, getManagementClusterClient(), clusterNamespace, clusterName, clusterType, logger) if err != nil { return false, err diff --git a/pkg/app/app.go b/pkg/app/app.go index 5c69c7b1..b2f55b24 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -203,14 +203,8 @@ func getCacheConfig() (disableFor []client.Object, byObject map[client.Object]ca disableFor = []client.Object{} byObject = map[client.Object]cache.ByObject{} - // Only Secrets of type addons.projectsveltos.io/cluster-profile (policyRefs) are ever read - // through a cached client. Every other Secret read in this codebase - kubeconfigs, Helm - // registry credentials/CA, cosign/GPG verification keys - goes through - // getManagementClusterDirectClient(), which always bypasses the cache. So this scoping is - // safe unconditionally, not just when --disable-secret-caching is set, and Secret is - // deliberately left out of disableFor below: the (now permanently scoped) cache is safe and - // cheaper than forcing every Secret read live. - fieldSelector := fields.OneTermEqualSelector("type", string(libsveltosv1beta1.ClusterProfileSecretType)) + // Cache every Secret except Helm's own release-history storage (type helm.sh/release.v1). + fieldSelector := fields.OneTermNotEqualSelector("type", "helm.sh/release.v1") byObject[&corev1.Secret{}] = cache.ByObject{ Field: fieldSelector, }