Skip to content

Commit f801efc

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

File tree

4 files changed

+177
-2
lines changed

4 files changed

+177
-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: 94 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,72 @@ func TestReconcileDelete(t *testing.T) {
543571
}
544572
}
545573

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

0 commit comments

Comments
 (0)