Skip to content

Commit 992a230

Browse files
soulebsouleb
committed
Add unit tests for custom certs and insecureSkipTLSVerify in helm OCI
Signed-off-by: Soule BA <[email protected]>
1 parent 3e3008f commit 992a230

File tree

7 files changed

+198
-30
lines changed

7 files changed

+198
-30
lines changed

‎api/v1beta2/helmrepository_types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ type HelmRepositorySpec struct {
8080
// +optional
8181
Timeout *metav1.Duration `json:"timeout,omitempty"`
8282

83-
// InsecureSkipTLSverify skips the validation of the TLS certificate of the
83+
// InsecureSkipTLSVerify skips the validation of the TLS certificate of the
8484
// OCI registry endpoint.
8585
// +optional
86-
InsecureSkipTLSverify bool `json:"insecureSkipTLSverify,omitempty"`
86+
InsecureSkipTLSVerify bool `json:"insecureSkipTLSverify,omitempty"`
8787

8888
// Suspend tells the controller to suspend the reconciliation of this
8989
// HelmRepository.

‎docs/api/v1beta2/source.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,19 @@ Its default value is 60s.</p>
844844
</tr>
845845
<tr>
846846
<td>
847+
<code>insecureSkipTLSverify</code><br>
848+
<em>
849+
bool
850+
</em>
851+
</td>
852+
<td>
853+
<em>(Optional)</em>
854+
<p>InsecureSkipTLSverify skips the validation of the TLS certificate of the
855+
OCI registry endpoint.</p>
856+
</td>
857+
</tr>
858+
<tr>
859+
<td>
847860
<code>suspend</code><br>
848861
<em>
849862
bool
@@ -2511,6 +2524,19 @@ Its default value is 60s.</p>
25112524
</tr>
25122525
<tr>
25132526
<td>
2527+
<code>insecureSkipTLSverify</code><br>
2528+
<em>
2529+
bool
2530+
</em>
2531+
</td>
2532+
<td>
2533+
<em>(Optional)</em>
2534+
<p>InsecureSkipTLSverify skips the validation of the TLS certificate of the
2535+
OCI registry endpoint.</p>
2536+
</td>
2537+
</tr>
2538+
<tr>
2539+
<td>
25142540
<code>suspend</code><br>
25152541
<em>
25162542
bool

‎internal/controller/helmchart_controller.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,10 +576,12 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
576576
}
577577
}
578578

