diff --git a/controllers/clustersummary_watchers.go b/controllers/clustersummary_watchers.go index b5243d2b..c79fd542 100644 --- a/controllers/clustersummary_watchers.go +++ b/controllers/clustersummary_watchers.go @@ -24,12 +24,14 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" - "k8s.io/client-go/dynamic/dynamicinformer" - "k8s.io/client-go/informers" "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/cache" @@ -366,32 +368,39 @@ func (m *manager) startWatcher(ctx context.Context, gvk *schema.GroupVersionKind } logger.V(logs.LogInfo).Info("start watcher") - // dynamic informer needs to be told which type to watch - dcinformer, err := m.getDynamicInformer(gvk) + + lw, err := m.getListerWatcher(ctx, gvk) if err != nil { - logger.Error(err, "Failed to get informer") + logger.Error(err, "Failed to get lister watcher") return err } watcherCtx, cancel := context.WithCancel(ctx) //nolint:gosec // cancel is stored in m.watchers and called when the watcher is stopped m.watchers[*gvk] = cancel - go m.runInformer(watcherCtx.Done(), dcinformer.Informer(), logger) + + // Nothing here ever reads a resource back out of this watcher: react() below only needs + // the identity (and, for the "ignore status changes" case, the Generation) of the object + // carried by the event that fired it. Back the reflector with a store that forwards each + // event straight to react() and discards the object right after, instead of retaining + // every instance of gvk - cluster-wide, for whatever type is referenced by any + // ClusterSummary's policyRefs/kustomizationRefs/templateResourceRefs - in an indexed cache + // for as long as the watcher stays active, the way a SharedIndexInformer would. + store := newDiscardingStore(func(oldGeneration *int64, newObj client.Object) { + m.react(oldGeneration, newObj, logger) + }) + reflector := cache.NewReflector(lw, &unstructured.Unstructured{}, store, 0) + go reflector.Run(watcherCtx.Done()) + return nil } -func (m *manager) getDynamicInformer(gvk *schema.GroupVersionKind) (informers.GenericInformer, error) { - // Grab a dynamic interface that we can create informers from +// getListerWatcher returns a cache.ListerWatcher scoped to gvk's resource, cluster-wide - +// matching the scope the previous SharedIndexInformer-based watcher used. +func (m *manager) getListerWatcher(ctx context.Context, gvk *schema.GroupVersionKind) (cache.ListerWatcher, error) { d, err := dynamic.NewForConfig(m.config) if err != nil { return nil, err } - // Create a factory object that can generate informers for resource types - factory := dynamicinformer.NewFilteredDynamicSharedInformerFactory( - d, - 0, - corev1.NamespaceAll, - nil, - ) dc := discovery.NewDiscoveryClientForConfigOrDie(m.config) groupResources, err := restmapper.GetAPIGroupResources(dc) @@ -402,8 +411,7 @@ func (m *manager) getDynamicInformer(gvk *schema.GroupVersionKind) (informers.Ge mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { - // getDynamicInformer is only called after verifying resource - // is installed. + // getListerWatcher is only called after verifying resource is installed. return nil, err } @@ -413,33 +421,139 @@ func (m *manager) getDynamicInformer(gvk *schema.GroupVersionKind) (informers.Ge Resource: mapping.Resource.Resource, } - informer := factory.ForResource(resourceId) - return informer, nil -} - -func (m *manager) runInformer(stopCh <-chan struct{}, s cache.SharedIndexInformer, - logger logr.Logger) { + resourceClient := d.Resource(resourceId) - handlers := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - m.react(nil, obj.(client.Object), logger) + return &cache.ListWatch{ + ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { + return resourceClient.List(ctx, options) }, - DeleteFunc: func(obj interface{}) { - m.react(nil, obj.(client.Object), logger) - }, - UpdateFunc: func(oldObj, newObj interface{}) { - m.react(oldObj.(client.Object), newObj.(client.Object), logger) + WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { + return resourceClient.Watch(ctx, options) }, + }, nil +} + +// discardingStore implements cache.ReflectorStore for use with a cache.Reflector, but never +// retains a full copy of any object: every Add/Update/Delete - including the ones replayed +// from the initial List - is forwarded straight to the handler and then dropped. The only +// state kept is, per watched object, its last-seen Generation (a single int64, not the whole +// object): templateResourceRefsWatchedIgnoreStatus consumers only care whether Spec/Metadata +// changed (a Generation bump), not status-only updates, and a Reflector's ReflectorStore.Update +// is only ever given the new object, never the old one - so that one comparison is the one +// piece of state this store can't discard. +type discardingStore struct { + handler func(oldGeneration *int64, newObj client.Object) + + mu sync.Mutex + generations map[corev1.ObjectReference]int64 +} + +func newDiscardingStore(handler func(oldGeneration *int64, newObj client.Object)) *discardingStore { + return &discardingStore{ + handler: handler, + generations: make(map[corev1.ObjectReference]int64), } - _, err := s.AddEventHandler(handlers) - if err != nil { - panic(1) +} + +func refForObject(obj interface{}) (ref corev1.ObjectReference, o client.Object, ok bool) { + u, ok := obj.(*unstructured.Unstructured) + if !ok { + return corev1.ObjectReference{}, nil, false } - s.Run(stopCh) + + ref = corev1.ObjectReference{ + Kind: u.GetObjectKind().GroupVersionKind().Kind, + APIVersion: u.GetObjectKind().GroupVersionKind().GroupVersion().String(), + Namespace: u.GetNamespace(), + Name: u.GetName(), + } + return ref, u, true +} + +func (s *discardingStore) Add(obj interface{}) error { + ref, o, ok := refForObject(obj) + if !ok { + return nil + } + + s.mu.Lock() + s.generations[ref] = o.GetGeneration() + s.mu.Unlock() + + s.handler(nil, o) + return nil +} + +func (s *discardingStore) Update(obj interface{}) error { + ref, o, ok := refForObject(obj) + if !ok { + return nil + } + + s.mu.Lock() + oldGeneration, known := s.generations[ref] + s.generations[ref] = o.GetGeneration() + s.mu.Unlock() + + if known { + s.handler(&oldGeneration, o) + } else { + s.handler(nil, o) + } + return nil +} + +func (s *discardingStore) Delete(obj interface{}) error { + ref, o, ok := refForObject(obj) + if !ok { + return nil + } + + s.mu.Lock() + delete(s.generations, ref) + s.mu.Unlock() + + s.handler(nil, o) + return nil +} + +// Replace is invoked by the Reflector after each (re)list, once per listed object. Those are +// treated the same way an informer's initial sync would: as Add events. +func (s *discardingStore) Replace(list []interface{}, _ string) error { + for i := range list { + if err := s.Add(list[i]); err != nil { + return err + } + } + return nil +} + +func (s *discardingStore) Resync() error { + return nil +} + +func (s *discardingStore) List() []interface{} { + return nil +} + +func (s *discardingStore) ListKeys() []string { + return nil +} + +func (s *discardingStore) Get(obj interface{}) (item interface{}, exists bool, err error) { + return nil, false, nil +} + +func (s *discardingStore) GetByKey(key string) (item interface{}, exists bool, err error) { + return nil, false, nil } -// react gets called when an instance of passed in gvk has been modified. -func (m *manager) react(oldObj, newObj client.Object, logger logr.Logger) { +// react gets called when an instance of the watched gvk has been added, deleted, or updated. +// oldGeneration is the object's previously observed Generation - nil if this is the first time +// it's been seen (Add/Delete events are always treated this way) - used only to decide whether +// templateResourceRefsWatchedIgnoreStatus consumers should be notified of what would otherwise +// be a status-only change. +func (m *manager) react(oldGeneration *int64, newObj client.Object, logger logr.Logger) { m.watchMu.RLock() defer m.watchMu.RUnlock() @@ -476,7 +590,7 @@ func (m *manager) react(oldObj, newObj client.Object, logger logr.Logger) { // Consumers ignoring Status changes if v, ok := m.templateResourceRefsWatchedIgnoreStatus[ref]; ok { // - Or the Generation has increased (Spec/Metadata change) - if oldObj == nil || oldObj.GetGeneration() != newObj.GetGeneration() { + if oldGeneration == nil || *oldGeneration != newObj.GetGeneration() { m.notify(v, logger) } else { logger.V(logs.LogDebug).Info("skipping notification: only status changed") diff --git a/controllers/delete_checks.go b/controllers/delete_checks.go index ac811938..417ea4e1 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, getManagementClusterClient(), + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), 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 97923dff..08672ada 100644 --- a/controllers/drift_detection_upgrade.go +++ b/controllers/drift_detection_upgrade.go @@ -289,8 +289,10 @@ func skipUpgrading(ctx context.Context, c client.Client, cluster client.Object, return ok, nil } + // 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, c, cluster.GetNamespace(), cluster.GetName(), + managedClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterDirectClient(), 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/handlers_helm.go b/controllers/handlers_helm.go index 41d398c8..80d493cc 100644 --- a/controllers/handlers_helm.go +++ b/controllers/handlers_helm.go @@ -158,7 +158,9 @@ func deployHelmCharts(ctx context.Context, c client.Client, var kubeconfig string if !isPullMode { adminNamespace, adminName := getClusterSummaryAdmin(clusterSummary) - remoteRestConfig, restErr := clustercache.GetManager().GetKubernetesRestConfig(ctx, c, + // Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the + // direct client rather than c. + remoteRestConfig, restErr := clustercache.GetManager().GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger) if restErr != nil { @@ -275,8 +277,10 @@ func postProcessDeployedHelmCharts(ctx context.Context, clusterSummary *configv1 return err } + // 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, c, clusterNamespace, clusterName, + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger) if err != nil { return err @@ -361,7 +365,9 @@ func undeployHelmCharts(ctx context.Context, c client.Client, return undeployHelmChartsInPullMode(ctx, c, clusterSummary, mgmtResources, logger) } - remoteRestConfig, err := clustercache.GetManager().GetKubernetesRestConfig(ctx, c, + // Kubeconfig Secret read: must bypass any Secret-cache scoping, so use the + // direct client rather than c. + remoteRestConfig, err := clustercache.GetManager().GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger) if err != nil { return err @@ -1735,8 +1741,10 @@ 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 = getManagementClusterClient().Get(ctx, + err = getManagementClusterDirectClient().Get(ctx, types.NamespacedName{ Namespace: credentialSecretNamespace, Name: currentChart.RegistryCredentialsConfig.CredentialsSecretRef.Name, @@ -2832,7 +2840,7 @@ func recoverRelease(ctx context.Context, clusterSummary *configv1beta1.ClusterSu requestedChart.ReleaseNamespace, secretName)) cacheMgr := clustercache.GetManager() - remoteClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterClient(), + remoteClient, err := cacheMgr.GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType, logger) if err != nil { @@ -4879,8 +4887,10 @@ 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 = c.Get(ctx, + err = getManagementClusterDirectClient().Get(ctx, types.NamespacedName{ Namespace: namespace, Name: credSecretRef.Name, @@ -4935,8 +4945,10 @@ 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 = c.Get(ctx, + err = getManagementClusterDirectClient().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 299c8e8f..53ca41b1 100644 --- a/controllers/handlers_helm_test.go +++ b/controllers/handlers_helm_test.go @@ -1786,27 +1786,45 @@ resources: credentialsBytes, err := json.Marshal(credentials) Expect(err).To(BeNil()) + // getCredentialsAndCAFiles reads these Secrets through getManagementClusterDirectClient(), + // 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. + credentialsNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: randomString(), + }, + } + Expect(testEnv.Create(context.TODO(), credentialsNamespace)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv, credentialsNamespace)).To(Succeed()) + secretCredentials := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Namespace: randomString(), + Namespace: credentialsNamespace.Name, Name: randomString(), }, Data: map[string][]byte{ - "config.json": credentialsBytes, + // The real API server (this Secret is now created via testEnv, not the local + // fake client) enforces this exact key for type=kubernetes.io/dockerconfigjson. + corev1.DockerConfigJsonKey: credentialsBytes, }, Type: corev1.SecretTypeDockerConfigJson, } + Expect(testEnv.Create(context.TODO(), secretCredentials)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv, secretCredentials)).To(Succeed()) caByte := []byte(randomString()) secretCA := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Namespace: randomString(), + Namespace: credentialsNamespace.Name, Name: randomString(), }, Data: map[string][]byte{ "ca.crt": caByte, }, } + Expect(testEnv.Create(context.TODO(), secretCA)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv, secretCA)).To(Succeed()) cluster := &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/handlers_kustomize.go b/controllers/handlers_kustomize.go index 9e5e823a..6824c384 100644 --- a/controllers/handlers_kustomize.go +++ b/controllers/handlers_kustomize.go @@ -101,7 +101,7 @@ func deployKustomizeRefs(ctx context.Context, c client.Client, return err } - remoteRestConfig, logger, err := getRestConfig(ctx, c, clusterSummary, logger) + remoteRestConfig, logger, err := getRestConfig(ctx, clusterSummary, logger) if err != nil { return err } @@ -260,7 +260,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 := clusterproxy.GetKubernetesClient(ctx, getManagementClusterClient(), + remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType, logger) if err != nil { @@ -268,7 +268,7 @@ func cleanStaleKustomizeResources(ctx context.Context, clusterSummary *configv1b } cacheMgr := clustercache.GetManager() - remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterSummary.Spec.ClusterNamespace, + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), 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 4eff46cd..86ce56e5 100644 --- a/controllers/handlers_resources.go +++ b/controllers/handlers_resources.go @@ -62,7 +62,7 @@ func deployResources(ctx context.Context, c client.Client, return err } - remoteRestConfig, logger, err := getRestConfig(ctx, c, clusterSummary, logger) + remoteRestConfig, logger, err := getRestConfig(ctx, clusterSummary, logger) if err != nil { return err } @@ -217,7 +217,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 := clusterproxy.GetKubernetesClient(ctx, getManagementClusterClient(), + remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType, logger) if err != nil { @@ -225,7 +225,7 @@ func cleanStaleResources(ctx context.Context, clusterSummary *configv1beta1.Clus } cacheMgr := clustercache.GetManager() - remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterSummary.Spec.ClusterNamespace, + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, "", "", clusterSummary.Spec.ClusterType, logger) if err != nil { return nil, nil, err @@ -437,14 +437,17 @@ 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, c, clusterNamespace, clusterName, + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, "", "", clusterSummary.Spec.ClusterType, logger) if err != nil { return err } - remoteClient, err := clusterproxy.GetKubernetesClient(ctx, c, clusterNamespace, clusterName, + remoteClient, err := clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, "", "", clusterSummary.Spec.ClusterType, logger) if err != nil { return err diff --git a/controllers/handlers_utils.go b/controllers/handlers_utils.go index e75e1c2d..0c76eab9 100644 --- a/controllers/handlers_utils.go +++ b/controllers/handlers_utils.go @@ -740,7 +740,9 @@ func getClusterSummaryAndClusterClient(ctx context.Context, clusterNamespace, cl } adminNamespace, adminName := getClusterSummaryAdmin(clusterSummary) - clusterClient, err := clusterproxy.GetKubernetesClient(ctx, c, clusterSummary.Spec.ClusterNamespace, + // 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, clusterSummary.Spec.ClusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger) if err != nil { return nil, nil, err @@ -1362,7 +1364,7 @@ func tranformGroupVersionKindToString(gvks []schema.GroupVersionKind) []string { } // getRestConfig returns restConfig to access remote cluster -func getRestConfig(ctx context.Context, c client.Client, clusterSummary *configv1beta1.ClusterSummary, +func getRestConfig(ctx context.Context, clusterSummary *configv1beta1.ClusterSummary, logger logr.Logger) (*rest.Config, logr.Logger, error) { clusterNamespace := clusterSummary.Spec.ClusterNamespace @@ -1373,8 +1375,10 @@ func getRestConfig(ctx context.Context, c client.Client, clusterSummary *configv WithValues("clusterSummary", clusterSummary.Name).WithValues("admin", fmt.Sprintf("%s/%s", adminNamespace, adminName)) logger.V(logs.LogDebug).Info("get remote restConfig") + // 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, c, clusterNamespace, clusterName, + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, adminNamespace, adminName, clusterSummary.Spec.ClusterType, logger) if err != nil { return nil, logger, err @@ -1809,7 +1813,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 clusterproxy.GetKubernetesClient(ctx, getManagementClusterClient(), + return clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, "", "", clusterType, logger) } @@ -1912,9 +1916,11 @@ func validatePreDeployChecks(ctx context.Context, c client.Client, clusterSummar clusterName := clusterSummary.Spec.ClusterName clusterType := clusterSummary.Spec.ClusterType + // 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, c, clusterNamespace, clusterName, + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, adminNamespace, adminName, clusterType, logger) if err != nil { return err diff --git a/controllers/helmchart_outdated_check.go b/controllers/helmchart_outdated_check.go index 345b97ef..146e9014 100644 --- a/controllers/helmchart_outdated_check.go +++ b/controllers/helmchart_outdated_check.go @@ -263,7 +263,7 @@ func processHelmChartKey(ctx context.Context, c client.Client, key helmChartKey, } } - versions, err := fetchAvailableVersions(ctx, c, key, credentialsSecretRef, logger) + versions, err := fetchAvailableVersions(ctx, key, credentialsSecretRef, logger) if err != nil { logger.V(logs.LogInfo).Info(fmt.Sprintf("skipping chart %q in %q: %v", key.chartName, key.repositoryURL, err)) recordHelmChartCheckFailure() diff --git a/controllers/helmchart_version_source.go b/controllers/helmchart_version_source.go index bac253c6..c1f8a68d 100644 --- a/controllers/helmchart_version_source.go +++ b/controllers/helmchart_version_source.go @@ -33,7 +33,6 @@ import ( repo "helm.sh/helm/v4/pkg/repo/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" logs "github.com/projectsveltos/libsveltos/lib/logsettings" ) @@ -58,15 +57,18 @@ const ( // setResolvedHelmChartIdentity at deploy time, so no cluster context is needed here) from the // management cluster and extracts username/password for hostname. Returns empty strings, no // error, when secretRef is nil (anonymous access). -func resolveChartCredentials(ctx context.Context, c client.Client, secretRef *corev1.SecretReference, +func resolveChartCredentials(ctx context.Context, secretRef *corev1.SecretReference, hostname string) (username, password string, err error) { if secretRef == nil { 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 := c.Get(ctx, types.NamespacedName{Namespace: secretRef.Namespace, Name: secretRef.Name}, secret); getErr != nil { + if getErr := getManagementClusterDirectClient().Get(ctx, + types.NamespacedName{Namespace: secretRef.Namespace, Name: secretRef.Name}, secret); getErr != nil { return "", "", getErr } @@ -78,7 +80,7 @@ func resolveChartCredentials(ctx context.Context, c client.Client, secretRef *co // credentialsSecretRef, if non-nil, is used to authenticate the request; on any credential // resolution failure this falls back to an anonymous request rather than failing outright, // since that request may still succeed for a public chart. -func fetchAvailableVersions(ctx context.Context, c client.Client, key helmChartKey, +func fetchAvailableVersions(ctx context.Context, key helmChartKey, credentialsSecretRef *corev1.SecretReference, logger logr.Logger) ([]string, error) { fetchCtx, cancel := context.WithTimeout(ctx, fetchAvailableVersionsTimeout) @@ -91,7 +93,7 @@ func fetchAvailableVersions(ctx context.Context, c client.Client, key helmChartK return nil, fmt.Errorf("failed to parse repository URL %q: %w", key.repositoryURL, err) } - username, password, credErr := resolveChartCredentials(fetchCtx, c, credentialsSecretRef, parsedURL.Host) + username, password, credErr := resolveChartCredentials(fetchCtx, credentialsSecretRef, parsedURL.Host) if credErr != nil { logger.V(logs.LogDebug).Info(fmt.Sprintf("failed to resolve credentials for %s, trying anonymously: %v", key.repositoryURL, credErr)) diff --git a/controllers/management_cluster.go b/controllers/management_cluster.go index d0087a57..5e155133 100644 --- a/controllers/management_cluster.go +++ b/controllers/management_cluster.go @@ -30,7 +30,9 @@ import ( ) var ( - managementClusterClient client.Client + managementClusterClient client.Client + managementClusterDirectClient client.Client + managementClusterConfig *rest.Config managementClusterMapper *restmapper.DeferredDiscoveryRESTMapper managementClusterCachedDiscovery discovery.CachedDiscoveryInterface @@ -51,6 +53,20 @@ 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) { @@ -93,6 +109,14 @@ 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 064ea7c6..175f606a 100644 --- a/controllers/resourcesummary.go +++ b/controllers/resourcesummary.go @@ -133,7 +133,7 @@ func deployDriftDetectionCRDs(ctx context.Context, clusterNamespace, clusterName var err error cacheMgr := clustercache.GetManager() - remoteConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), clusterNamespace, + remoteConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, "", "", clusterType, logger) if err != nil { logger.V(logs.LogInfo).Error(err, "failed to get cluster rest config") @@ -251,7 +251,7 @@ func deployDriftDetectionManagerInManagedCluster(ctx context.Context, // Sveltos resources are deployed using cluster-admin role. cacheMgr := clustercache.GetManager() - remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterClient(), + remoteRestConfig, err := cacheMgr.GetKubernetesRestConfig(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, "", "", clusterType, logger) if err != nil { logger.V(logs.LogInfo).Error(err, "failed to get cluster rest config") @@ -836,7 +836,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 clusterproxy.GetKubernetesClient(ctx, getManagementClusterClient(), + return clustercache.GetManager().GetKubernetesClient(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, "", "", clusterType, logger) } diff --git a/controllers/resourcesummary_collection.go b/controllers/resourcesummary_collection.go index 9346443e..6ece024d 100644 --- a/controllers/resourcesummary_collection.go +++ b/controllers/resourcesummary_collection.go @@ -327,7 +327,12 @@ func collectResourceSummariesFromCluster(ctx context.Context, c client.Client, c return nil } - if !sveltos_upgrade.IsDriftDetectionVersionCompatible(ctx, getManagementClusterClient(), getSveltosNamespace(), version, + // 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, 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 5542f4e7..5a9de47b 100644 --- a/controllers/source_integrity.go +++ b/controllers/source_integrity.go @@ -94,8 +94,10 @@ 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 := getManagementClusterClient().Get(ctx, + if err := getManagementClusterDirectClient().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", @@ -222,8 +224,10 @@ 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 := getManagementClusterClient().Get(ctx, + if err := getManagementClusterDirectClient().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 70e32b11..54e9b692 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, getManagementClusterClient(), clusterNamespace, + mapper, err = cacheMgr.GetMapper(ctx, getManagementClusterDirectClient(), clusterNamespace, clusterName, clusterType, logger) if err != nil { return false, err diff --git a/go.mod b/go.mod index 341e3f32..66f6fa91 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/onsi/gomega v1.42.1 github.com/opencontainers/image-spec v1.1.1 github.com/pkg/errors v0.9.1 - github.com/projectsveltos/libsveltos v1.13.1-0.20260814131235-7d76aa957079 + github.com/projectsveltos/libsveltos v1.13.1-0.20260817055546-e9cd5c7232a0 github.com/prometheus/client_golang v1.24.1 github.com/sigstore/cosign/v3 v3.1.3 github.com/sigstore/sigstore v1.10.9 diff --git a/go.sum b/go.sum index 2790905b..af60191b 100644 --- a/go.sum +++ b/go.sum @@ -639,8 +639,8 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/poy/onpar v1.1.2 h1:QaNrNiZx0+Nar5dLgTVp5mXkyoVFIbepjyEoGSnhbAY= github.com/poy/onpar v1.1.2/go.mod h1:6X8FLNoxyr9kkmnlqpK6LSoiOtrO6MICtWwEuWkLjzg= -github.com/projectsveltos/libsveltos v1.13.1-0.20260814131235-7d76aa957079 h1:ZrE9xrYQ2QbcYanyMDWYr6UG7/iqq6UQf00kGnOSMLc= -github.com/projectsveltos/libsveltos v1.13.1-0.20260814131235-7d76aa957079/go.mod h1:dgQKoyCm3xr5TV0vJxbq+dkuBLmwm4mqt6tMAgJiP+s= +github.com/projectsveltos/libsveltos v1.13.1-0.20260817055546-e9cd5c7232a0 h1:LK8aK1edMBB1Et3fkdY6dzfvIVZ9eZ4BEp3aa7kt7oc= +github.com/projectsveltos/libsveltos v1.13.1-0.20260817055546-e9cd5c7232a0/go.mod h1:dgQKoyCm3xr5TV0vJxbq+dkuBLmwm4mqt6tMAgJiP+s= github.com/projectsveltos/lua-utils/glua-json v0.0.0-20251212200258-2b3cdcb7c0f5 h1:khnc+994UszxZYu69J+R5FKiLA/Nk1JQj0EYAkwTWz0= github.com/projectsveltos/lua-utils/glua-json v0.0.0-20251212200258-2b3cdcb7c0f5/go.mod h1:yVL8KQFa9tmcxgwl9nwIMtKgtmIVC1zaFRSCfOwYvPY= github.com/projectsveltos/lua-utils/glua-runes v0.0.0-20251212200258-2b3cdcb7c0f5 h1:YbsebwRwTRhV8QacvEAdFqxcxHdeu7JTVtsBovbkgos= diff --git a/pkg/app/app.go b/pkg/app/app.go index 2a55e15a..5c69c7b1 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -202,19 +202,26 @@ func Run() { func getCacheConfig() (disableFor []client.Object, byObject map[client.Object]cache.ByObject) { 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)) + byObject[&corev1.Secret{}] = cache.ByObject{ + Field: fieldSelector, + } + if disableCaching { - // Note: Only Secrets with type addons.projectsveltos.io/cluster-profile are cached - // The default client of the manager won't use the cache for secrets at all. + // ConfigMaps have no equivalent type field to scope by, so this only forces ConfigMap + // reads to bypass the cache; it does not reduce the ConfigMap informer's memory + // footprint (ClusterSummaryReconciler watches all ConfigMaps cluster-wide unscoped). disableFor = []client.Object{ - &corev1.Secret{}, &corev1.ConfigMap{}, } - - fieldSelector := fields.OneTermEqualSelector("type", string(libsveltosv1beta1.ClusterProfileSecretType)) - - byObject[&corev1.Secret{}] = cache.ByObject{ - Field: fieldSelector, - } } return }