Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions .github/actions/deploy-lifecycle-manager-e2e/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,45 @@ runs:
else
echo "E2E_KUSTOMIZE_DIR=config/watcher_local_test" >> $GITHUB_ENV
fi
- name: Patch local OCI registry host
if: ${{ matrix.e2e-test != 'oci-reg-cred-secret' && matrix.e2e-test != 'module-transferred-to-another-oci-registry' }}
working-directory: lifecycle-manager
shell: bash
run: |
pushd ${E2E_KUSTOMIZE_DIR}
echo \
"- op: add
path: /spec/template/spec/containers/0/args/-
value: --oci-registry-host=http://k3d-kcp-registry.localhost:5000" >> oci_registry_host.yaml
cat oci_registry_host.yaml
kustomize edit add patch --path oci_registry_host.yaml --kind Deployment
popd
- name: Patch remote OCI registry host
if: ${{ matrix.e2e-test == 'module-transferred-to-another-oci-registry' }}
working-directory: lifecycle-manager
shell: bash
run: |
pushd ${E2E_KUSTOMIZE_DIR}
echo \
"- op: add
path: /spec/template/spec/containers/0/args/-
value: --oci-registry-host=https://europe-west3-docker.pkg.dev/sap-kyma-jellyfish-dev/restricted-market" >> oci_registry_host.yaml
cat oci_registry_host.yaml
kustomize edit add patch --path oci_registry_host.yaml --kind Deployment
popd
- name: Patch private OCI registry secret
if: ${{ matrix.e2e-test == 'oci-reg-cred-secret' }}
working-directory: lifecycle-manager
shell: bash
run: |
pushd ${E2E_KUSTOMIZE_DIR}
echo \
"- op: add
path: /spec/template/spec/containers/0/args/-
value: --oci-registry-cred-secret=private-oci-reg-creds" >> oci_registry_host.yaml
cat oci_registry_host.yaml
kustomize edit add patch --path oci_registry_host.yaml --kind Deployment
popd
- name: Patch purge finalizer flags
if: ${{ matrix.e2e-test == 'purge-controller' || matrix.e2e-test == 'purge-metrics'}}
working-directory: lifecycle-manager
Expand Down Expand Up @@ -148,14 +187,6 @@ runs:
cat legacy-secret-rotation.yaml
kustomize edit add patch --path legacy-secret-rotation.yaml --kind Deployment
popd
- name: Use private OCI registry credentials
if: ${{matrix.e2e-test == 'oci-reg-cred-secret'}}
working-directory: lifecycle-manager
shell: bash
run: |
pushd ${E2E_KUSTOMIZE_DIR}
sed -i 's|value: --oci-registry-host=europe-docker.pkg.dev/kyma-project/kyma-modules|value: --oci-registry-cred-secret=private-oci-reg-creds|' kustomization.yaml
popd
- name: Create and use maintenance window policy
if: ${{matrix.e2e-test == 'maintenance-windows' ||
matrix.e2e-test == 'maintenance-windows-initial-installation' ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,9 @@ runs:
sed -i 's/k3d-private-oci-reg.localhost:5001/private-oci-reg.localhost:5000/g' ./template.yaml
kubectl get crds
kubectl apply -f template.yaml
- name: Create and apply ModuleReleaseMeta from the latest release
- name: Create and apply ModuleReleaseMeta from the template-operator repo
working-directory: template-operator
if: ${{ matrix.e2e-test == 'kyma-metrics' ||
matrix.e2e-test == 'non-blocking-deletion' ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick info on why this testcase is not important anymore? I don't know the details.

matrix.e2e-test == 'purge-controller' ||
matrix.e2e-test == 'purge-metrics' ||
matrix.e2e-test == 'kyma-deprovision-with-foreground-propagation' ||
Expand Down
24 changes: 24 additions & 0 deletions .github/scripts/debug/teardown.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
#!/usr/bin/env bash
kubectl config use-context k3d-kcp

k3d cluster list
echo "--- KCP ModuleTemplate ---"
kubectl get moduletemplate -n kcp-system -o wide
kubectl get moduletemplate -n kcp-system -o yaml

echo "--- KCP ModuleReleaseMeta ---"
kubectl get modulereleasemeta -n kcp-system -o wide
kubectl get modulereleasemeta -n kcp-system -o yaml

echo "--- KCP Kyma ---"
kubectl get kyma -n kcp-system -o wide
kubectl get kyma -n kcp-system -o yaml

echo "--- KCP Manifest ---"
kubectl get manifest -n kcp-system -o wide
kubectl get manifest -n kcp-system -o yaml

echo "--- KLM DEPLOYMENT ---"
kubectl get deploy klm-controller-manager -n kcp-system -o yaml
kubectl describe deploy klm-controller-manager -n kcp-system
Expand All @@ -13,7 +30,14 @@ set -e

kubectl config use-context k3d-skr


echo "--- SKR DEPLOYMENT OVERVIEW ---"
kubectl get deploy -A -o wide

echo "--- SKR-WEBHOOK POD ---"
kubectl describe deploy/skr-webhook -n kyma-system
kubectl get pods -l app=skr-webhook -n kyma-system -o wide

echo "--- SKR-WEBHOOK LOGS ---"
kubectl logs deploy/skr-webhook -n kyma-system --container server

58 changes: 44 additions & 14 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"net/http"
"net/http/pprof"
"os"
"strings"
"time"

certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
Expand Down Expand Up @@ -74,8 +75,10 @@ import (
"github.com/kyma-project/lifecycle-manager/internal/remote"
"github.com/kyma-project/lifecycle-manager/internal/repository/istiogateway"
kymarepository "github.com/kyma-project/lifecycle-manager/internal/repository/kyma"
"github.com/kyma-project/lifecycle-manager/internal/repository/oci"
secretrepository "github.com/kyma-project/lifecycle-manager/internal/repository/secret"
"github.com/kyma-project/lifecycle-manager/internal/service/accessmanager"
"github.com/kyma-project/lifecycle-manager/internal/service/componentdescriptor"
"github.com/kyma-project/lifecycle-manager/internal/service/kyma/status/modules"
"github.com/kyma-project/lifecycle-manager/internal/service/kyma/status/modules/generator"
"github.com/kyma-project/lifecycle-manager/internal/service/kyma/status/modules/generator/fromerror"
Expand Down Expand Up @@ -221,21 +224,45 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma
}

sharedMetrics := metrics.NewSharedMetrics()
descriptorProvider := provider.NewCachedDescriptorProvider()

ociRegistryHost := getOciRegistryHost(mgr.GetConfig(), flagVar, logger)
var insecure bool

if noSchemeRef, found := strings.CutPrefix(ociRegistryHost, "http://"); found {
insecure = true
ociRegistryHost = noSchemeRef
} else if noSchemeRef, found := strings.CutPrefix(ociRegistryHost, "https://"); found {
ociRegistryHost = noSchemeRef
}
Comment on lines +228 to +236
Copy link
Contributor

@ruanxin ruanxin Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this whole block can be getOciRegistryHost, and I would design this ociRegistryHost as url.URL type, you can use url.Parse(rawURL string), then you don't need to deal with cutting prefix, validating anymore. And you can use Scheme field to determine if it's insecure, so there is no need to claim insecure here and pass to NewRepository

Copy link
Member Author

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that, but this is the existing code - and it returns a string, not a url.URL...
Of course I could refactor that and tens of other places in the code. But read this, I've just added this disclaimer as the first note in the PR description:

Note to reviewers: I was really trying to keep this PR as minimal as possible. There are many things to fix/improve/cleanup and many times I've seen that, but I decided not to, in order to introduce as few changes as possible.
Keep that in mind when suggesting improvements. Do we really want to make this PR even bigger? I would rather prefer to make a note in the sources and then create a follow-up PR to clean all the notes. Just remember that the work remaining in https://github.com/kyma-project/lifecycle-manager/issues/2526 and our other efforts (like simplifying/getting rid of templatelookup.go) will probably make most of these review-fixes obsolete or unnecessary.
Up to you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s totally fine — the review comments are meant to highlight potential issues, not necessarily to be fixed right away. If you’d prefer not to address them in this PR, feel free to create a follow-up issue instead.

At the same time, not mentioning these code smells would be my oversight. Continuously improving our existing codebase should always be our goal.


ocmDescriptorRepository, err := oci.NewRepository(
keychainLookupFromFlag(mgr.GetClient(), flagVar),
ociRegistryHost,
insecure,
)
if err != nil {
logger.Error(err, "failed to create OCM descriptor repository")
os.Exit(bootstrapFailedExitCode)
}

ocmDescriptorService, err := componentdescriptor.NewService(ocmDescriptorRepository)
if err != nil {
logger.Error(err, "failed to create OCM descriptor service")
os.Exit(bootstrapFailedExitCode)
}
descriptorProvider := provider.NewCachedDescriptorProvider(ocmDescriptorService)

kymaMetrics := metrics.NewKymaMetrics(sharedMetrics)
mandatoryModulesMetrics := metrics.NewMandatoryModulesMetrics()
maintenanceWindow := initMaintenanceWindow(flagVar.MinMaintenanceWindowSize, logger)
metrics.NewFipsMetrics().Update()

//nolint:godox // this will be used in the future
// TODO: use the oci registry host //nolint:godox // this will be used in the future
_ = getOciRegistryHost(mgr.GetConfig(), flagVar, logger)

setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager,
kymaMetrics, logger, maintenanceWindow)
setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, accessManagerService, logger,
eventRecorder)
setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, logger)
setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder,
flagVar, options, skrWebhookManager, kymaMetrics, logger, maintenanceWindow, ociRegistryHost)
setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics,
accessManagerService, logger, eventRecorder)
setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options,
mandatoryModulesMetrics, logger, ociRegistryHost)
setupMandatoryModuleDeletionReconciler(mgr, descriptorProvider, eventRecorder, flagVar, options, logger)
if flagVar.EnablePurgeFinalizer {
setupPurgeReconciler(mgr, skrContextProvider, eventRecorder, flagVar, options, logger)
Expand Down Expand Up @@ -379,7 +406,7 @@ func scheduleMetricsCleanup(kymaMetrics *metrics.KymaMetrics, cleanupIntervalInM
func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDescriptorProvider,
skrContextFactory remote.SkrContextProvider, event event.Event, flagVar *flags.FlagVar, options ctrlruntime.Options,
skrWebhookManager *watcher.SkrWebhookManifestManager, kymaMetrics *metrics.KymaMetrics,
setupLog logr.Logger, maintenanceWindow maintenancewindows.MaintenanceWindow,
setupLog logr.Logger, maintenanceWindow maintenancewindows.MaintenanceWindow, ociRegistryHost string,
) {
options.RateLimiter = internal.RateLimiter(flagVar.FailureBaseDelay,
flagVar.FailureMaxDelay, flagVar.RateLimiterFrequency, flagVar.RateLimiterBurst)
Expand Down Expand Up @@ -420,6 +447,7 @@ func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDe
flagVar.RemoteSyncNamespace),
TemplateLookup: templatelookup.NewTemplateLookup(kcpClient, descriptorProvider,
moduleTemplateInfoLookupStrategies),
OCIRegistryHost: ociRegistryHost,
}).SetupWithManager(
mgr, options, kyma.SetupOptions{
ListenerAddr: flagVar.KymaListenerAddr,
Expand Down Expand Up @@ -476,7 +504,7 @@ func setupManifestReconciler(mgr ctrl.Manager,
manifestClient := manifestclient.NewManifestClient(event, mgr.GetClient())
orphanDetectionClient := kymarepository.NewClient(mgr.GetClient())
orphanDetectionService := orphan.NewDetectionService(orphanDetectionClient)
specResolver := spec.NewResolver(keychainLookupFromFlag(mgr, flagVar), img.NewPathExtractor())
specResolver := spec.NewResolver(keychainLookupFromFlag(mgr.GetClient(), flagVar), img.NewPathExtractor())
clientCache := skrclientcache.NewService()
skrClient := skrclient.NewService(mgr.GetConfig().QPS, mgr.GetConfig().Burst, accessManagerService)

Expand Down Expand Up @@ -504,9 +532,9 @@ func setupManifestReconciler(mgr ctrl.Manager,
}

//nolint:ireturn // constructor functions can return interfaces
func keychainLookupFromFlag(mgr ctrl.Manager, flagVar *flags.FlagVar) spec.KeyChainLookup {
func keychainLookupFromFlag(clnt client.Client, flagVar *flags.FlagVar) spec.KeyChainLookup {
if flagVar.OciRegistryCredSecretName != "" {
return keychainprovider.NewFromSecretKeyChainProvider(mgr.GetClient(),
return keychainprovider.NewFromSecretKeyChainProvider(clnt,
types.NamespacedName{
Namespace: shared.DefaultControlPlaneNamespace,
Name: flagVar.OciRegistryCredSecretName,
Expand Down Expand Up @@ -547,6 +575,7 @@ func setupMandatoryModuleReconciler(mgr ctrl.Manager,
options ctrlruntime.Options,
metrics *metrics.MandatoryModulesMetrics,
setupLog logr.Logger,
ociRegistryHost string,
) {
options.RateLimiter = internal.RateLimiter(flagVar.FailureBaseDelay,
flagVar.FailureMaxDelay, flagVar.RateLimiterFrequency, flagVar.RateLimiterBurst)
Expand All @@ -564,6 +593,7 @@ func setupMandatoryModuleReconciler(mgr ctrl.Manager,
RemoteSyncNamespace: flagVar.RemoteSyncNamespace,
DescriptorProvider: descriptorProvider,
Metrics: metrics,
OCIRegistryHost: ociRegistryHost,
}).SetupWithManager(mgr, options); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "MandatoryModule")
os.Exit(bootstrapFailedExitCode)
Expand Down
3 changes: 0 additions & 3 deletions config/watcher_local_test/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ patches:
- op: add
path: /spec/template/spec/containers/0/args/-
value: --leader-election-retry-period=3s
- op: add
path: /spec/template/spec/containers/0/args/-
value: --oci-registry-host=europe-docker.pkg.dev/kyma-project/kyma-modules
- op: replace
path: /spec/template/spec/containers/0/imagePullPolicy
value: Always
Expand Down
3 changes: 0 additions & 3 deletions config/watcher_local_test_gcm/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@ patches:
- op: add
path: /spec/template/spec/containers/0/args/-
value: --leader-election-retry-period=3s
- op: add
path: /spec/template/spec/containers/0/args/-
value: --oci-registry-host=europe-docker.pkg.dev/kyma-project/kyma-modules
- op: replace
path: /spec/template/spec/containers/0/imagePullPolicy
value: Always
Expand Down
3 changes: 2 additions & 1 deletion internal/controller/kyma/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ type Reconciler struct {
Metrics *metrics.KymaMetrics
RemoteCatalog *remote.RemoteCatalog
TemplateLookup *templatelookup.TemplateLookup
OCIRegistryHost string
}

func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
Expand Down Expand Up @@ -584,7 +585,7 @@ func (r *Reconciler) updateKyma(ctx context.Context, kyma *v1beta2.Kyma) error {

func (r *Reconciler) reconcileManifests(ctx context.Context, kyma *v1beta2.Kyma) error {
templates := r.TemplateLookup.GetRegularTemplates(ctx, kyma)
prsr := parser.NewParser(r.Client, r.DescriptorProvider, r.RemoteSyncNamespace)
prsr := parser.NewParser(r.Client, r.DescriptorProvider, r.RemoteSyncNamespace, r.OCIRegistryHost)
modules := prsr.GenerateModulesFromTemplates(kyma, templates)

runner := sync.New(r)
Expand Down
39 changes: 28 additions & 11 deletions internal/controller/mandatorymodule/deletion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ import (
"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/internal/descriptor/provider"
"github.com/kyma-project/lifecycle-manager/internal/descriptor/types/ocmidentity"
"github.com/kyma-project/lifecycle-manager/internal/event"
"github.com/kyma-project/lifecycle-manager/pkg/log"
"github.com/kyma-project/lifecycle-manager/pkg/queue"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
"github.com/kyma-project/lifecycle-manager/pkg/util"
)

Expand Down Expand Up @@ -76,7 +78,18 @@ func (r *DeletionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
return ctrl.Result{}, nil
}

manifests, err := r.getCorrespondingManifests(ctx, template)
mrm, err := r.GetModuleReleaseMeta(ctx, template.Spec.ModuleName, template.Namespace)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to find ModuleReleaseMeta for Mandatory Module %s: %w",
template.Name, err)
}
ocmi, err := ocmidentity.New(mrm.Spec.OcmComponentName, template.Spec.Version)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to create OCM identity for Mandatory Module %s: %w",
template.Spec.ModuleName, err)
}

manifests, err := r.getCorrespondingManifests(ctx, template.Namespace, *ocmi)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get MandatoryModuleManifests: %w", err)
}
Expand All @@ -96,6 +109,12 @@ func (r *DeletionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
return ctrl.Result{Requeue: true}, nil
}

func (r *DeletionReconciler) GetModuleReleaseMeta(ctx context.Context, moduleName, namespace string) (
*v1beta2.ModuleReleaseMeta, error,
) {
return templatelookup.GetModuleReleaseMeta(ctx, r.Client, moduleName, namespace)
}

func (r *DeletionReconciler) updateTemplateFinalizer(ctx context.Context,
template *v1beta2.ModuleTemplate,
) (ctrl.Result, error) {
Expand All @@ -107,22 +126,18 @@ func (r *DeletionReconciler) updateTemplateFinalizer(ctx context.Context,
}

func (r *DeletionReconciler) getCorrespondingManifests(ctx context.Context,
template *v1beta2.ModuleTemplate) ([]v1beta2.Manifest,
namespace string, ocmi ocmidentity.Component) ([]v1beta2.Manifest,
error,
) {
manifests := &v1beta2.ManifestList{}
descriptor, err := r.DescriptorProvider.GetDescriptor(template)
if err != nil {
return nil, fmt.Errorf("not able to get descriptor from template: %w", err)
}
if err := r.List(ctx, manifests, &client.ListOptions{
Namespace: template.Namespace,
Namespace: namespace,
LabelSelector: k8slabels.SelectorFromSet(k8slabels.Set{shared.IsMandatoryModule: "true"}),
}); client.IgnoreNotFound(err) != nil {
return nil, fmt.Errorf("not able to list mandatory module manifests: %w", err)
}

filtered := filterManifestsByFQDNAndVersion(manifests.Items, descriptor.GetName(), descriptor.GetVersion())
filtered := filterManifestsByComponentIdentity(manifests.Items, ocmi)

return filtered, nil
}
Expand All @@ -137,16 +152,18 @@ func (r *DeletionReconciler) removeManifests(ctx context.Context, manifests []v1
return nil
}

func filterManifestsByFQDNAndVersion(manifests []v1beta2.Manifest,
fqdn, moduleVersion string,
// filterManifestsByComponentIdentity filters the manifests by OCM Component Name and module version.
// OCM Component Name is a fully qualified name that looks like: 'kyma-project.io/module/<module-name>'.
func filterManifestsByComponentIdentity(manifests []v1beta2.Manifest,
ocmi ocmidentity.Component,
) []v1beta2.Manifest {
filteredManifests := make([]v1beta2.Manifest, 0)
for _, manifest := range manifests {
if manifest.Annotations == nil {
continue
}

if manifest.Annotations[shared.FQDN] == fqdn && manifest.Spec.Version == moduleVersion {
if manifest.Annotations[shared.FQDN] == ocmi.Name() && manifest.Spec.Version == ocmi.Version() {
filteredManifests = append(filteredManifests, manifest)
}
}
Expand Down
Loading
Loading