Skip to content

Commit

Permalink
Add unit tests for custom certs and insecureSkipTLSVerify in helm OCI
Browse files Browse the repository at this point in the history 
Signed-off-by: Soule BA <soule@weave.works>
  • Loading branch information
@souleb
souleb committed May 17, 2023
1 parent 77224b9 commit cd898af
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 30 deletions.
4 changes: 2 additions & 2 deletions api/v1beta2/helmrepository_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ type HelmRepositorySpec struct {
// +optional
Timeout *metav1.Duration `json:"timeout,omitempty"`

// InsecureSkipTLSverify skips the validation of the TLS certificate of the
// InsecureSkipTLSVerify skips the validation of the TLS certificate of the
// OCI registry endpoint.
// +optional
InsecureSkipTLSverify bool `json:"insecureSkipTLSverify,omitempty"`
InsecureSkipTLSVerify bool `json:"insecureSkipTLSverify,omitempty"`

// Suspend tells the controller to suspend the reconciliation of this
// HelmRepository.
Expand Down
26 changes: 26 additions & 0 deletions docs/api/v1beta2/source.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,19 @@ Its default value is 60s.</p>
</tr>
<tr>
<td>
<code>insecureSkipTLSverify</code><br>
<em>
bool
</em>
</td>
<td>
<em>(Optional)</em>
<p>InsecureSkipTLSverify skips the validation of the TLS certificate of the
OCI registry endpoint.</p>
</td>
</tr>
<tr>
<td>
<code>suspend</code><br>
<em>
bool
Expand Down Expand Up @@ -2511,6 +2524,19 @@ Its default value is 60s.</p>
</tr>
<tr>
<td>
<code>insecureSkipTLSverify</code><br>
<em>
bool
</em>
</td>
<td>
<em>(Optional)</em>
<p>InsecureSkipTLSverify skips the validation of the TLS certificate of the
OCI registry endpoint.</p>
</td>
</tr>
<tr>
<td>
<code>suspend</code><br>
<em>
bool
Expand Down
12 changes: 8 additions & 4 deletions internal/controller/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,10 +579,12 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
}
}

if repo.Spec.InsecureSkipTLSverify {
tlsConfig.InsecureSkipVerify = true
if tlsConfig == nil {
tlsConfig = &tls.Config{}
}

tlsConfig.InsecureSkipVerify = repo.Spec.InsecureSkipTLSVerify

loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
if err != nil {
e := &serror.Event{
Expand Down Expand Up @@ -1093,10 +1095,12 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
}
}

if obj.Spec.InsecureSkipTLSverify {
tlsConfig.InsecureSkipVerify = true
if tlsConfig == nil {
tlsConfig = &tls.Config{}
}

tlsConfig.InsecureSkipVerify = obj.Spec.InsecureSkipTLSVerify

loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
if err != nil {
return nil, err
Expand Down
89 changes: 79 additions & 10 deletions internal/controller/helmchart_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2249,18 +2249,20 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
type secretOptions struct {
username string
password string
ca []byte
}

tests := []struct {
name string
url string
registryOpts registryOptions
secretOpts secretOptions
provider string
providerImg string
want sreconcile.Result
wantErr bool
assertConditions []metav1.Condition
name string
url string
registryOpts registryOptions
secretOpts secretOptions
insecureSkipTLSVerify bool
provider string
providerImg string
want sreconcile.Result
wantErr bool
assertConditions []metav1.Condition
}{
{
name: "HTTP without basic auth",
Expand Down Expand Up @@ -2325,6 +2327,53 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
*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{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
ca: []byte("invalid-ca"),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"),
},
},
{
name: "HTTPS With CA cert",
want: sreconcile.ResultSuccess,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
ca: []byte(tlsCA),
},
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 InsecureSkipTLSVerify",
want: sreconcile.ResultSuccess,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
insecureSkipTLSVerify: 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'"),
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -2368,8 +2417,11 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
repo.Spec.URL = tt.providerImg
}

repo.Spec.InsecureSkipTLSVerify = tt.insecureSkipTLSVerify

var secret *corev1.Secret
if tt.secretOpts.username != "" && tt.secretOpts.password != "" {
secret := &corev1.Secret{
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Expand All @@ -2379,7 +2431,24 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
server.registryHost, tt.secretOpts.username, tt.secretOpts.password)),
},
}
}