579-
if repo.Spec.InsecureSkipTLSverify {
580-
tlsConfig.InsecureSkipVerify = true
579+
if tlsConfig == nil {
580+
tlsConfig = &tls.Config{}
581581
}
582582

583+
tlsConfig.InsecureSkipVerify = repo.Spec.InsecureSkipTLSVerify
584+
583585
loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
584586
if err != nil {
585587
e := &serror.Event{
@@ -1090,10 +1092,12 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
10901092
}
10911093
}
10921094

1093-
if obj.Spec.InsecureSkipTLSverify {
1094-
tlsConfig.InsecureSkipVerify = true
1095+
if tlsConfig == nil {
1096+
tlsConfig = &tls.Config{}
10951097
}
10961098

1099+
tlsConfig.InsecureSkipVerify = obj.Spec.InsecureSkipTLSVerify
1100+
10971101
loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
10981102
if err != nil {
10991103
return nil, err

‎internal/controller/helmchart_controller_test.go

Lines changed: 79 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2274,18 +2274,20 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
22742274
type secretOptions struct {
22752275
username string
22762276
password string
2277+
ca []byte
22772278
}
22782279

22792280
tests := []struct {
2280-
name string
2281-
url string
2282-
registryOpts registryOptions
2283-
secretOpts secretOptions
2284-
provider string
2285-
providerImg string
2286-
want sreconcile.Result
2287-
wantErr bool
2288-
assertConditions []metav1.Condition
2281+
name string
2282+
url string
2283+
registryOpts registryOptions
2284+
secretOpts secretOptions
2285+
insecureSkipTLSVerify bool
2286+
provider string
2287+
providerImg string
2288+
want sreconcile.Result
2289+
wantErr bool
2290+
assertConditions []metav1.Condition
22892291
}{
22902292
{
22912293
name: "HTTP without basic auth",
@@ -2350,6 +2352,53 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
23502352
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
23512353
},
23522354
},
2355+
{
2356+
name: "HTTPS With invalid CA cert",
2357+
wantErr: true,
2358+
registryOpts: registryOptions{
2359+
withBasicAuth: true,
2360+
},
2361+
secretOpts: secretOptions{
2362+
username: testRegistryUsername,
2363+
password: testRegistryPassword,
2364+
ca: []byte("invalid-ca"),
2365+
},
2366+
assertConditions: []metav1.Condition{
2367+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"),
2368+
},
2369+
},
2370+
{
2371+
name: "HTTPS With CA cert",
2372+
want: sreconcile.ResultSuccess,
2373+
registryOpts: registryOptions{
2374+
withBasicAuth: true,
2375+
},
2376+
secretOpts: secretOptions{
2377+
username: testRegistryUsername,
2378+
password: testRegistryPassword,
2379+
ca: []byte(tlsCA),
2380+
},
2381+
assertConditions: []metav1.Condition{
2382+
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
2383+
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
2384+
},
2385+
},
2386+
{
2387+
name: "HTTPS With InsecureSkipTLSVerify",
2388+
want: sreconcile.ResultSuccess,
2389+
registryOpts: registryOptions{
2390+
withBasicAuth: true,
2391+
},
2392+
secretOpts: secretOptions{
2393+
username: testRegistryUsername,
2394+
password: testRegistryPassword,
2395+
},
2396+
insecureSkipTLSVerify: true,
2397+
assertConditions: []metav1.Condition{
2398+
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
2399+
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
2400+
},
2401+
},
23532402
}
23542403

23552404
for _, tt := range tests {
@@ -2396,8 +2445,11 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
23962445
repo.Spec.URL = tt.providerImg
23972446
}
23982447

2448+
repo.Spec.InsecureSkipTLSVerify = tt.insecureSkipTLSVerify
2449+
2450+
var secret *corev1.Secret
23992451
if tt.secretOpts.username != "" && tt.secretOpts.password != "" {
2400-
secret := &corev1.Secret{
2452+
secret = &corev1.Secret{
24012453
ObjectMeta: metav1.ObjectMeta{
24022454
Name: "auth-secretref",
24032455
},
@@ -2407,7 +2459,24 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
24072459
server.registryHost, tt.secretOpts.username, tt.secretOpts.password)),
24082460
},
24092461
}
2462+
}
2463+
2464+
if tt.secretOpts.ca != nil {
2465+
if secret == nil {
2466+
secret = &corev1.Secret{
2467+
ObjectMeta: metav1.ObjectMeta{
2468+
Name: "auth-secretref",
2469+
},
2470+
Data: map[string][]byte{
2471+
"caFile": tt.secretOpts.ca,
2472+
},
2473+
}
2474+
} else {
2475+
secret.Data["caFile"] = tt.secretOpts.ca
2476+
}
2477+
}
24102478

2479+
if secret != nil {
24112480
repo.Spec.SecretRef = &meta.LocalObjectReference{
24122481
Name: secret.Name,
24132482
}

‎internal/controller/helmrepository_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
441441
}
442442
}
443443

444-
if obj.Spec.InsecureSkipTLSverify {
444+
if obj.Spec.InsecureSkipTLSVerify {
445445
tlsConfig.InsecureSkipVerify = true
446446
}
447447

‎internal/controller/helmrepository_controller_oci.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,12 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
349349
}
350350
}
351351

