Fix missing apiVersion for NS obj and linter

Signed-off-by: Danil-Grigorev <[email protected]>

Author: Danil-Grigorev

internal/controller/genericprovider_controller.go

@@ -361,16 +361,14 @@ func calculateHash(ctx context.Context, k8sClient client.Client, provider generi
 func applyFromCache(ctx context.Context, cl client.Client, provider genericprovider.GenericProvider) (bool, error) {
 	log := log.FromContext(ctx)
 
-	configMap, err := providerCacheConfigMap(ctx, cl, provider)
-	if err != nil {
+	configMap := &corev1.ConfigMap{}
+	if err := cl.Get(ctx, client.ObjectKey{Name: ProviderCacheName(provider), Namespace: provider.GetNamespace()}, configMap); apierrors.IsNotFound(err) {
+		// config map does not exist, nothing to apply
+		return false, nil
+	} else if err != nil {
 		log.Error(err, "failed to get provider config map")
 
-		return false, err
-	}
-
-	// config map does not exist, nothing to apply
-	if configMap == nil {
-		return false, nil
+		return false, fmt.Errorf("failed to get cache ConfigMap: %w", err)
 	}
 
 	// calculate combined hash for provider and config map cache
@@ -395,6 +393,7 @@ func applyFromCache(ctx context.Context, cl client.Client, provider genericprovi
 	}
 
 	log.Info("Applying provider configuration from cache")
+
 	errs := []error{}
 
 	mr := configclient.NewMemoryReader()
@@ -404,7 +403,9 @@ func applyFromCache(ctx context.Context, cl client.Client, provider genericprovi
 	}
 
 	// Fetch configuration variables from the secret. See API field docs for more info.
-	initReaderVariables(ctx, cl, mr, provider)
+	if err := initReaderVariables(ctx, cl, mr, provider); err != nil {
+		return false, err
+	}
 
 	processor := yamlprocessor.NewSimpleProcessor()
 	for _, manifest := range configMap.Data {
@@ -422,7 +423,7 @@ func applyFromCache(ctx context.Context, cl client.Client, provider genericprovi
 			return false, err
 		}
 
-		if len(manifest) > 1 {
+		if len(manifests) > 1 {
 			return false, fmt.Errorf("multiple manifests found: %d", len(manifests))
 		} else if len(manifests) == 0 {
 			continue
@@ -441,20 +442,21 @@ func applyFromCache(ctx context.Context, cl client.Client, provider genericprovi
 			return false, err
 		}
 
-		manifest, err = processor.Process([]byte(manifest), mr.Get)
+		manifest, err = processor.Process(manifest, mr.Get)
 		if err != nil {
 			log.Error(err, "failed to process manifest")
 
 			return false, err
 		}
-		manifests, err := utilyaml.ToUnstructured([]byte(manifest))
+
+		manifests, err := utilyaml.ToUnstructured(manifest)
 		if err != nil {
 			log.Error(err, "failed to convert yaml to unstructured")
 
 			return false, err
 		}
 
-		if len(manifest) > 1 {
+		if len(manifests) > 1 {
 			return false, fmt.Errorf("multiple manifests found: %d", len(manifests))
 		} else if len(manifests) == 0 {
 			continue
@@ -478,9 +480,9 @@ func applyFromCache(ctx context.Context, cl client.Client, provider genericprovi
 
 // setCacheHash calculates current provider and configMap hash, and updates it on the configMap.
 func setCacheHash(ctx context.Context, cl client.Client, provider genericprovider.GenericProvider) error {
-	configMap, err := providerCacheConfigMap(ctx, cl, provider)
-	if err != nil {
-		return err
+	configMap := &corev1.ConfigMap{}
+	if err := cl.Get(ctx, client.ObjectKey{Name: ProviderCacheName(provider), Namespace: provider.GetNamespace()}, configMap); err != nil {
+		return fmt.Errorf("failed to get cache ConfigMaps: %w", err)
 	}
 
 	helper, err := patch.NewHelper(configMap, cl)

internal/controller/manifests_downloader.go

@@ -24,7 +24,6 @@ import (
 	"io"
 
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -44,10 +43,6 @@ const (
 	operatorManagedLabel      = "managed-by.operator.cluster.x-k8s.io"
 	operatorCacheLabel        = "cached-by.operator.cluster.x-k8s.io"
 
-	cacheVersionLabelName = "provider-cache.cluster.x-k8s.io/version"
-	cacheTypeLabel        = "provider-cache.cluster.x-k8s.io/type"
-	cacheNameLabel        = "provider-cache.cluster.x-k8s.io/name"
-
 	maxConfigMapSize = 1 * 1024 * 1024
 	ociSource        = "oci"
 )
@@ -330,18 +325,6 @@ func providerLabelSelector(provider operatorv1.GenericProvider) *metav1.LabelSel
 	}
 }
 
-// providerCacheConfigMap finds a cached ConfigMap the given provider label selector.
-func providerCacheConfigMap(ctx context.Context, cl client.Client, provider operatorv1.GenericProvider) (*corev1.ConfigMap, error) {
-	configMap := &corev1.ConfigMap{}
-	if err := cl.Get(ctx, client.ObjectKey{Name: ProviderCacheName(provider), Namespace: provider.GetNamespace()}, configMap); apierrors.IsNotFound(err) {
-		return nil, nil
-	} else if err != nil {
-		return nil, fmt.Errorf("failed to list ConfigMaps: %w", err)
-	}
-
-	return configMap, nil
-}
-
 // ProviderLabels returns default set of labels that identify a config map with downloaded manifests.
 func ProviderLabels(provider operatorv1.GenericProvider) map[string]string {
 	labels := map[string]string{

internal/controller/phases.go

@@ -26,13 +26,13 @@ import (
 
 	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/serializer"
 	"k8s.io/apimachinery/pkg/types"
 	versionutil "k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes/scheme"
-	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha2"
 	"sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider"
@@ -48,6 +48,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	utilyaml "sigs.k8s.io/cluster-api/util/yaml"
 )
 
 // fakeURL is the stub url for custom providers, missing from clusterctl repository.
@@ -322,7 +324,9 @@ func (p *PhaseReconciler) secretReader(ctx context.Context, providers ...configc
 	}
 
 	// Fetch configuration variables from the secret. See API field docs for more info.
-	initReaderVariables(ctx, p.ctrlClient, mr, p.provider)
+	if err := initReaderVariables(ctx, p.ctrlClient, mr, p.provider); err != nil {
+		return nil, err
+	}
 
 	isCustom := true
 
@@ -564,12 +568,14 @@ func (p *phaseReconciler) store(ctx context.Context) (reconcile.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 	log.Info("Storing provider in cache")
 
-	componentsFile, err := p.components.Yaml()
+	components := addNamespaceIfMissing(p.components.Objs(), p.provider.GetNamespace())
+
+	componentsFile, err := utilyaml.FromUnstructured(components)
 	if err != nil {
 		return reconcile.Result{}, wrapPhaseError(err, operatorv1.ComponentsCustomizationErrorReason, operatorv1.ProviderInstalledCondition)
 	}
 
-	kinds, _, err := clientgoscheme.Scheme.ObjectKinds(&corev1.ConfigMap{})
+	kinds, _, err := scheme.Scheme.ObjectKinds(&corev1.ConfigMap{})
 	if err != nil || len(kinds) == 0 {
 		log.Error(err, "cannot fetch kind of the ConfigMap resource")
 		err = fmt.Errorf("cannot fetch kind of the ConfigMap resource: %w", err)
@@ -587,7 +593,8 @@ func (p *phaseReconciler) store(ctx context.Context) (reconcile.Result, error) {
 			Namespace:   p.provider.GetNamespace(),
 			Annotations: map[string]string{},
 		},
-		Data: map[string]string{},
+		Data:       map[string]string{},
+		BinaryData: map[string][]byte{},
 	}
 
 	gvk := p.provider.GetObjectKind().GroupVersionKind()
@@ -606,7 +613,7 @@ func (p *phaseReconciler) store(ctx context.Context) (reconcile.Result, error) {
 		configMap.Annotations[operatorv1.CompressedAnnotation] = "true"
 	}
 
-	for i, manifest := range strings.Split(string(componentsFile), "---") {
+	for i, manifest := range strings.Split(string(componentsFile), "\n---") {
 		manifest = strings.TrimSpace(manifest)
 
 		if compress {
@@ -620,7 +627,7 @@ func (p *phaseReconciler) store(ctx context.Context) (reconcile.Result, error) {
 			continue
 		}
 
-		configMap.Data[fmt.Sprintf("%d", i)] = string(manifest)
+		configMap.Data[fmt.Sprintf("%d", i)] = manifest
 	}
 
 	if err := p.ctrlClient.Patch(ctx, configMap, client.Apply, client.ForceOwnership, client.FieldOwner(cacheOwner)); err != nil {
@@ -631,6 +638,7 @@ func (p *phaseReconciler) store(ctx context.Context) (reconcile.Result, error) {
 
 	// Perform template processing with variable replacement.
 	p.options.SkipTemplateProcess = false
+
 	p.components, err = repository.NewComponents(repository.ComponentsInput{
 		Provider:     p.providerConfig,
 		ConfigClient: p.configClient,
@@ -645,7 +653,34 @@ func (p *phaseReconciler) store(ctx context.Context) (reconcile.Result, error) {
 	return reconcile.Result{}, nil
 }
 
-// Upgrade ensure all the clusterctl CRDs are available before installing the provider,
+// addNamespaceIfMissing adda a Namespace object if missing (this ensure the targetNamespace will be created).
+func addNamespaceIfMissing(objs []unstructured.Unstructured, targetNamespace string) []unstructured.Unstructured {
+	namespaceObjectFound := false
+
+	for _, o := range objs {
+		// if the object has Kind Namespace, fix the namespace name
+		if o.GetKind() == namespaceKind {
+			namespaceObjectFound = true
+		}
+	}
+
+	// if there isn't an object with Kind Namespace, add it
+	if !namespaceObjectFound {
+		objs = append(objs, unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"apiVersion": "v1",
+				"kind":       namespaceKind,
+				"metadata": map[string]interface{}{
+					"name": targetNamespace,
+				},
+			},
+		})
+	}
+
+	return objs
+}
+
+// upgrade ensure all the clusterctl CRDs are available before installing the provider,
 // and update existing components if required.
 func (p *PhaseReconciler) Upgrade(ctx context.Context) (*Result, error) {
 	log := ctrl.LoggerFrom(ctx)
@@ -689,7 +724,7 @@ func (p *PhaseReconciler) Install(ctx context.Context) (*Result, error) {
 	log.Info("Installing provider")
 
 	if err := clusterClient.ProviderComponents().Create(ctx, p.components.Objs()); err != nil {
-		reason := "Install failed!"
+		reason := "Install failed"
 		if wait.Interrupted(err) {
 			reason = "Timed out waiting for deployment to become ready"
 		}