Skip to content

Commit

Permalink
secretRef take precedence over provider
Browse files Browse the repository at this point in the history 
if secretRef is provided, we do not attempt to resolve oidc

Signed-off-by: Soule BA <soule@weave.works>
  • Loading branch information
@souleb
souleb committed Sep 7, 2022
1 parent f97bbb6 commit eaab1a0
Show file tree
Hide file tree
Showing 8 changed files with 476 additions and 70 deletions.
12 changes: 4 additions & 8 deletions controllers/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
}

loginOpts = append([]helmreg.LoginOption{}, loginOpt)
}

if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, repo)
} else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuthFromAdapter(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
e := &serror.Event{
Err: fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr),
Expand Down Expand Up @@ -991,10 +989,8 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
}

loginOpts = append([]helmreg.LoginOption{}, loginOpt)
}

if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, repo)
} else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuthFromAdapter(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
return nil, fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr)
}
Expand Down
234 changes: 223 additions & 11 deletions controllers/helmchart_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/fluxcd/pkg/apis/meta"
Expand Down Expand Up @@ -893,21 +894,11 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
chartPath = "testdata/charts/helmchart-0.1.0.tgz"
)

// Login to the registry
err := testRegistryServer.registryClient.Login(testRegistryServer.registryHost,
helmreg.LoginOptBasicAuth(testRegistryUsername, testRegistryPassword),
helmreg.LoginOptInsecure(true))
g.Expect(err).NotTo(HaveOccurred())

// Load a test chart
chartData, err := ioutil.ReadFile(chartPath)
g.Expect(err).NotTo(HaveOccurred())
metadata, err := extractChartMeta(chartData)
g.Expect(err).NotTo(HaveOccurred())

// Upload the test chart
ref := fmt.Sprintf("%s/testrepo/%s:%s", testRegistryServer.registryHost, metadata.Name, metadata.Version)
_, err = testRegistryServer.registryClient.Push(chartData, ref)
metadata, err := loadTestChartToOCI(chartData, chartPath, testRegistryServer)
g.Expect(err).NotTo(HaveOccurred())

storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
Expand Down Expand Up @@ -2038,6 +2029,198 @@ func TestHelmChartReconciler_notify(t *testing.T) {
}
}

func TestHelmChartReconciler_reconcileSource_authStrategy(t *testing.T) {
const (
chartPath = "testdata/charts/helmchart-0.1.0.tgz"
)

type secretOptions struct {
username string
password string
}

tests := []struct {
name string
url string
registryOpts registryOptions
secretOpts secretOptions
provider string
providerImg string
want sreconcile.Result
wantErr bool
assertConditions []metav1.Condition
}{
{
name: "HTTP without basic auth",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<helmchart>' chart with version '<version>'"),
},
},
{
name: "HTTP with basic auth secret",
want: sreconcile.ResultSuccess,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<helmchart>' chart with version '<version>'"),
},
},
{
name: "HTTP registry - basic auth with invalid secret",
want: sreconcile.ResultEmpty,
wantErr: true,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: "wrong-pass",
password: "wrong-pass",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"),
},
},
{
name: "with contextual login provider",
wantErr: true,
provider: "aws",
providerImg: "oci://123456789000.dkr.ecr.us-east-2.amazonaws.com/test",
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to get credential from"),
},
},
{
name: "with contextual login provider and secretRef",
want: sreconcile.ResultSuccess,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
provider: "azure",
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<helmchart>' chart with version '<version>'"),
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

builder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme())
workspaceDir := t.TempDir()
server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts)

g.Expect(err).NotTo(HaveOccurred())

// Load a test chart
chartData, err := ioutil.ReadFile(chartPath)

// Upload the test chart
metadata, err := loadTestChartToOCI(chartData, chartPath, server)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(err).ToNot(HaveOccurred())

repo := &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
},
Spec: sourcev1.HelmRepositorySpec{
Interval: metav1.Duration{Duration: interval},
Timeout: &metav1.Duration{Duration: timeout},
Type: sourcev1.HelmRepositoryTypeOCI,
Provider: sourcev1.GenericOCIProvider,
URL: fmt.Sprintf("oci://%s/testrepo", server.registryHost),
},
}

if tt.provider != "" {
repo.Spec.Provider = tt.provider
}
// If a provider specific image is provided, overwrite existing URL
// set earlier. It'll fail but it's necessary to set them because
// the login check expects the URLs to be of certain pattern.
if tt.providerImg != "" {
repo.Spec.URL = tt.providerImg
}

