Skip to content

Commit

Permalink
Merge pull request #1097 from souleb/custom-cert-oci
Browse files Browse the repository at this point in the history 
Helm OCI: Add support for TLS registries with self-signed certs
  • Loading branch information
@stefanprodan
stefanprodan authored Aug 7, 2023
2 parents 6377c6f + d45c08c commit 6fa8d05
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 6fa8d05

Please sign in to comment.