Support remote installation of modules in private registries (#688)

* fetching secret from cluster depending on the remoteModuleTemplateRef * Add remoteModuleTemplateRef in Kyma-Crd documentation * improve documentation * use targetCluster instead of KCP * revert flags change * changes based on remoteModuleTemplateRef * fixes * add test * fix linting * change name in test * revert timeout change * fix linting * fix error check * change name * remove test from kyma controller * Implement test * fix linting * remove extra line * Update docs/developer-tutorials/config-private-registry.md Co-authored-by: Małgorzata Świeca <[email protected]> * Update docs/technical-reference/api/kyma-cr.md Co-authored-by: Małgorzata Świeca <[email protected]> * Update docs/technical-reference/api/kyma-cr.md Co-authored-by: Małgorzata Świeca <[email protected]> * PR comment * Empty commit * add test * Empty commit * increase test timeout * increase test interval * increase test timeout * increase test interval --------- Co-authored-by: Małgorzata Świeca <[email protected]>
kyma-project · Jul 7, 2023 · 0fdaf79 · 0fdaf79
1 parent 24bb079
commit 0fdaf79
Show file tree

Hide file tree

Showing 14 changed files with 158 additions and 25 deletions.
diff --git a/api/v1beta2/operator_labels.go b/api/v1beta2/operator_labels.go
@@ -17,7 +17,8 @@ const (
 	Signature      = OperatorPrefix + Separator + "signature"
 	ModuleName     = OperatorPrefix + Separator + "module-name"
 	// Notice: This label is intended solely for testing purposes and should not be used in production module templates.
-	UseLocalTemplate = OperatorPrefix + Separator + "use-local-template"
+	UseLocalTemplate       = OperatorPrefix + Separator + "use-local-template"
+	IsRemoteModuleTemplate = OperatorPrefix + Separator + "remote-template"
 
 	//nolint:gosec
 	OCIRegistryCredLabel = "oci-registry-cred"

diff --git a/controllers/control-plane/kyma_remote_sync_test.go b/controllers/control-plane/kyma_remote_sync_test.go
@@ -22,6 +22,7 @@ var (
 	ErrNotContainsExpectedAnnotation = errors.New("kyma CR not contains expected CRD annotation")
 	ErrContainsUnexpectedAnnotation  = errors.New("kyma CR contains unexpected CRD annotation")
 	ErrAnnotationNotUpdated          = errors.New("kyma CR annotation not updated")
+	ErrRemoteTemplateLabelNotFound   = errors.New("manifest does not contain remote template label")
 )
 
 var _ = Describe("Kyma with remote module templates", Ordered, func() {
@@ -82,6 +83,21 @@ var _ = Describe("Kyma with remote module templates", Ordered, func() {
 			Should(Succeed())
 	})
 
+	It("Manifest should contain remoteModuleTemplate label", func() {
+		Eventually(func() error {
+			manifest, err := GetManifest(ctx, controlPlaneClient, kyma, moduleInSkr)
+			if err != nil {
+				return err
+			}
+
+			if manifest.Labels[v1beta2.IsRemoteModuleTemplate] != v1beta2.EnableLabelValue {
+				return ErrRemoteTemplateLabelNotFound
+			}
+			return nil
+		}, Timeout, Interval).
+			Should(Succeed())
+	})
+
 	It("Should not delete the module template on SKR upon Kyma deletion", func() {
 		Eventually(DeleteCR, Timeout, Interval).
 			WithContext(ctx).

diff --git a/controllers/kyma_test.go b/controllers/kyma_test.go
@@ -23,8 +23,9 @@ var _ = Describe("Kyma with no Module", Ordered, func() {
 
 	It("Should result in a ready state immediately", func() {
 		By("having transitioned the CR State to Ready as there are no modules")
-		Eventually(IsKymaInState(ctx, controlPlaneClient, kyma.GetName(), v1beta2.StateReady),
-			Timeout, Interval).Should(BeTrue())
+		Eventually(IsKymaInState, Timeout, Interval).
+			WithArguments(ctx, controlPlaneClient, kyma.GetName(), v1beta2.StateReady).
+			Should(BeTrue())
 	})
 })
 

diff --git a/controllers/purge/purge_controller_test.go b/controllers/purge/purge_controller_test.go
@@ -65,8 +65,9 @@ var _ = Describe("When kyma is not deleted within configured timeout", Ordered,
 		})
 
 		By("Target finalizers should be dropped", func() {
-			Eventually(IsKymaInState(ctx, controlPlaneClient, kyma.GetName(), v1beta2.StateDeleting),
-				Timeout, Interval).Should(BeTrue())
+			Eventually(IsKymaInState, Timeout, Interval).
+				WithArguments(ctx, controlPlaneClient, kyma.GetName(), v1beta2.StateDeleting).
+				Should(BeTrue())
 			Eventually(getIssuerFinalizers, Timeout, Interval).
 				WithContext(ctx).
 				WithArguments(client.ObjectKeyFromObject(issuer1), controlPlaneClient).

diff --git a/docs/developer-tutorials/config-private-registry.md b/docs/developer-tutorials/config-private-registry.md
@@ -37,7 +37,7 @@ Before you proceed, prepare your registry credentials. Check also how to deal wi
 
    > **NOTE:** The `"operator.kyma-project.io/managed-by": "lifecycle-manager"` label is mandatory for the Lifecycle Manager runtime controller to know which resources to cache.
 
-3. Deploy to the KCP cluster in each environment.
+3. Deploy the Secret in the same cluster where the ModuleTemplate is to be located. For example, if the ModuleTemplate is in the SKR cluster, then the Secret should be deployed to the SKR, otherwise, it should be deployed to the KCP cluster.
 
 ### Generate a ModuleTemplate CR with the `oci-registry-cred` label
 

diff --git a/docs/technical-reference/api/kyma-cr.md b/docs/technical-reference/api/kyma-cr.md
@@ -103,6 +103,10 @@ In addition to this very flexible way of referencing modules, there is also anot
 While `CreateAndDelete` will cause the ModuleTemplate's **.spec.data** to be created and deleted to initialize a module with preconfigured defaults, `Ignore` can be used to only initialize the operator without initializing any default data.
 This allows users to be fully flexible in regard to when and how to initialize their module.
 
+### **.spec.modules[].remoteModuleTemplateRef**
+The `remoteModuleTemplateRef` flag allows the users to have their ModuleTemplate CR fetched from the SKR cluster instead of Kyma Control Plane (KCP). It should be the reference (FQDN,
+Namespace/Name, or module name label) to the ModuleTemplate CR. If not specified, the ModuleTemplate CR is fetched from the KCP cluster.
+
 ### **.status.state**
 
 The **state** attribute is a simple representation of the state of the entire Kyma CR installation. It is defined as an aggregated status that is either `Ready`, `Processing`, `Error`, or `Deleting`, based on the status of _all_ Manifest CRs on top of the validity/integrity of the synchronization to a remote cluster if enabled.

diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go
@@ -220,7 +220,11 @@ func (r *Reconciler) initialize(obj Object) error {
 }
 
 func (r *Reconciler) Spec(ctx context.Context, obj Object) (*Spec, error) {
-	spec, err := r.SpecResolver.Spec(ctx, obj)
+	targetClient, err := r.getTargetClient(ctx, obj)
+	if err != nil {
+		return nil, err
+	}
+	spec, err := r.SpecResolver.Spec(ctx, obj, targetClient)
 	if err != nil {
 		r.Event(obj, "Warning", "Spec", err.Error())
 		obj.SetStatus(obj.GetStatus().WithState(StateError).WithErr(err))

diff --git a/internal/declarative/v2/spec.go b/internal/declarative/v2/spec.go
@@ -2,10 +2,12 @@ package v2
 
 import (
 	"context"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type SpecResolver interface {
-	Spec(ctx context.Context, object Object) (*Spec, error)
+	Spec(ctx context.Context, object Object, remoteClient client.Client) (*Spec, error)
 }
 
 type Spec struct {
@@ -33,7 +35,7 @@ type CustomSpecFns struct {
 }
 
 func (s *CustomSpecFns) Spec(
-	ctx context.Context, obj Object,
+	ctx context.Context, obj Object, _ client.Client,
 ) (*Spec, error) {
 	return &Spec{
 		ManifestName: s.ManifestNameFn(ctx, obj),

diff --git a/internal/manifest/v1beta1/manifest_controller_test.go b/internal/manifest/v1beta1/manifest_controller_test.go
@@ -4,15 +4,24 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/kyma-project/lifecycle-manager/api/v1beta2"
 	declarative "github.com/kyma-project/lifecycle-manager/internal/declarative/v2"
+	"github.com/kyma-project/lifecycle-manager/pkg/ocmextensions"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/apimachinery/pkg/util/yaml"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
-var ErrManifestStateMisMatch = errors.New("ManifestState mismatch")
+var (
+	ErrManifestStateMisMatch = errors.New("ManifestState mismatch")
+	ErrAuthSecretNotFound    = errors.New("auth secret error not found in manifest")
+	ErrNotInErrorState       = errors.New("manifest not found in error state")
+)
 
 var _ = Describe(
 	"Given manifest with OCI specs", func() {
@@ -98,3 +107,42 @@ var _ = Describe(
 		)
 	},
 )
+
+var _ = Describe(
+	"Given manifest with private registry", func() {
+		manifest := &v1beta2.Manifest{}
+		manifestPath := filepath.Join("../../../pkg/test_samples/oci", "private-registry-manifest.yaml")
+		manifestFile, err := os.ReadFile(manifestPath)
+		Expect(err).ToNot(HaveOccurred())
+		err = yaml.Unmarshal(manifestFile, manifest)
+		manifest.SetNamespace(metav1.NamespaceDefault)
+		manifest.SetName("private-registry-manifest")
+		manifest.SetLabels(map[string]string{
+			v1beta2.KymaName: string(uuid.NewUUID()),
+		})
+		manifest.SetResourceVersion("")
+		Expect(err).ToNot(HaveOccurred())
+
+		It("Should create Manifest", func() {
+			Expect(k8sClient.Create(ctx, manifest)).To(Succeed())
+		})
+
+		It("Manifest should be in Error state with no auth secret found error message", func() {
+			Eventually(func() error {
+				status, err := getManifestStatus(manifest.GetName())
+				if err != nil {
+					return err
+				}
+
+				if status.State != declarative.StateError {
+					return ErrNotInErrorState
+				}
+				if !strings.Contains(status.LastOperation.Operation, ocmextensions.ErrNoAuthSecretFound.Error()) {
+					return ErrAuthSecretNotFound
+				}
+				return nil
+			}, 3*standardTimeout, 3*standardInterval).
+				Should(Succeed())
+		})
+	},
+)
diff --git a/internal/manifest/v1beta1/spec_resolver.go b/internal/manifest/v1beta1/spec_resolver.go
@@ -45,7 +45,8 @@ var (
 	ErrInvalidObjectPassedToSpecResolution = errors.New("invalid object passed to spec resolution")
 )
 
-func (m *ManifestSpecResolver) Spec(ctx context.Context, obj declarative.Object) (*declarative.Spec, error) {
+func (m *ManifestSpecResolver) Spec(ctx context.Context, obj declarative.Object,
+	remoteClient client.Client) (*declarative.Spec, error) {
 	manifest, ok := obj.(*v1beta2.Manifest)
 	if !ok {
 		return nil, fmt.Errorf(
@@ -59,7 +60,12 @@ func (m *ManifestSpecResolver) Spec(ctx context.Context, obj declarative.Object)
 		return nil, err
 	}
 
-	rawManifestInfo, err := m.getRawManifestForInstall(ctx, manifest.Spec.Install, specType)
+	targetClient := m.KCP.Client
+	if manifest.Labels[v1beta2.IsRemoteModuleTemplate] == v1beta2.EnableLabelValue {
+		targetClient = remoteClient
+	}
+
+	rawManifestInfo, err := m.getRawManifestForInstall(ctx, manifest.Spec.Install, specType, targetClient)
 	if err != nil {
 		return nil, err
 	}
@@ -90,6 +96,7 @@ func (m *ManifestSpecResolver) getRawManifestForInstall(
 	ctx context.Context,
 	install v1beta2.InstallInfo,
 	specType v1beta2.RefTypeMetadata,
+	targetClient client.Client,
 ) (*RawManifestInfo, error) {
 	var err error
 	switch specType {
@@ -99,7 +106,7 @@ func (m *ManifestSpecResolver) getRawManifestForInstall(
 			return nil, err
 		}
 
-		keyChain, err := m.lookupKeyChain(ctx, imageSpec)
+		keyChain, err := m.lookupKeyChain(ctx, imageSpec, targetClient)
 		if err != nil {
 			return nil, err
 		}
@@ -123,12 +130,12 @@ func (m *ManifestSpecResolver) getRawManifestForInstall(
 }
 
 func (m *ManifestSpecResolver) lookupKeyChain(
-	ctx context.Context, imageSpec v1beta2.ImageSpec,
+	ctx context.Context, imageSpec v1beta2.ImageSpec, targetClient client.Client,
 ) (authn.Keychain, error) {
 	var keyChain authn.Keychain
 	var err error
 	if imageSpec.CredSecretSelector != nil {
-		if keyChain, err = ocmextensions.GetAuthnKeychain(ctx, imageSpec.CredSecretSelector, m.KCP.Client); err != nil {
+		if keyChain, err = ocmextensions.GetAuthnKeychain(ctx, imageSpec.CredSecretSelector, targetClient); err != nil {
 			return nil, err
 		}
 	} else {

diff --git a/pkg/module/common/module.go b/pkg/module/common/module.go
@@ -3,6 +3,7 @@ package common
 import (
 	"fmt"
 	"hash/fnv"
+	"strconv"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -46,7 +47,7 @@ func (m *Module) ApplyLabelsAndAnnotations(
 		lbls[v1beta2.ControllerName] = m.Template.GetLabels()[v1beta2.ControllerName]
 	}
 	lbls[v1beta2.ChannelLabel] = m.Template.Spec.Channel
-
+	lbls[v1beta2.IsRemoteModuleTemplate] = strconv.FormatBool(m.IsRemoteModuleTemplate(kyma))
 	lbls[v1beta2.ManagedBy] = v1beta2.OperatorName
 
 	m.SetLabels(lbls)
@@ -59,6 +60,16 @@ func (m *Module) ApplyLabelsAndAnnotations(
 	m.SetAnnotations(anns)
 }
 
+func (m *Module) IsRemoteModuleTemplate(kyma *v1beta2.Kyma) bool {
+	for _, module := range kyma.Spec.Modules {
+		if module.Name == m.ModuleName {
+			return module.RemoteModuleTemplateRef != ""
+		}
+	}
+
+	return false
+}
+
 func (m *Module) ContainsExpectedOwnerReference(ownerName string) bool {
 	if m.GetOwnerReferences() == nil {
 		return false

diff --git a/pkg/module/parse/template_to_module.go b/pkg/module/parse/template_to_module.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	"github.com/kyma-project/lifecycle-manager/api/v1beta2"
+	"github.com/kyma-project/lifecycle-manager/pkg/remote"
 	"github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -117,6 +118,7 @@ func overwriteNameAndNamespace(template *channel.ModuleTemplateTO, name, namespa
 	}
 }
 
+// nolint:funlen
 func (p *Parser) newManifestFromTemplate(
 	ctx context.Context,
 	module v1beta2.Module,
@@ -134,6 +136,11 @@ func (p *Parser) newManifestFromTemplate(
 		manifest.Spec.Resource = template.Spec.Data.DeepCopy()
 	}
 
+	clusterClient := p.Client
+	if module.RemoteModuleTemplateRef != "" {
+		clusterClient = remote.SyncContextFromContext(ctx).RuntimeClient
+	}
+
 	var layers img.Layers
 	var err error
 	descriptor, err := template.Spec.GetDescriptor()
@@ -150,14 +157,14 @@ func (p *Parser) newManifestFromTemplate(
 			return nil, err
 		}
 		componentDescriptor, err = p.ComponentDescriptorCache.GetRemoteDescriptor(ctx,
-			descriptorCacheKey, descriptor, p.Client)
+			descriptorCacheKey, descriptor, clusterClient)
 		if err != nil {
 			return nil, err
 		}
 	}
 
 	verification, err := signature.NewVerification(ctx,
-		p.Client,
+		clusterClient,
 		p.EnableVerification,
 		p.PublicKeyFilePath,
 		module.Name)

diff --git a/pkg/test_samples/oci/private-registry-manifest.yaml b/pkg/test_samples/oci/private-registry-manifest.yaml
@@ -0,0 +1,33 @@
+apiVersion: operator.kyma-project.io/v1beta2
+kind: Manifest
+metadata:
+  annotations:
+    operator.kyma-project.io/fqdn: kyma-project.io/template-operator
+spec:
+  config:
+    credSecretSelector:
+      matchLabels:
+        operator.kyma-project.io/oci-registry-cred: test-operator
+    name: kyma-project.io/template-operator
+    ref: sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+    repo: github.com/kyma-project/template-operator/component-descriptors
+    type: oci-ref
+  install:
+    name: raw-manifest
+    source:
+      credSecretSelector:
+        matchLabels:
+          operator.kyma-project.io/oci-registry-cred: test-operator
+      name: kyma-project.io/template-operator
+      ref: sha256:8f01a545b7f53f91dd9f0bb7b74b30dfa2e571dae8d38fd0f56af66f809378fd
+      repo: github.com/kyma-project/template-operator/component-descriptors
+      type: oci-ref
+  remote: true
+  resource:
+    apiVersion: operator.kyma-project.io/v1alpha1
+    kind: Sample
+    metadata:
+      name: sample-yaml
+      namespace: kyma-system
+    spec:
+      resourceFilePath: ./module-data/yaml
diff --git a/pkg/testutils/utils.go b/pkg/testutils/utils.go
@@ -192,14 +192,12 @@ func GetKyma(ctx context.Context, testClient client.Client, name, namespace stri
 	return kymaInCluster, nil
 }
 
-func IsKymaInState(ctx context.Context, kcpClient client.Client, kymaName string, state v1beta2.State) func() bool {
-	return func() bool {
-		kymaFromCluster, err := GetKyma(ctx, kcpClient, kymaName, "")
-		if err != nil || kymaFromCluster.Status.State != state {
-			return false
-		}
-		return true
+func IsKymaInState(ctx context.Context, kcpClient client.Client, kymaName string, state v1beta2.State) bool {
+	kymaFromCluster, err := GetKyma(ctx, kcpClient, kymaName, "")
+	if err != nil || kymaFromCluster.Status.State != state {
+		return false
 	}
+	return true
 }
 
 func GetManifestSpecRemote(