if tt.secretOpts.username != "" && tt.secretOpts.password != "" {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{
".dockerconfigjson": []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`,
server.registryHost, tt.secretOpts.username, tt.secretOpts.password)),
},
}

repo.Spec.SecretRef = &meta.LocalObjectReference{
Name: secret.Name,
}
builder.WithObjects(secret, repo)
} else {
builder.WithObjects(repo)
}

obj := &sourcev1.HelmChart{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
},
Spec: sourcev1.HelmChartSpec{
Chart: metadata.Name,
Version: metadata.Version,
SourceRef: sourcev1.LocalHelmChartSourceReference{
Kind: sourcev1.HelmRepositoryKind,
Name: repo.Name,
},
Interval: metav1.Duration{Duration: interval},
},
}

r := &HelmChartReconciler{
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Getters: testGetters,
RegistryClientGenerator: registry.ClientGenerator,
}

var b chart.Build
defer func() {
if _, err := os.Stat(b.Path); !os.IsNotExist(err) {
err := os.Remove(b.Path)
g.Expect(err).NotTo(HaveOccurred())
}
}()

assertConditions := tt.assertConditions
for k := range assertConditions {
assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<helmchart>", metadata.Name)
assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<version>", metadata.Version)
}

got, err := r.reconcileSource(ctx, obj, &b)
if tt.wantErr {
g.Expect(err).ToNot(BeNil())
} else {
g.Expect(err).To(BeNil())
}
g.Expect(got).To(Equal(tt.want))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
})
}
}

// extractChartMeta is used to extract a chart metadata from a byte array
func extractChartMeta(chartData []byte) (*hchart.Metadata, error) {
ch, err := loader.LoadArchive(bytes.NewReader(chartData))
Expand All @@ -2046,3 +2229,32 @@ func extractChartMeta(chartData []byte) (*hchart.Metadata, error) {
}
return ch.Metadata, nil
}

func loadTestChartToOCI(chartData []byte, chartPath string, server *registryClientTestServer) (*hchart.Metadata, error) {
// Login to the registry
err := server.registryClient.Login(server.registryHost,
helmreg.LoginOptBasicAuth(testRegistryUsername, testRegistryPassword),
helmreg.LoginOptInsecure(true))
if err != nil {
return nil, err
}

// Load a test chart
chartData, err = ioutil.ReadFile(chartPath)
if err != nil {
return nil, err
}
metadata, err := extractChartMeta(chartData)
if err != nil {
return nil, err
}

// Upload the test chart
ref := fmt.Sprintf("%s/testrepo/%s:%s", server.registryHost, metadata.Name, metadata.Version)
_, err = server.registryClient.Push(chartData, ref)
if err != nil {
return nil, err
}

return metadata, nil
}
44 changes: 5 additions & 39 deletions controllers/helmrepository_controller_oci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"net/url"
"os"
"strings"
"time"

helmgetter "helm.sh/helm/v3/pkg/getter"
Expand All @@ -42,12 +41,10 @@ import (

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/oci"
"github.com/fluxcd/pkg/oci/auth/login"
"github.com/fluxcd/pkg/runtime/conditions"
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
"github.com/google/go-containerregistry/pkg/name"

"github.com/fluxcd/source-controller/api/v1beta2"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
Expand Down Expand Up @@ -294,10 +291,8 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
if loginOpt != nil {
loginOpts = append(loginOpts, loginOpt)
}
}

if obj.Spec.Provider != sourcev1.GenericOCIProvider && obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, obj)
} else if obj.Spec.Provider != sourcev1.GenericOCIProvider && obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuthFromAdapter(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
Expand Down Expand Up @@ -380,41 +375,12 @@ func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime
r.Eventf(obj, eventType, reason, msg)
}

// oidcAuth generates the OIDC credential authenticator based on the specified cloud provider.
func oidcAuth(ctx context.Context, obj *sourcev1.HelmRepository) (helmreg.LoginOption, error) {
url := strings.TrimPrefix(obj.Spec.URL, sourcev1.OCIRepositoryPrefix)
ref, err := name.ParseReference(url)
if err != nil {
return nil, fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err)
}

loginOpt, err := loginWithManager(ctx, obj.Spec.Provider, url, ref)
if err != nil {
return nil, fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err)
}

return loginOpt, nil
}

func loginWithManager(ctx context.Context, provider, url string, ref name.Reference) (helmreg.LoginOption, error) {
opts := login.ProviderOptions{}
switch provider {
case sourcev1.AmazonOCIProvider:
opts.AwsAutoLogin = true
case sourcev1.AzureOCIProvider:
opts.AzureAutoLogin = true
case sourcev1.GoogleOCIProvider:
opts.GcpAutoLogin = true
}

auth, err := login.NewManager().Login(ctx, url, ref, opts)
// oidcAuthFromAdapter generates the OIDC credential authenticator based on the specified cloud provider.
func oidcAuthFromAdapter(ctx context.Context, url, provider string) (helmreg.LoginOption, error) {
auth, err := oidcAuth(ctx, url, provider)
if err != nil {
return nil, err
}

if auth == nil {
return nil, nil
}

return registry.OIDCAdaptHelper(auth)
}
Loading

0 comments on commit eaab1a0

Please sign in to comment.