🐛 pointer checks if user doesn't specify CASecret

[faiq]

What this PR does / why we need it:

This PR fixes a nil pointer panic if a user does not specify CASecretRef in TLSConfig.

I0603 14:53:15.290968       1 controller.go:115] "Observed a panic in reconciler: runtime error: invalid memory address or nil pointer dereference" controller="helmchartproxy" controllerGroup="addons.cluster.x-k8s.io" controllerKind="HelmChartProxy" HelmChartProxy="default/```" namespace="default" name="node-feature-discovery-docker-cluster-cilium-helm-addon5" reconcileID="8498516d-cf78-49bb-bf2a-0ee6b355f62e"
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x20d81cd]

goroutine 268 [running]:
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:116 +0x1e5
panic({0x24a6b00?, 0x4314de0?})
	runtime/panic.go:914 +0x21f
sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy.constructHelmReleaseProxy(0x0, 0xc00088e900, {0xc0004d8d20, 0x1c5}, 0xc000aaf460)
	sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy/helmchartproxy_controller_phases.go:242 +0x84d
sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy.(*HelmChartProxyReconciler).createOrUpdateHelmReleaseProxy(0xc0002a6690, {0x2ce2120, 0xc000ae4870}, 0x0, 0x12?, 0xc000bcf460, {0xc0004d8d20, 0x1c5})
	sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy/helmchartproxy_controller_phases.go:144 +0xb3
sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy.(*HelmChartProxyReconciler).reconcileForCluster(_, {_, _}, _, {{{0x21f74a1, 0x7}, {0xc000a16078, 0x18}}, {{0xc0005c1140, 0x21}, ...}, ...})
	sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy/helmchartproxy_controller_phases.go:98 +0x71e
sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy.(*HelmChartProxyReconciler).reconcileNormal(0x28edbe7?, {0x2ce2120, 0xc000ae4870}, 0xc00088e900, {0xc000a3a9c0, 0x1, 0x1}, {0x43ad560, 0x0, 0x0})
	sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy/helmchartproxy_controller.go:221 +0x2f8
sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy.(*HelmChartProxyReconciler).Reconcile(0xc0002a6690, {0x2ce2120?, 0xc000ae4870}, {{{0xc0000bd4a0, 0x7}, {0xc000a966c0, 0x38}}})
	sigs.k8s.io/cluster-api-addon-provider-helm/controllers/helmchartproxy/helmchartproxy_controller.go:188 +0x106c
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x2ce8638?, {0x2ce2120?, 0xc000ae4870?}, {{{0xc0000bd4a0?, 0xb?}, {0xc000a966c0?, 0x0?}}})
	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:119 +0xb7
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc0005f20a0, {0x2ce2158, 0xc000979c20}, {0x25e5020?, 0xc0007dafe0?})
	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:316 +0x3cc
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc0005f20a0, {0x2ce2158, 0xc000979c20})
	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266 +0x1af
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227 +0x79
created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 in goroutine 178
	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:223 +0x565

This is the yaml that is giving the issue.

$ k get hcp node-feature-discovery-docker-cluster-cilium-helm-addon5 -o yaml
apiVersion: addons.cluster.x-k8s.io/v1alpha1
kind: HelmChartProxy
metadata:
  creationTimestamp: "2024-06-03T14:52:58Z"
  finalizers:
  - helmchartproxy.addons.cluster.x-k8s.io
  generation: 1
  name: node-feature-discovery-docker-cluster-cilium-helm-addon5
  namespace: default
  ownerReferences:
  - apiVersion: cluster.x-k8s.io/v1beta1
    kind: Cluster
    name: docker-cluster-cilium-helm-addon5
    uid: ef487087-bf8a-44d1-8286-a05174cd8ecb
  resourceVersion: "2551"
  uid: 9bd0db75-96b2-43ed-b628-3460f60afae2
spec:
  chartName: node-feature-discovery
  clusterSelector:
    matchLabels:
      cluster.x-k8s.io/cluster-name: docker-cluster-cilium-helm-addon5
  namespace: node-feature-discovery
  options:
    enableClientCache: false
    install:
      createNamespace: true
    timeout: 10m0s
    upgrade:
      maxHistory: 10
  releaseName: node-feature-discovery
  repoURL: oci://mindthegap.default.svc/charts
  tlsConfig:
    insecureSkipTLSVerify: true

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

[mboersma]

@faiq thanks for contributing this! Do you think it's related to the bug just fixed in #252?

It's probably a good idea to add these nil checks regardless. Could you add a unit test case to helmreleaseproxy_controller_test.go that triggers this panic so we know this fixes it and CAAPH doesn't regress?

cluster-api-addon-provider-helm/controllers/helmreleaseproxy/helmreleaseproxy_controller_test.go

Line 320 in a95e8c5

func TestReconcileNormalWithCredentialRef(t *testing.T) {

[faiq]

@mboersma

I added a case in controllers/helmchartproxy/helmchartproxy_controller_phases_test.go that highlights the issue.

I tried to add a case in helmreleaseproxy_controller_test.go but I was not able to figure out how to do.

[faiq]

@mboersma i spoke too soon.

i also added another case for the helmrelease controller. both of these tests panic on main.

[mboersma]

/lgtm
/approve

Thanks @faiq!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: faiq, mboersma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment