Skip to content

🐛 pointer checks if user doesn't specify CASecret #253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2024
Merged

🐛 pointer checks if user doesn't specify CASecret #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controllers/helmchartproxy/helmchartproxy_controller_phases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func constructHelmReleaseProxy(existing *addonsv1alpha1.HelmReleaseProxy, helmCh

helmReleaseProxy.Spec.TLSConfig = helmChartProxy.Spec.TLSConfig

if helmReleaseProxy.Spec.TLSConfig != nil {
if helmReleaseProxy.Spec.TLSConfig != nil && helmReleaseProxy.Spec.TLSConfig.CASecretRef != nil {
// If the namespace is not set, set it to the namespace of the HelmChartProxy
if helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace == "" {
helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace = helmChartProxy.Namespace
Expand Down
79 changes: 79 additions & 0 deletions controllers/helmchartproxy/helmchartproxy_controller_phases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,85 @@ func TestConstructHelmReleaseProxy(t *testing.T) {
},
},
},
{
name: "construct helm release proxy with insecure TLS",
existing: nil,
helmChartProxy: &addonsv1alpha1.HelmChartProxy{
TypeMeta: metav1.TypeMeta{
APIVersion: addonsv1alpha1.GroupVersion.String(),
Kind: "HelmChartProxy",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-hcp",
Namespace: "test-namespace",
},
Spec: addonsv1alpha1.HelmChartProxySpec{
ReleaseName: "test-release-name",
ChartName: "test-chart-name",
RepoURL: "https://test-repo-url",
ReleaseNamespace: "test-release-namespace",
Options: addonsv1alpha1.HelmOptions{
EnableClientCache: true,
Timeout: &metav1.Duration{
Duration: 10 * time.Minute,
},
},
TLSConfig: &addonsv1alpha1.TLSConfig{
InsecureSkipTLSVerify: true,
},
},
},
parsedValues: "test-parsed-values",
cluster: &clusterv1.Cluster{
TypeMeta: metav1.TypeMeta{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Cluster",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
},
},
expected: &addonsv1alpha1.HelmReleaseProxy{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-chart-name-test-cluster-",
Namespace: "test-namespace",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: addonsv1alpha1.GroupVersion.String(),
Kind: "HelmChartProxy",
Name: "test-hcp",
Controller: ptr.To(true),
BlockOwnerDeletion: ptr.To(true),
},
},
Labels: map[string]string{
clusterv1.ClusterNameLabel: "test-cluster",
addonsv1alpha1.HelmChartProxyLabelName: "test-hcp",
},
},
Spec: addonsv1alpha1.HelmReleaseProxySpec{
ClusterRef: corev1.ObjectReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Cluster",
Name: "test-cluster",
},
ReleaseName: "test-release-name",
ChartName: "test-chart-name",
RepoURL: "https://test-repo-url",
ReleaseNamespace: "test-release-namespace",
Values: "test-parsed-values",
Options: addonsv1alpha1.HelmOptions{
EnableClientCache: true,
Timeout: &metav1.Duration{
Duration: 10 * time.Minute,
},
},
TLSConfig: &addonsv1alpha1.TLSConfig{
InsecureSkipTLSVerify: true,
},
},
},
},
}