352-
if obj.Spec.InsecureSkipTLSverify {
353-
tlsConfig.InsecureSkipVerify = true
352+
if tlsConfig == nil {
353+
tlsConfig = &tls.Config{}
354354
}
355355

356+
tlsConfig.InsecureSkipVerify = obj.Spec.InsecureSkipTLSVerify
357+
356358
loginOpt, err := makeLoginOption(authenticator, keychain, obj.Spec.URL)
357359
if err != nil {
358360
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())

‎internal/controller/helmrepository_controller_oci_test.go

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -164,18 +164,20 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
164164
type secretOptions struct {
165165
username string
166166
password string
167+
ca []byte
167168
}
168169

169170
tests := []struct {
170-
name string
171-
url string
172-
registryOpts registryOptions
173-
secretOpts secretOptions
174-
provider string
175-
providerImg string
176-
want ctrl.Result
177-
wantErr bool
178-
assertConditions []metav1.Condition
171+
name string
172+
url string
173+
registryOpts registryOptions
174+
secretOpts secretOptions
175+
insecureSkipTLSVerify bool
176+
provider string
177+
providerImg string
178+
want ctrl.Result
179+
wantErr bool
180+
assertConditions []metav1.Condition
179181
}{
180182
{
181183
name: "HTTP without basic auth",
@@ -239,6 +241,52 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
239241
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"),
240242
},
241243
},
244+
{
245+
name: "HTTPS With invalid CA cert",
246+
wantErr: true,
247+
registryOpts: registryOptions{
248+
withBasicAuth: true,
249+
},
250+
secretOpts: secretOptions{
251+
username: testRegistryUsername,
252+
password: testRegistryPassword,
253+
ca: []byte("invalid-ca"),
254+
},
255+
assertConditions: []metav1.Condition{
256+
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"),
257+
*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to login to registry"),
258+
},
259+
},
260+
{
261+
name: "HTTPS With CA cert",
262+
want: ctrl.Result{RequeueAfter: interval},
263+
registryOpts: registryOptions{
264+
withBasicAuth: true,
265+
},
266+
secretOpts: secretOptions{
267+
username: testRegistryUsername,
268+
password: testRegistryPassword,
269+
ca: []byte(tlsCA),
270+
},
271+
assertConditions: []metav1.Condition{
272+
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"),
273+
},
274+
},
275+
{
276+
name: "HTTPS With InsecureSkipTLSVerify",
277+
want: ctrl.Result{RequeueAfter: interval},
278+
registryOpts: registryOptions{
279+
withBasicAuth: true,
280+
},
281+
secretOpts: secretOptions{
282+
username: testRegistryUsername,
283+
password: testRegistryPassword,
284+
},
285+
insecureSkipTLSVerify: true,
286+
assertConditions: []metav1.Condition{
287+
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"),
288+
},
289+
},
242290
}
243291

244292
for _, tt := range tests {
@@ -277,8 +325,11 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
277325
obj.Spec.URL = tt.providerImg
278326
}
279327

328+
obj.Spec.InsecureSkipTLSVerify = tt.insecureSkipTLSVerify
329+
330+
var secret *corev1.Secret
280331
if tt.secretOpts.username != "" && tt.secretOpts.password != "" {
281-
secret := &corev1.Secret{
332+
secret = &corev1.Secret{
282333
ObjectMeta: metav1.ObjectMeta{
283334
Name: "auth-secretref",
284335
},
@@ -288,9 +339,25 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
288339
server.registryHost, tt.secretOpts.username, tt.secretOpts.password)),
289340
},
290341
}
342+
}
291343

292-
clientBuilder.WithObjects(secret)
344+
if tt.secretOpts.ca != nil {
345+
if secret == nil {
346+
secret = &corev1.Secret{
347+
ObjectMeta: metav1.ObjectMeta{
348+
Name: "auth-secretref",
349+
},
350+
Data: map[string][]byte{
351+
"caFile": tt.secretOpts.ca,
352+
},
353+
}
354+
} else {
355+
secret.Data["caFile"] = tt.secretOpts.ca
356+
}
357+
}
293358

359+
if secret != nil {
360+
clientBuilder.WithObjects(secret)
294361
obj.Spec.SecretRef = &meta.LocalObjectReference{
295362
Name: secret.Name,
296363
}

0 commit comments

Comments
 (0)