From c795da2280468f8a4324c58e624bfeab177e9006 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 11:11:53 +0200 Subject: [PATCH] introduce `internal/helm/registry` package This new package holds all Helm OCI registry-specific code now so we have a single location to look for such code which makes it easier to find yourself around. Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 15 ++++--- controllers/helmchart_controller_test.go | 10 ++--- controllers/helmrepository_controller_oci.go | 47 +++----------------- controllers/suite_test.go | 18 ++++---- internal/helm/registry/auth.go | 44 ++++++++++++++++++ internal/helm/{util => registry}/client.go | 4 +- main.go | 6 +-- 7 files changed, 77 insertions(+), 67 deletions(-) create mode 100644 internal/helm/registry/auth.go rename internal/helm/{util => registry}/client.go (94%) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 80bb773f..07efa41e 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -29,7 +29,7 @@ import ( "time" helmgetter "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -64,6 +64,7 @@ import ( sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" "github.com/fluxcd/source-controller/internal/util" + "github.com/fluxcd/source-controller/internal/helm/registry" ) // helmChartReadyCondition contains all the conditions information @@ -380,7 +381,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 // Assert source has an artifact if s.GetArtifact() == nil || !r.Storage.ArtifactExist(*s.GetArtifact()) { - if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || !registry.IsOCI(helmRepo.Spec.URL) { + if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || !helmreg.IsOCI(helmRepo.Spec.URL) { conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "NoSourceArtifact", "no artifact available for %s source '%s'", obj.Spec.SourceRef.Kind, obj.Spec.SourceRef.Name) r.eventLogf(ctx, obj, events.EventTypeTrace, "NoSourceArtifact", @@ -447,7 +448,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { var ( tlsConfig *tls.Config - loginOpts []registry.LoginOption + loginOpts []helmreg.LoginOption ) // Construct the Getter options from the HelmRepository data @@ -492,7 +493,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } // Build registryClient options from secret - loginOpt, err := loginOptionFromSecret(repo.Spec.URL, *secret) + loginOpt, err := registry.LoginOptionFromSecret(repo.Spec.URL, *secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -503,14 +504,14 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, e } - loginOpts = append([]registry.LoginOption{}, loginOpt) + loginOpts = append([]helmreg.LoginOption{}, loginOpt) } // Initialize the chart repository var chartRepo chart.Remote switch repo.Spec.Type { case sourcev1.HelmRepositoryTypeOCI: - if !registry.IsOCI(repo.Spec.URL) { + if !helmreg.IsOCI(repo.Spec.URL) { err := fmt.Errorf("invalid OCI registry URL: %s", repo.Spec.URL) return chartRepoErrorReturn(err, obj) } @@ -551,7 +552,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { r.IncCacheEvents(event, obj.Name, obj.Namespace) })) - if err != nil { + if err != nil { return chartRepoErrorReturn(err, obj) } chartRepo = httpChartRepo diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 9796ea6e..59ff1d0b 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -36,7 +36,7 @@ import ( . "github.com/onsi/gomega" hchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,7 +54,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/chart" - "github.com/fluxcd/source-controller/internal/helm/util" + "github.com/fluxcd/source-controller/internal/helm/registry" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) @@ -793,8 +793,8 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { // Login to the registry err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, - registry.LoginOptBasicAuth(testUsername, testPassword), - registry.LoginOptInsecure(true)) + helmreg.LoginOptBasicAuth(testUsername, testPassword), + helmreg.LoginOptInsecure(true)) g.Expect(err).NotTo(HaveOccurred()) // Load a test chart @@ -975,7 +975,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { EventRecorder: record.NewFakeRecorder(32), Getters: testGetters, Storage: storage, - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, } repository := &sourcev1.HelmRepository{ diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 7702e446..ba2d356d 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -17,15 +17,12 @@ limitations under the License. package controllers import ( - "bytes" "context" "fmt" - "net/url" "os" "strings" "time" - "github.com/docker/cli/cli/config" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" helper "github.com/fluxcd/pkg/runtime/controller" @@ -33,12 +30,13 @@ import ( "github.com/fluxcd/pkg/runtime/predicates" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" + "github.com/fluxcd/source-controller/internal/helm/registry" "github.com/fluxcd/source-controller/internal/helm/repository" intpredicates "github.com/fluxcd/source-controller/internal/predicates" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" helmgetter "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" @@ -94,7 +92,7 @@ type HelmRepositoryOCIReconciler struct { // and an optional file name. // The file is used to store the registry client credentials. // The caller is responsible for deleting the file. -type RegistryClientGeneratorFunc func(isLogin bool) (*registry.Client, string, error) +type RegistryClientGeneratorFunc func(isLogin bool) (*helmreg.Client, string, error) // helmRepositoryOCIReconcileFunc is the function type for all the // v1beta2.HelmRepository (sub)reconcile functions for OCI type. The type implementations @@ -257,7 +255,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *source } func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { - var loginOpts []registry.LoginOption + var loginOpts []helmreg.LoginOption // Configure any authentication related options if obj.Spec.SecretRef != nil { // Attempt to retrieve secret @@ -276,7 +274,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * } // Construct actual options - loginOpt, err := loginOptionFromSecret(obj.Spec.URL, secret) + loginOpt, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -301,7 +299,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * // validateSource the HelmRepository object by checking the url and connecting to the underlying registry // with he provided credentials. -func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, logOpts ...registry.LoginOption) (sreconcile.Result, error) { +func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, logOpts ...helmreg.LoginOption) (sreconcile.Result, error) { registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) if err != nil { e := &serror.Stalling{ @@ -354,36 +352,3 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s return sreconcile.ResultSuccess, nil } - -// loginOptionFromSecret derives authentication data from a Secret to login to an OCI registry. This Secret -// may either hold "username" and "password" fields or be of the corev1.SecretTypeDockerConfigJson type and hold -// a corev1.DockerConfigJsonKey field with a complete Docker configuration. If both, "username" and "password" are -// empty, a nil LoginOption and a nil error will be returned. -func loginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.LoginOption, error) { - var username, password string - if secret.Type == corev1.SecretTypeDockerConfigJson { - dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) - if err != nil { - return nil, fmt.Errorf("unable to load Docker config: %w", err) - } - parsedURL, err := url.Parse(registryURL) - if err != nil { - return nil, fmt.Errorf("unable to parse registry URL: %w", err) - } - authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) - if err != nil { - return nil, fmt.Errorf("unable to get authentication data from Secret: %w", err) - } - username = authConfig.Username - password = authConfig.Password - } else { - username, password = string(secret.Data["username"]), string(secret.Data["password"]) - } - switch { - case username == "" && password == "": - return nil, nil - case username == "" || password == "": - return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name) - } - return registry.LoginOptBasicAuth(username, password), nil -} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 7cef15e3..6531d633 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -30,7 +30,7 @@ import ( "golang.org/x/crypto/bcrypt" "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" @@ -49,7 +49,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" "github.com/fluxcd/source-controller/internal/features" - "github.com/fluxcd/source-controller/internal/helm/util" + "github.com/fluxcd/source-controller/internal/helm/registry" // +kubebuilder:scaffold:imports ) @@ -94,7 +94,7 @@ var ( ) var ( - testRegistryClient *registry.Client + testRegistryClient *helmreg.Client testRegistryserver *RegistryClientTestServer ) @@ -113,7 +113,7 @@ type RegistryClientTestServer struct { Out io.Writer DockerRegistryHost string WorkspaceDir string - RegistryClient *registry.Client + RegistryClient *helmreg.Client } func SetupServer(server *RegistryClientTestServer) string { @@ -129,9 +129,9 @@ func SetupServer(server *RegistryClientTestServer) string { server.Out = &out // init test client - server.RegistryClient, err = registry.NewClient( - registry.ClientOptDebug(true), - registry.ClientOptWriter(server.Out), + server.RegistryClient, err = helmreg.NewClient( + helmreg.ClientOptDebug(true), + helmreg.ClientOptWriter(server.Out), ) if err != nil { panic(fmt.Sprintf("failed to create registry client: %s", err)) @@ -202,7 +202,7 @@ func TestMain(m *testing.M) { testRegistryserver = &RegistryClientTestServer{} registryWorkspaceDir := SetupServer(testRegistryserver) - testRegistryClient, err = registry.NewClient(registry.ClientOptWriter(os.Stdout)) + testRegistryClient, err = helmreg.NewClient(helmreg.ClientOptWriter(os.Stdout)) if err != nil { panic(fmt.Sprintf("Failed to create OCI registry client")) } @@ -241,7 +241,7 @@ func TestMain(m *testing.M) { EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, Getters: testGetters, - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, }).SetupWithManager(testEnv); err != nil { panic(fmt.Sprintf("Failed to start HelmRepositoryOCIReconciler: %v", err)) } diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go new file mode 100644 index 00000000..64922cdd --- /dev/null +++ b/internal/helm/registry/auth.go @@ -0,0 +1,44 @@ +package registry + +import ( + "bytes" + "fmt" + "net/url" + + "github.com/docker/cli/cli/config" + "helm.sh/helm/v3/pkg/registry" + corev1 "k8s.io/api/core/v1" +) + +// LoginOptionFromSecret derives authentication data from a Secret to login to an OCI registry. This Secret +// may either hold "username" and "password" fields or be of the corev1.SecretTypeDockerConfigJson type and hold +// a corev1.DockerConfigJsonKey field with a complete Docker configuration. If both, "username" and "password" are +// empty, a nil LoginOption and a nil error will be returned. +func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.LoginOption, error) { + var username, password string + if secret.Type == corev1.SecretTypeDockerConfigJson { + dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) + if err != nil { + return nil, fmt.Errorf("unable to load Docker config: %w", err) + } + parsedURL, err := url.Parse(registryURL) + if err != nil { + return nil, fmt.Errorf("unable to parse registry URL: %w", err) + } + authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) + if err != nil { + return nil, fmt.Errorf("unable to get authentication data from Secret: %w", err) + } + username = authConfig.Username + password = authConfig.Password + } else { + username, password = string(secret.Data["username"]), string(secret.Data["password"]) + } + switch { + case username == "" && password == "": + return nil, nil + case username == "" || password == "": + return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name) + } + return registry.LoginOptBasicAuth(username, password), nil +} diff --git a/internal/helm/util/client.go b/internal/helm/registry/client.go similarity index 94% rename from internal/helm/util/client.go rename to internal/helm/registry/client.go index 1bd8944f..0e835e8f 100644 --- a/internal/helm/util/client.go +++ b/internal/helm/registry/client.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package registry import ( "io" @@ -26,7 +26,7 @@ import ( // RegistryClientGenerator generates a registry client and a temporary credential file. // The client is meant to be used for a single reconciliation. // The file is meant to be used for a single reconciliation and deleted after. -func RegistryClientGenerator(isLogin bool) (*registry.Client, string, error) { +func ClientGenerator(isLogin bool) (*registry.Client, string, error) { if isLogin { // create a temporary file to store the credentials // this is needed because otherwise the credentials are stored in ~/.docker/config.json. diff --git a/main.go b/main.go index 5088d599..a4b878a2 100644 --- a/main.go +++ b/main.go @@ -42,7 +42,7 @@ import ( "github.com/fluxcd/pkg/runtime/pprof" "github.com/fluxcd/pkg/runtime/probes" "github.com/fluxcd/source-controller/internal/features" - "github.com/fluxcd/source-controller/internal/helm/util" + "github.com/fluxcd/source-controller/internal/helm/registry" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/controllers" @@ -239,7 +239,7 @@ func main() { Metrics: metricsH, Getters: getters, ControllerName: controllerName, - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, }).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), @@ -270,7 +270,7 @@ func main() { if err = (&controllers.HelmChartReconciler{ Client: mgr.GetClient(), - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, Storage: storage, Getters: getters, EventRecorder: eventRecorder,