Skip to content

Commit

Permalink
Add support for custom certificate and skip-tls-verify in helm OCI
Browse files Browse the repository at this point in the history 
If implemented user will be able to provide their own custom start and
bypass tls verification when interacting with OCI registries over https
to pull helmCharts.

Signed-off-by: Soule BA <soule@weave.works>
  • Loading branch information
@souleb @stefanprodan
souleb authored and stefanprodan committed Aug 7, 2023
1 parent 6377c6f commit d45c08c
Show file tree
Hide file tree
Showing 14 changed files with 612 additions and 182 deletions.
2 changes: 0 additions & 2 deletions docs/spec/v1beta2/helmrepositories.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,6 @@ a deprecation warning will be logged.

### Cert secret reference

**Note:** TLS authentication is not yet supported by OCI Helm repositories.

`.spec.certSecretRef.name` is an optional field to specify a secret containing TLS
certificate data. The secret can contain the following keys:

Expand Down
27 changes: 19 additions & 8 deletions internal/controller/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
if err != nil {
return chartRepoConfigErrorReturn(err, obj)
}
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)

clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
e := &serror.Event{
Err: err,
Expand All @@ -521,6 +522,15 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
if certsTmpDir != "" {
defer func() {
if err := os.RemoveAll(certsTmpDir); err != nil {
r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
"failed to delete temporary certificates directory: %s", err)
}
}()
}

getterOpts := clientOpts.GetterOpts

// Initialize the chart repository
Expand All @@ -536,7 +546,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, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil)
registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry())
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to construct Helm client: %w", err),
Expand Down Expand Up @@ -585,8 +595,8 @@ 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 clientOpts.RegLoginOpt != nil {
err = ociChartRepo.Login(clientOpts.RegLoginOpt)
if clientOpts.MustLoginToRegistry() {
err = ociChartRepo.Login(clientOpts.RegLoginOpts...)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to login to OCI registry: %w", err),
Expand Down Expand Up @@ -983,15 +993,15 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()

clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
return nil, err
}
getterOpts := clientOpts.GetterOpts

var chartRepo repository.Downloader
if helmreg.IsOCI(normalizedURL) {
registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil)
registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry())
if err != nil {
return nil, fmt.Errorf("failed to create registry client: %w", err)
}
Expand All @@ -1002,6 +1012,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
repository.WithOCIGetterOptions(getterOpts),
repository.WithOCIRegistryClient(registryClient),
repository.WithCertificatesStore(certsTmpDir),
repository.WithCredentialsFile(credentialsFile))
if err != nil {
errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))
Expand All @@ -1016,8 +1027,8 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont

