Skip to content

Commit 1c89844

Browse files
faiqfaiq
committed
fix: pointer checks if user doesn't specify CASecret
1 parent 5eb79f5 commit 1c89844

File tree

4 files changed

+175
-2
lines changed

4 files changed

+175
-2
lines changed

‎controllers/helmchartproxy/helmchartproxy_controller_phases.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ func constructHelmReleaseProxy(existing *addonsv1alpha1.HelmReleaseProxy, helmCh
237237

238238
helmReleaseProxy.Spec.TLSConfig = helmChartProxy.Spec.TLSConfig
239239

240-
if helmReleaseProxy.Spec.TLSConfig != nil {
240+
if helmReleaseProxy.Spec.TLSConfig != nil && helmReleaseProxy.Spec.TLSConfig.CASecretRef != nil {
241241
// If the namespace is not set, set it to the namespace of the HelmChartProxy
242242
if helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace == "" {
243243
helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace = helmChartProxy.Namespace

‎controllers/helmchartproxy/helmchartproxy_controller_phases_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,6 +1010,85 @@ func TestConstructHelmReleaseProxy(t *testing.T) {
10101010
},
10111011
},
10121012
},
1013+
{
1014+
name: "construct helm release proxy with insecure TLS",
1015+
existing: nil,
1016+
helmChartProxy: &addonsv1alpha1.HelmChartProxy{
1017+
TypeMeta: metav1.TypeMeta{
1018+
APIVersion: addonsv1alpha1.GroupVersion.String(),
1019+
Kind: "HelmChartProxy",
1020+
},
1021+
ObjectMeta: metav1.ObjectMeta{
1022+
Name: "test-hcp",
1023+
Namespace: "test-namespace",
1024+
},
1025+
Spec: addonsv1alpha1.HelmChartProxySpec{
1026+
ReleaseName: "test-release-name",
1027+
ChartName: "test-chart-name",
1028+
RepoURL: "https://test-repo-url",
1029+
ReleaseNamespace: "test-release-namespace",
1030+
Options: addonsv1alpha1.HelmOptions{
1031+
EnableClientCache: true,
1032+
Timeout: &metav1.Duration{
1033+
Duration: 10 * time.Minute,
1034+
},
1035+
},
1036+
TLSConfig: &addonsv1alpha1.TLSConfig{
1037+
InsecureSkipTLSVerify: true,
1038+
},
1039+
},
1040+
},
1041+
parsedValues: "test-parsed-values",
1042+
cluster: &clusterv1.Cluster{
1043+
TypeMeta: metav1.TypeMeta{
1044+
APIVersion: clusterv1.GroupVersion.String(),
1045+
Kind: "Cluster",
1046+
},
1047+
ObjectMeta: metav1.ObjectMeta{
1048+
Name: "test-cluster",
1049+
},
1050+
},
1051+
expected: &addonsv1alpha1.HelmReleaseProxy{
1052+
ObjectMeta: metav1.ObjectMeta{
1053+
GenerateName: "test-chart-name-test-cluster-",
1054+
Namespace: "test-namespace",
1055+
OwnerReferences: []metav1.OwnerReference{
1056+
{
1057+
APIVersion: addonsv1alpha1.GroupVersion.String(),
1058+
Kind: "HelmChartProxy",
1059+
Name: "test-hcp",
1060+
Controller: ptr.To(true),
1061+
BlockOwnerDeletion: ptr.To(true),
1062+
},
1063+
},
1064+
Labels: map[string]string{
1065+
clusterv1.ClusterNameLabel: "test-cluster",
1066+
addonsv1alpha1.HelmChartProxyLabelName: "test-hcp",
1067+
},
1068+
},
1069+
Spec: addonsv1alpha1.HelmReleaseProxySpec{
1070+
ClusterRef: corev1.ObjectReference{
1071+
APIVersion: clusterv1.GroupVersion.String(),
1072+
Kind: "Cluster",
1073+
Name: "test-cluster",
1074+
},
1075+
ReleaseName: "test-release-name",
1076+
ChartName: "test-chart-name",
1077+
RepoURL: "https://test-repo-url",
1078+
ReleaseNamespace: "test-release-namespace",
1079+
Values: "test-parsed-values",
1080+
Options: addonsv1alpha1.HelmOptions{
1081+
EnableClientCache: true,
1082+
Timeout: &metav1.Duration{
1083+
Duration: 10 * time.Minute,
1084+
},
1085+
},
1086+
TLSConfig: &addonsv1alpha1.TLSConfig{
1087+
InsecureSkipTLSVerify: true,
1088+
},
1089+
},
1090+
},
1091+
},
10131092
}
10141093

10151094
for _, tc := range testCases {

‎controllers/helmreleaseproxy/helmreleaseproxy_controller.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,9 @@ func (r *HelmReleaseProxyReconciler) getCredentials(ctx context.Context, helmRel
399399
// getCAFile fetches the CA certificate from a Secret and writes it to a temporary file, returning the path to the temporary file.
400400
func (r *HelmReleaseProxyReconciler) getCAFile(ctx context.Context, helmReleaseProxy *addonsv1alpha1.HelmReleaseProxy) (string, error) {
401401
caFilePath := ""
402-
if helmReleaseProxy.Spec.TLSConfig != nil && helmReleaseProxy.Spec.TLSConfig.CASecretRef.Name != "" {
402+
if helmReleaseProxy.Spec.TLSConfig != nil &&
403+
helmReleaseProxy.Spec.TLSConfig.CASecretRef != nil &&
404+
helmReleaseProxy.Spec.TLSConfig.CASecretRef.Name != "" {
403405
// By default, the secret is in the same namespace as the HelmReleaseProxy
404406
if helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace == "" {
405407
helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace = helmReleaseProxy.Namespace

‎controllers/helmreleaseproxy/helmreleaseproxy_controller_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,34 @@ var (
131131
},
132132
}
133133

134+
defaultProxyWithSkipTLS = &addonsv1alpha1.HelmReleaseProxy{
135+
TypeMeta: metav1.TypeMeta{
136+
Kind: "HelmReleaseProxy",
137+
APIVersion: "addons.cluster.x-k8s.io/v1alpha1",
138+
},
139+
ObjectMeta: metav1.ObjectMeta{
140+
Name: "test-proxy",
141+
Namespace: "default",
142+
},
143+
Spec: addonsv1alpha1.HelmReleaseProxySpec{
144+
ClusterRef: corev1.ObjectReference{
145+
APIVersion: "cluster.x-k8s.io/v1beta1",
146+
Kind: "Cluster",
147+
Namespace: "default",
148+
Name: "test-cluster",
149+
},
150+
RepoURL: "https://test-repo",
151+
ChartName: "test-chart",
152+
Version: "test-version",
153+
ReleaseName: "test-release",
154+
ReleaseNamespace: "default",
155+
Values: "test-values",
156+
TLSConfig: &addonsv1alpha1.TLSConfig{
157+
InsecureSkipTLSVerify: true,
158+
},
159+
},
160+
}
161+
134162
generateNameProxy = &addonsv1alpha1.HelmReleaseProxy{
135163
TypeMeta: metav1.TypeMeta{
136164
Kind: "HelmReleaseProxy",
@@ -543,6 +571,70 @@ func TestReconcileDelete(t *testing.T) {
543571
}
544572
}
545573

574+
func TestTLSSettings(t *testing.T) {
575+
testcases := []struct {
576+
name string
577+
helmReleaseProxy *addonsv1alpha1.HelmReleaseProxy
578+
clientExpect func(g *WithT, c *mocks.MockClientMockRecorder)
579+
expect func(g *WithT, hrp *addonsv1alpha1.HelmReleaseProxy)
580+
expectedError string
581+
}{
582+
{
583+
name: "test",
584+
helmReleaseProxy: defaultProxyWithSkipTLS.DeepCopy(),
585+
clientExpect: func(g *WithT, c *mocks.MockClientMockRecorder) {
586+
c.InstallOrUpgradeHelmRelease(ctx, restConfig, "", "", defaultProxyWithSkipTLS.Spec).Return(&helmRelease.Release{
587+
Name: "test-release",
588+
Version: 1,
589+
Info: &helmRelease.Info{
590+
Status: helmRelease.StatusDeployed,
591+
},
592+
}, nil).Times(1)
593+
},
594+
expect: func(g *WithT, hrp *addonsv1alpha1.HelmReleaseProxy) {
595+
_, ok := hrp.Annotations[addonsv1alpha1.IsReleaseNameGeneratedAnnotation]
596+
g.Expect(ok).To(BeFalse())
597+
g.Expect(hrp.Spec.ReleaseName).To(Equal("test-release"))
598+
g.Expect(hrp.Status.Revision).To(Equal(1))
599+
g.Expect(hrp.Status.Status).To(BeEquivalentTo(helmRelease.StatusDeployed))
600+
601+
g.Expect(conditions.Has(hrp, addonsv1alpha1.HelmReleaseReadyCondition)).To(BeTrue())
602+
g.Expect(conditions.IsTrue(hrp, addonsv1alpha1.HelmReleaseReadyCondition)).To(BeTrue())
603+
},
604+
expectedError: "",
605+
},
606+
}
607+
for _, tc := range testcases {
608+
tc := tc
609+
t.Run(tc.name, func(t *testing.T) {
610+
g := NewWithT(t)
611+
t.Parallel()
612+
mockCtrl := gomock.NewController(t)
613+
defer mockCtrl.Finish()
614+
615+
clientMock := mocks.NewMockClient(mockCtrl)
616+
tc.clientExpect(g, clientMock.EXPECT())
617+
618+
r := &HelmReleaseProxyReconciler{
619+
Client: fake.NewClientBuilder().
620+
WithScheme(fakeScheme).
621+
WithStatusSubresource(&addonsv1alpha1.HelmReleaseProxy{}).
622+
Build(),
623+
}
624+
caFilePath, err := r.getCAFile(ctx, tc.helmReleaseProxy)
625+
g.Expect(err).To(BeNil(), "did not expect error to get CA file")
626+
err = r.reconcileNormal(ctx, tc.helmReleaseProxy, clientMock, "", caFilePath, restConfig)
627+
if tc.expectedError != "" {
628+
g.Expect(err).To(HaveOccurred())
629+
g.Expect(err).To(MatchError(tc.expectedError), err.Error())
630+
} else {
631+
g.Expect(err).NotTo(HaveOccurred())
632+
tc.expect(g, tc.helmReleaseProxy)
633+
}
634+
})
635+
}
636+
}
637+
546638
func init() {
547639
_ = scheme.AddToScheme(fakeScheme)
548640
_ = clusterv1.AddToScheme(fakeScheme)

0 commit comments

Comments
 (0)