fix: check if resource is namespacescoped (#148)

* fix: check if resource is namespacescoped

* chore: reduce helm max history from 10 to 3 to avoid polluting the cluster

Author: matteogastaldello

README.md

@@ -205,7 +205,7 @@ These enviroment varibles can be changed in the Deployment of the composition-dy
 | URL_CHART_INSPECTOR                    | url to chart inspector   |  `http://chart-inspector.krateo-system.svc.cluster.local:8081/`             |   
 | KRATEO_NAMESPACE                       | namespace where krateo is installed       |  krateo-system |
 | HELM_REGISTRY_CONFIG_PATH | default helm config path | /tmp |
-| HELM_MAX_HISTORY | Max Helm History | 10 |
+| HELM_MAX_HISTORY | Max Helm History | 3 |
 | COMPOSITION_CONTROLLER_MAX_ERROR_RETRY_INTERVAL | The maximum interval between retries when an error occurs. This should be less than the half of the poll interval. |  0m |
 | COMPOSITION_CONTROLLER_MIN_ERROR_RETRY_INTERVAL | The minimum interval between retries when an error occurs. This should be less than max-error-retry-interval. | 1m |
 | COMPOSITION_CONTROLLER_METRICS_SERVER_PORT | The port where the metrics server will be listening. If not set, the metrics server is disabled. |  |

internal/composition/composition.go

@@ -35,7 +35,7 @@ var (
 	helmRegistryConfigPath = env.String(helmRegistryConfigPathEnvVar, helmclient.DefaultRegistryConfigPath)
 	krateoNamespace        = env.String(krateoNamespaceEnvVar, krateoNamespaceDefault)
 	helmRegistryConfigFile = filepath.Join(helmRegistryConfigPath, registry.CredentialsFileBasename)
-	helmMaxHistory         = env.Int(helmMaxHistoryEnvvar, 10)
+	helmMaxHistory         = env.Int(helmMaxHistoryEnvvar, 3)
 )
 
 const (
@@ -150,7 +150,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
 		return controller.ExternalObservation{}, fmt.Errorf("updating cr with values: %w", err)
 	}
 
-	hc, err := h.helmClientForResource(mg, pkg.RegistryAuth)
+	hc, _, err := h.helmClientForResource(mg, pkg.RegistryAuth)
 	if err != nil {
 		log.Error(err, "Getting helm client")
 		return controller.ExternalObservation{}, err
@@ -217,7 +217,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
 	}
 
 	tracer := &tracer.Tracer{}
-	hc, err = h.helmClientForResourceWithTransportWrapper(mg, pkg.RegistryAuth, func(rt http.RoundTripper) http.RoundTripper {
+	hc, _, err = h.helmClientForResourceWithTransportWrapper(mg, pkg.RegistryAuth, func(rt http.RoundTripper) http.RoundTripper {
 		return tracer.WithRoundTripper(rt)
 	})
 	if err != nil {
@@ -365,7 +365,7 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
 	}
 
 	// Install the helm chart
-	hc, err := h.helmClientForResource(mg, pkg.RegistryAuth)
+	hc, clientset, err := h.helmClientForResource(mg, pkg.RegistryAuth)
 	if err != nil {
 		log.Error(err, "Getting helm client")
 		return err
@@ -398,7 +398,7 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
 	}
 	log.Debug("Installing composition package", "package", pkg.URL)
 
-	all, err := helmchart.GetResourcesRefFromRelease(rel, mg.GetNamespace())
+	all, err := helmchart.GetResourcesRefFromRelease(rel, mg.GetNamespace(), clientset)
 	if err != nil {
 		log.Error(err, "Getting resources from release")
 		return fmt.Errorf("getting resources from release: %w", err)
@@ -473,7 +473,7 @@ func (h *handler) Update(ctx context.Context, mg *unstructured.Unstructured) err
 	}
 
 	// Update the helm chart
-	hc, err := h.helmClientForResource(mg, pkg.RegistryAuth)
+	hc, clientset, err := h.helmClientForResource(mg, pkg.RegistryAuth)
 	if err != nil {
 		log.Error(err, "Getting helm client")
 		return err
@@ -484,7 +484,7 @@ func (h *handler) Update(ctx context.Context, mg *unstructured.Unstructured) err
 		return err
 	}
 
-	all, err := helmchart.GetResourcesRefFromRelease(rel, mg.GetNamespace())
+	all, err := helmchart.GetResourcesRefFromRelease(rel, mg.GetNamespace(), clientset)
 	if err != nil {
 		log.Error(err, "Getting resources from release")
 		return fmt.Errorf("getting resources from release: %w", err)
@@ -576,7 +576,7 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err
 		return fmt.Errorf("helm chart package info getter must be specified")
 	}
 
-	hc, err := h.helmClientForResource(mg, nil)
+	hc, _, err := h.helmClientForResource(mg, nil)
 	if err != nil {
 		log.Error(err, "Getting helm client")
 		return err
@@ -667,7 +667,7 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err
 	return nil
 }
 
-func (h *handler) helmClientForResource(mg *unstructured.Unstructured, registryAuth *helmclient.RegistryAuth) (helmclient.Client, error) {
+func (h *handler) helmClientForResource(mg *unstructured.Unstructured, registryAuth *helmclient.RegistryAuth) (helmclient.Client, helmclient.CachedClientsInterface, error) {
 	log := h.logger.WithValues("apiVersion", mg.GetAPIVersion()).
 		WithValues("kind", mg.GetKind()).
 		WithValues("name", mg.GetName()).
@@ -676,7 +676,7 @@ func (h *handler) helmClientForResource(mg *unstructured.Unstructured, registryA
 	clientSet, err := helmclient.NewCachedClients(h.kubeconfig)
 	if err != nil {
 		log.Error(err, "Creating cached helm client set.")
-		return nil, err
+		return nil, nil, err
 	}
 
 	opts := &helmclient.Options{
@@ -700,16 +700,17 @@ func (h *handler) helmClientForResource(mg *unstructured.Unstructured, registryA
 		RegistryAuth: (registryAuth),
 	}
 
-	return helmclient.NewCachedClientFromRestConf(
+	hc, err := helmclient.NewCachedClientFromRestConf(
 		&helmclient.RestConfClientOptions{
 			Options:    opts,
 			RestConfig: h.kubeconfig,
 		},
-		&clientSet,
+		clientSet,
 	)
+	return hc, clientSet, err
 }
 
-func (h *handler) helmClientForResourceWithTransportWrapper(mg *unstructured.Unstructured, registryAuth *helmclient.RegistryAuth, transportWrapper func(http.RoundTripper) http.RoundTripper) (helmclient.Client, error) {
+func (h *handler) helmClientForResourceWithTransportWrapper(mg *unstructured.Unstructured, registryAuth *helmclient.RegistryAuth, transportWrapper func(http.RoundTripper) http.RoundTripper) (helmclient.Client, helmclient.CachedClientsInterface, error) {
 	opts := &helmclient.Options{
 		Namespace:        mg.GetNamespace(),
 		RepositoryCache:  "/tmp/.helmcache",
@@ -723,17 +724,18 @@ func (h *handler) helmClientForResourceWithTransportWrapper(mg *unstructured.Uns
 
 	clientSet, err := helmclient.NewCachedClients(h.kubeconfig)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	cfg := rest.CopyConfig(h.kubeconfig)
 	cfg.WrapTransport = transportWrapper
 
-	return helmclient.NewCachedClientFromRestConf(
+	hc, err := helmclient.NewCachedClientFromRestConf(
 		&helmclient.RestConfClientOptions{
 			Options:    opts,
 			RestConfig: cfg,
 		},
-		&clientSet,
+		clientSet,
 	)
+	return hc, clientSet, err
 }

internal/helmclient/client.go

@@ -80,7 +80,7 @@ func NewClientFromRestConf(options *RestConfClientOptions) (Client, error) {
 	return newClient(options.Options, clientGetter, settings)
 }
 
-func NewCachedClientFromRestConf(options *RestConfClientOptions, clientset *CachedClients) (Client, error) {
+func NewCachedClientFromRestConf(options *RestConfClientOptions, clientset CachedClientsInterface) (Client, error) {
 	settings := cli.New()
 
 	clientGetter := NewCachedRESTClientGetter(options.Namespace, nil, options.RestConfig, clientset)

internal/helmclient/client_getter.go

@@ -78,19 +78,26 @@ func (c *RESTClientGetter) ToRawKubeConfigLoader() clientcmd.ClientConfig {
 	return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, overrides)
 }
 
-type CachedClients struct {
+type CachedClientsInterface interface {
+	DiscoveryClient() discovery.CachedDiscoveryInterface
+	RESTMapper() meta.RESTMapper
+}
+
+var _ CachedClientsInterface = &cachedClients{}
+
+type cachedClients struct {
 	discoveryClient discovery.CachedDiscoveryInterface
 	_RESTMapper     meta.RESTMapper
 }
 
-func NewCachedClients(cfg *rest.Config) (CachedClients, error) {
+func NewCachedClients(cfg *rest.Config) (CachedClientsInterface, error) {
 	dir, err := os.MkdirTemp("", "helmclient")
 	if err != nil {
-		return CachedClients{}, err
+		return cachedClients{}, err
 	}
 	cachedDiscovery, err := disk.NewCachedDiscoveryClientForConfig(cfg, dir, "", 0)
 	if err != nil {
-		return CachedClients{}, err
+		return cachedClients{}, err
 	}
 
 	cachedDiscovery.Invalidate()
@@ -99,12 +106,20 @@ func NewCachedClients(cfg *rest.Config) (CachedClients, error) {
 	mapper.Reset()
 	expander := restmapper.NewShortcutExpander(mapper, cachedDiscovery, nil)
 
-	return CachedClients{
+	return cachedClients{
 		discoveryClient: cachedDiscovery,
 		_RESTMapper:     expander,
 	}, nil
 }
 
+func (c cachedClients) DiscoveryClient() discovery.CachedDiscoveryInterface {
+	return c.discoveryClient
+}
+
+func (c cachedClients) RESTMapper() meta.RESTMapper {
+	return c._RESTMapper
+}
+
 var _ clientcmd.ClientConfig = &namespaceClientConfig{}
 
 type namespaceClientConfig struct {
@@ -129,12 +144,12 @@ func (c namespaceClientConfig) ConfigAccess() clientcmd.ConfigAccess {
 
 // NewCachedRESTClientGetter returns a RESTClientGetter using the provided 'namespace', 'kubeConfig', and 'restConfig',
 // and uses cached clients for discovery and REST mapping to improve performance and reduce API server load.
-func NewCachedRESTClientGetter(namespace string, kubeConfig []byte, restConfig *rest.Config, clients *CachedClients, opts ...RESTClientOption) *CachedRESTClientGetter {
+func NewCachedRESTClientGetter(namespace string, kubeConfig []byte, restConfig *rest.Config, clients CachedClientsInterface, opts ...RESTClientOption) *CachedRESTClientGetter {
 	return &CachedRESTClientGetter{
 		kubeConfig:      kubeConfig,
 		restConfig:      restConfig,
-		discoveryClient: clients.discoveryClient,
-		restMapper:      clients._RESTMapper,
+		discoveryClient: clients.DiscoveryClient(),
+		restMapper:      clients.RESTMapper(),
 		namespaceConfig: &namespaceClientConfig{namespace: namespace},
 		opts:            opts,
 	}

internal/tools/helmchart/helmchart.go

@@ -6,13 +6,14 @@ import (
 	"io"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+
 	"github.com/krateoplatformops/plumbing/maps"
 	"github.com/krateoplatformops/unstructured-runtime/pkg/pluralizer"
 	unstructuredtools "github.com/krateoplatformops/unstructured-runtime/pkg/tools/unstructured"
 
 	"github.com/krateoplatformops/composition-dynamic-controller/internal/helmclient"
 	"github.com/krateoplatformops/unstructured-runtime/pkg/controller/objectref"
-	"github.com/krateoplatformops/unstructured-runtime/pkg/meta"
 
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/release"
@@ -84,39 +85,7 @@ type RenderTemplateOptions struct {
 	Pluralizer     pluralizer.PluralizerInterface
 }
 
-func RenderTemplate(ctx context.Context, opts RenderTemplateOptions) (*release.Release, []objectref.ObjectRef, error) {
-	dat, err := ExtractValuesFromSpec(opts.Resource)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	chartSpec := helmclient.ChartSpec{
-		ReleaseName: meta.GetReleaseName(opts.Resource),
-		Namespace:   opts.Resource.GetNamespace(),
-		ChartName:   opts.PackageUrl,
-		Version:     opts.PackageVersion,
-		ValuesYaml:  string(dat),
-		Repo:        opts.Repo,
-	}
-	if opts.Credentials != nil {
-		chartSpec.Username = opts.Credentials.Username
-		chartSpec.Password = opts.Credentials.Password
-	}
-
-	rel, err := opts.HelmClient.TemplateChartRaw(&chartSpec, nil)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	all, err := GetResourcesRefFromRelease(rel, opts.Resource.GetNamespace())
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to get resources from release: %w", err)
-	}
-
-	return rel, all, nil
-}
-
-func GetResourcesRefFromRelease(rel *release.Release, defaultNamespace string) ([]objectref.ObjectRef, error) {
+func GetResourcesRefFromRelease(rel *release.Release, defaultNamespace string, clientset helmclient.CachedClientsInterface) ([]objectref.ObjectRef, error) {
 	// build an io.Reader that streams manifest + hooks without concatenating into a single []byte
 	var readers []io.Reader
 	if rel != nil {
@@ -159,6 +128,16 @@ func GetResourcesRefFromRelease(rel *release.Release, defaultNamespace string) (
 
 		if namespace == "" {
 			namespace = defaultNamespace
+
+			// Check if the resource is cluster-scoped
+			gvk := schema.FromAPIVersionAndKind(apiVersion, kind)
+			mapping, err := clientset.RESTMapper().RESTMapping(gvk.GroupKind(), gvk.Version)
+			if err != nil {
+				return nil, fmt.Errorf("failed to get REST mapping for %s: %w", gvk.String(), err)
+			}
+			if mapping.Scope.Name() == meta.RESTScopeNameRoot {
+				namespace = ""
+			}
 		}
 		// skip helm hooks if present
 		if hook != "" {
@@ -203,7 +182,6 @@ func CheckResource(ctx context.Context, ref objectref.ObjectRef, opts CheckResou
 		un, err = opts.DynamicClient.Resource(gvr).
 			Get(ctx, ref.Name, metav1.GetOptions{})
 	}
-
 	if err != nil {
 		return nil, err
 	}
@@ -236,9 +214,7 @@ func FindAllReleases(hc helmclient.Client) ([]*release.Release, error) {
 	}
 
 	res := make([]*release.Release, 0, len(all))
-	for _, el := range all {
-		res = append(res, el)
-	}
+	res = append(res, all...)
 
 	return res, nil
 }