for _, tc := range testCases {
Expand Down
4 changes: 3 additions & 1 deletion controllers/helmreleaseproxy/helmreleaseproxy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,9 @@ func (r *HelmReleaseProxyReconciler) getCredentials(ctx context.Context, helmRel
// getCAFile fetches the CA certificate from a Secret and writes it to a temporary file, returning the path to the temporary file.
func (r *HelmReleaseProxyReconciler) getCAFile(ctx context.Context, helmReleaseProxy *addonsv1alpha1.HelmReleaseProxy) (string, error) {
caFilePath := ""
if helmReleaseProxy.Spec.TLSConfig != nil && helmReleaseProxy.Spec.TLSConfig.CASecretRef.Name != "" {
if helmReleaseProxy.Spec.TLSConfig != nil &&
helmReleaseProxy.Spec.TLSConfig.CASecretRef != nil &&
helmReleaseProxy.Spec.TLSConfig.CASecretRef.Name != "" {
// By default, the secret is in the same namespace as the HelmReleaseProxy
if helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace == "" {
helmReleaseProxy.Spec.TLSConfig.CASecretRef.Namespace = helmReleaseProxy.Namespace
Expand Down
94 changes: 94 additions & 0 deletions controllers/helmreleaseproxy/helmreleaseproxy_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,34 @@ var (
},
}

defaultProxyWithSkipTLS = &addonsv1alpha1.HelmReleaseProxy{
TypeMeta: metav1.TypeMeta{
Kind: "HelmReleaseProxy",
APIVersion: "addons.cluster.x-k8s.io/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-proxy",
Namespace: "default",
},
Spec: addonsv1alpha1.HelmReleaseProxySpec{
ClusterRef: corev1.ObjectReference{
APIVersion: "cluster.x-k8s.io/v1beta1",
Kind: "Cluster",
Namespace: "default",
Name: "test-cluster",
},
RepoURL: "https://test-repo",
ChartName: "test-chart",
Version: "test-version",
ReleaseName: "test-release",
ReleaseNamespace: "default",
Values: "test-values",
TLSConfig: &addonsv1alpha1.TLSConfig{
InsecureSkipTLSVerify: true,
},
},
}

generateNameProxy = &addonsv1alpha1.HelmReleaseProxy{
TypeMeta: metav1.TypeMeta{
Kind: "HelmReleaseProxy",
Expand Down Expand Up @@ -543,6 +571,72 @@ func TestReconcileDelete(t *testing.T) {
}
}

func TestTLSSettings(t *testing.T) {
t.Parallel()

testcases := []struct {
name string
helmReleaseProxy *addonsv1alpha1.HelmReleaseProxy
clientExpect func(g *WithT, c *mocks.MockClientMockRecorder)
expect func(g *WithT, hrp *addonsv1alpha1.HelmReleaseProxy)
expectedError string
}{
{
name: "test",
helmReleaseProxy: defaultProxyWithSkipTLS.DeepCopy(),
clientExpect: func(g *WithT, c *mocks.MockClientMockRecorder) {
c.InstallOrUpgradeHelmRelease(ctx, restConfig, "", "", defaultProxyWithSkipTLS.Spec).Return(&helmRelease.Release{
Name: "test-release",
Version: 1,
Info: &helmRelease.Info{
Status: helmRelease.StatusDeployed,
},
}, nil).Times(1)
},
expect: func(g *WithT, hrp *addonsv1alpha1.HelmReleaseProxy) {
_, ok := hrp.Annotations[addonsv1alpha1.IsReleaseNameGeneratedAnnotation]
g.Expect(ok).To(BeFalse())
g.Expect(hrp.Spec.ReleaseName).To(Equal("test-release"))
g.Expect(hrp.Status.Revision).To(Equal(1))
g.Expect(hrp.Status.Status).To(BeEquivalentTo(helmRelease.StatusDeployed))

g.Expect(conditions.Has(hrp, addonsv1alpha1.HelmReleaseReadyCondition)).To(BeTrue())
g.Expect(conditions.IsTrue(hrp, addonsv1alpha1.HelmReleaseReadyCondition)).To(BeTrue())
},
expectedError: "",
},
}
for _, tc := range testcases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)
t.Parallel()
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

clientMock := mocks.NewMockClient(mockCtrl)
tc.clientExpect(g, clientMock.EXPECT())

r := &HelmReleaseProxyReconciler{
Client: fake.NewClientBuilder().
WithScheme(fakeScheme).
WithStatusSubresource(&addonsv1alpha1.HelmReleaseProxy{}).
Build(),
}
caFilePath, err := r.getCAFile(ctx, tc.helmReleaseProxy)
g.Expect(err).ToNot(HaveOccurred(), "did not expect error to get CA file")
err = r.reconcileNormal(ctx, tc.helmReleaseProxy, clientMock, "", caFilePath, restConfig)
if tc.expectedError != "" {
g.Expect(err).To(HaveOccurred())
g.Expect(err).To(MatchError(tc.expectedError), err.Error())
} else {
g.Expect(err).NotTo(HaveOccurred())
tc.expect(g, tc.helmReleaseProxy)
}
})
}
}

func init() {
_ = scheme.AddToScheme(fakeScheme)
_ = clusterv1.AddToScheme(fakeScheme)
Expand Down
Loading