// 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 clientOpts.RegLoginOpt != nil {
err = ociChartRepo.Login(clientOpts.RegLoginOpt)
if clientOpts.MustLoginToRegistry() {
err = ociChartRepo.Login(clientOpts.RegLoginOpts...)
if err != nil {
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository: %w", err))
// clean up the credentialsFile
Expand Down
161 changes: 125 additions & 36 deletions internal/controller/helmchart_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
g.Expect(err).NotTo(HaveOccurred())

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

storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
Expand Down Expand Up @@ -2244,53 +2244,74 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
url string
registryOpts registryOptions
secretOpts secretOptions
secret *corev1.Secret
certsecret *corev1.Secret
insecure bool
provider string
providerImg string
want sreconcile.Result
wantErr bool
assertConditions []metav1.Condition
}{
{
name: "HTTP without basic auth",
want: sreconcile.ResultSuccess,
name: "HTTP without basic auth",
want: sreconcile.ResultSuccess,
insecure: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
{
name: "HTTP with basic auth secret",
want: sreconcile.ResultSuccess,
name: "HTTP with basic auth secret",
want: sreconcile.ResultSuccess,
insecure: true,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
{
name: "HTTP registry - basic auth with invalid secret",
want: sreconcile.ResultEmpty,
wantErr: true,
name: "HTTP registry - basic auth with invalid secret",
want: sreconcile.ResultEmpty,
wantErr: true,
insecure: true,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: "wrong-pass",
password: "wrong-pass",
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"),
},
},
{
name: "with contextual login provider",
wantErr: true,
insecure: true,
provider: "aws",
providerImg: "oci://123456789000.dkr.ecr.us-east-2.amazonaws.com/test",
assertConditions: []metav1.Condition{
Expand All @@ -2303,16 +2324,87 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
registryOpts: registryOptions{
withBasicAuth: true,
},
insecure: true,
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
provider: "azure",
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
{
name: "HTTPS With invalid CA cert",
wantErr: true,
registryOpts: registryOptions{
withTLS: true,
withClientCertAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
certsecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "certs-secretref",
},
Data: map[string][]byte{
"caFile": []byte("invalid caFile"),
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid caFile"),
},
},
{
name: "HTTPS With CA cert",
want: sreconcile.ResultSuccess,
registryOpts: registryOptions{
withTLS: true,
withClientCertAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
certsecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "certs-secretref",
},
Data: map[string][]byte{
"caFile": tlsCA,
"certFile": clientPublicKey,
"keyFile": clientPrivateKey,
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
}

for _, tt := range tests {
Expand All @@ -2325,7 +2417,9 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {

workspaceDir := t.TempDir()

tt.registryOpts.disableDNSMocking = true
if tt.insecure {
tt.registryOpts.disableDNSMocking = true
}
server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts)
g.Expect(err).NotTo(HaveOccurred())
t.Cleanup(func() {
Expand All @@ -2337,7 +2431,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())

// Upload the test chart
metadata, err := loadTestChartToOCI(chartData, chartPath, server)
metadata, err := loadTestChartToOCI(chartData, server, "testdata/certs/client.pem", "testdata/certs/client-key.pem", "testdata/certs/ca.pem")
g.Expect(err).ToNot(HaveOccurred())

repo := &helmv1.HelmRepository{
Expand All @@ -2364,25 +2458,26 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
}

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)),
},
}
tt.secret.Data[".dockerconfigjson"] = []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`,
server.registryHost, tt.secretOpts.username, tt.secretOpts.password))
}

if tt.secret != nil {
repo.Spec.SecretRef = &meta.LocalObjectReference{
Name: secret.Name,
Name: tt.secret.Name,
}
clientBuilder.WithObjects(secret, repo)
} else {
clientBuilder.WithObjects(repo)
clientBuilder.WithObjects(tt.secret)
}

if tt.certsecret != nil {
repo.Spec.CertSecretRef = &meta.LocalObjectReference{
Name: tt.certsecret.Name,
}
clientBuilder.WithObjects(tt.certsecret)
}

clientBuilder.WithObjects(repo)

obj := &helmv1.HelmChart{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
Expand Down Expand Up @@ -2456,7 +2551,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T
g.Expect(err).ToNot(HaveOccurred())

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

storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
Expand Down Expand Up @@ -2687,30 +2782,24 @@ func extractChartMeta(chartData []byte) (*hchart.Metadata, error) {
return ch.Metadata, nil
}

func loadTestChartToOCI(chartData []byte, chartPath string, server *registryClientTestServer) (*hchart.Metadata, error) {
func loadTestChartToOCI(chartData []byte, server *registryClientTestServer, certFile, keyFile, cafile string) (*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 = os.ReadFile(chartPath)
helmreg.LoginOptTLSClientConfig(certFile, keyFile, cafile))
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to login to OCI registry: %w", err)
}
metadata, err := extractChartMeta(chartData)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to extract chart metadata: %w", 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 nil, fmt.Errorf("failed to push chart: %w", err)
}

return metadata, nil
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/helmrepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
return sreconcile.ResultEmpty, e
}

clientOpts, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL)
clientOpts, _, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL)
if err != nil {
if errors.Is(err, getter.ErrDeprecatedTLSConfig) {
ctrl.LoggerFrom(ctx).
Expand Down
Loading

0 comments on commit d45c08c

Please sign in to comment.