Skip to content
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.

Sign up for GitHub

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

Support dockerconfigjson with OCI HelmRepositories #725

Merged
merged 10 commits into from
May 24, 2022
Merged

Support dockerconfigjson with OCI HelmRepositories #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1070d12
fix nil pointer dereference
May 20, 2022
bb4d886
dockerconfigjson for OCI registry authentication
May 20, 2022
ce072c7
better variable names; improved logging
May 22, 2022
d5e3c37
fix code formatting
May 23, 2022
ace21c5
make tidy
May 23, 2022
c795da2
introduce `internal/helm/registry` package
May 23, 2022
a3be7e5
document generateBuildResult
May 23, 2022
09a2458
fix import order
May 23, 2022
7cfd94e
fix func doc
May 23, 2022
bb569be
include Secret name in returned errors
May 24, 2022
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
20 changes: 10 additions & 10 deletions controllers/helmchart_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -60,6 +60,7 @@ import (
serror "github.com/fluxcd/source-controller/internal/error"
"github.com/fluxcd/source-controller/internal/helm/chart"
"github.com/fluxcd/source-controller/internal/helm/getter"
"github.com/fluxcd/source-controller/internal/helm/registry"
"github.com/fluxcd/source-controller/internal/helm/repository"
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
logOpts []registry.LoginOption
loginOpts []helmreg.LoginOption
)

// Construct the Getter options from the HelmRepository data
Expand Down Expand Up @@ -492,7 +493,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
}

// Build registryClient options from secret
logOpt, err := loginOptionFromSecret(*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),
Expand All @@ -503,14 +504,14 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
return sreconcile.ResultEmpty, e
}

logOpts = append([]registry.LoginOption{}, logOpt)
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)
}
Expand All @@ -519,7 +520,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
// TODO@souleb: remove this once the registry move to Oras v2
// or rework to enable reusing credentials to avoid the unneccessary handshake operations
registryClient, file, err := r.RegistryClientGenerator(logOpts != nil)
registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil)
if err != nil {
return chartRepoErrorReturn(err, obj)
}
Expand All @@ -540,14 +541,13 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *

// If login options are configured, use them to login to the registry
// The OCIGetter will later retrieve the stored credentials to pull the chart
if logOpts != nil {
err = ociChartRepo.Login(logOpts...)
if loginOpts != nil {
err = ociChartRepo.Login(loginOpts...)
if err != nil {
return chartRepoErrorReturn(err, obj)
}
}
default:
var httpChartRepo *repository.ChartRepository
httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
r.IncCacheEvents(event, obj.Name, obj.Namespace)
Expand Down
40 changes: 35 additions & 5 deletions controllers/helmchart_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controllers
import (
"bytes"
"context"
"encoding/base64"
"errors"
"fmt"
"io"
Expand All @@ -35,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"
Expand All @@ -53,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"
)
Expand Down Expand Up @@ -792,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
Expand Down Expand Up @@ -825,6 +826,35 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build)
cleanFunc func(g *WithT, build *chart.Build)
}{
{
name: "Reconciles chart build with docker repository credentials",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{
".dockerconfigjson": []byte(`{"auths":{"` +
testRegistryserver.DockerRegistryHost + `":{"` +
`auth":"` + base64.StdEncoding.EncodeToString([]byte(testUsername+":"+testPassword)) + `"}}}`),
},
},
beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) {
obj.Spec.Chart = metadata.Name
obj.Spec.Version = metadata.Version
repository.Spec.SecretRef = &meta.LocalObjectReference{Name: "auth"}
},
want: sreconcile.ResultSuccess,
assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) {
g.Expect(build.Name).To(Equal(metadata.Name))
g.Expect(build.Version).To(Equal(metadata.Version))
g.Expect(build.Path).ToNot(BeEmpty())
g.Expect(build.Path).To(BeARegularFile())
},
cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed())
},
},
{
name: "Reconciles chart build with repository credentials",
secret: &corev1.Secret{
Expand Down Expand Up @@ -945,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{
Expand Down
28 changes: 10 additions & 18 deletions controllers/helmrepository_controller_oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,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"
Expand Down Expand Up @@ -91,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
Expand Down Expand Up @@ -254,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 logOpts []registry.LoginOption
var loginOpts []helmreg.LoginOption
// Configure any authentication related options
if obj.Spec.SecretRef != nil {
// Attempt to retrieve secret
Expand All @@ -273,7 +274,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *
}

// Construct actual options
logOpt, err := loginOptionFromSecret(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),
Expand All @@ -284,10 +285,12 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *
return sreconcile.ResultEmpty, e
}

logOpts = append(logOpts, logOpt)
if loginOpt != nil {
loginOpts = append(loginOpts, loginOpt)
}
}

if result, err := r.validateSource(ctx, obj, logOpts...); err != nil || result == sreconcile.ResultEmpty {
if result, err := r.validateSource(ctx, obj, loginOpts...); err != nil || result == sreconcile.ResultEmpty {
return result, err
}

Expand All @@ -296,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{
Expand Down Expand Up @@ -349,14 +352,3 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s

return sreconcile.ResultSuccess, nil
}

func loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) {
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
}
Loading