-
Notifications
You must be signed in to change notification settings - Fork 32
feat: Read component descriptor from oci registry #2736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
c7ddfe0
99ce9b3
ce5670b
b546aeb
34eca26
7341633
b18d7aa
08635e9
8315ca4
20dcf54
2f5e277
f7bd553
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ import ( | |
"net/http" | ||
"net/http/pprof" | ||
"os" | ||
"strings" | ||
"time" | ||
|
||
certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" | ||
|
@@ -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" | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this whole block can be There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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) | ||
|
@@ -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, | ||
|
@@ -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) | ||
|
||
|
@@ -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, | ||
|
@@ -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) | ||
|
@@ -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) | ||
|
There was a problem hiding this comment.
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.