if tt.secretOpts.ca != nil {
if secret == nil {
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Data: map[string][]byte{
"caFile": tt.secretOpts.ca,
},
}
} else {
secret.Data["caFile"] = tt.secretOpts.ca
}
}

if secret != nil {
repo.Spec.SecretRef = &meta.LocalObjectReference{
Name: secret.Name,
}
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 @@ -443,7 +443,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
}
}

if obj.Spec.InsecureSkipTLSverify {
if obj.Spec.InsecureSkipTLSVerify {
tlsConfig.InsecureSkipVerify = true
}

Expand Down
6 changes: 4 additions & 2 deletions internal/controller/helmrepository_controller_oci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,12 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
}
}

if obj.Spec.InsecureSkipTLSverify {
tlsConfig.InsecureSkipVerify = true
if tlsConfig == nil {
tlsConfig = &tls.Config{}
}

tlsConfig.InsecureSkipVerify = obj.Spec.InsecureSkipTLSVerify

loginOpt, err := makeLoginOption(authenticator, keychain, obj.Spec.URL)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
Expand Down
89 changes: 78 additions & 11 deletions internal/controller/helmrepository_controller_oci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,18 +164,20 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
type secretOptions struct {
username string
password string
ca []byte
}

tests := []struct {
name string
url string
registryOpts registryOptions
secretOpts secretOptions
provider string
providerImg string
want ctrl.Result
wantErr bool
assertConditions []metav1.Condition
name string
url string
registryOpts registryOptions
secretOpts secretOptions
insecureSkipTLSVerify bool
provider string
providerImg string
want ctrl.Result
wantErr bool
assertConditions []metav1.Condition
}{
{
name: "HTTP without basic auth",
Expand Down Expand Up @@ -239,6 +241,52 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"),
},
},
{
name: "HTTPS With invalid CA cert",
wantErr: true,
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
ca: []byte("invalid-ca"),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"),
*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to login to registry"),
},
},
{
name: "HTTPS With CA cert",
want: ctrl.Result{RequeueAfter: interval},
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
ca: []byte(tlsCA),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"),
},
},
{
name: "HTTPS With InsecureSkipTLSVerify",
want: ctrl.Result{RequeueAfter: interval},
registryOpts: registryOptions{
withBasicAuth: true,
},
secretOpts: secretOptions{
username: testRegistryUsername,
password: testRegistryPassword,
},
insecureSkipTLSVerify: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"),
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -274,8 +322,11 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
obj.Spec.URL = tt.providerImg
}

obj.Spec.InsecureSkipTLSVerify = tt.insecureSkipTLSVerify

var secret *corev1.Secret
if tt.secretOpts.username != "" && tt.secretOpts.password != "" {
secret := &corev1.Secret{
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Expand All @@ -285,9 +336,25 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
server.registryHost, tt.secretOpts.username, tt.secretOpts.password)),
},
}
}

builder.WithObjects(secret)
if tt.secretOpts.ca != nil {
if secret == nil {
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "auth-secretref",
},
Data: map[string][]byte{
"caFile": tt.secretOpts.ca,
},
}
} else {
secret.Data["caFile"] = tt.secretOpts.ca
}
}

if secret != nil {
builder.WithObjects(secret)
obj.Spec.SecretRef = &meta.LocalObjectReference{
Name: secret.Name,
}
Expand Down

0 comments on commit cd898af

Please sign in to comment.