fix: issues with mindthegap deplotment

[faiq]

What problem does this PR solve?:

This adds trust-manager to allow us to use self hosted helm charts in mindthegap. See: kubernetes-sigs/cluster-api-addon-provider-helm#104

We can think about this in a couple of different parts:

1. Installing trust-manager and CRDs. We need to install trust manager so that we can create a certificate bundle with our self signed cert to caaph-controller-manager. We pull the CRDs separately because we are unable to use them in our templates because they aren't available at that time. See the comment in the script for more details.

2. Modifying CAAPH to use the bundle. This is done through a job it has a corresponding Service Account/Role/Role Binding.

3. Various little fixes. ie changing repository url and versions to using v

I tested this by creating a docker cluster.

The helm chart proxies use this value

- apiVersion: addons.cluster.x-k8s.io/v1alpha1
  kind: HelmChartProxy
  metadata:
    creationTimestamp: "2024-05-14T23:41:09Z"
    finalizers:
    - helmchartproxy.addons.cluster.x-k8s.io
    generation: 1
    name: node-feature-discovery-docker-cluster-cilium-helm-addon
    namespace: default
    ownerReferences:
    - apiVersion: cluster.x-k8s.io/v1beta1
      kind: Cluster
      name: docker-cluster-cilium-helm-addon
      uid: e0e943e4-6d63-4d85-8d37-f63160ff93e3
    resourceVersion: "3282"
    uid: e04b6b62-5f39-4528-87f7-5928d2afd4dd
  spec:
    chartName: node-feature-discovery
    clusterSelector:
      matchLabels:
        cluster.x-k8s.io/cluster-name: docker-cluster-cilium-helm-addon
    namespace: node-feature-discovery
    options:
      enableClientCache: false
      install:
        createNamespace: true
      timeout: 10m0s
      upgrade:
        maxHistory: 10
    releaseName: node-feature-discovery
    repoURL: oci://mindthegap.mindthegap.svc/charts

Which issue(s) this PR fixes:
Fixes #

How Has This Been Tested?:

Special notes for your reviewer:

[jimmidyson]

I'm not keen on trust-manager being a requirement for CAREN - shouldn't this be a user deployment configuration rather than a strict requirement? It could conflict with existing trust-manager deployment for example.

[faiq]

We can keep this functionality until we implement #497

We might be able to then set this behind a boolean gate. If that is the case, i'm not sure what the implication for konvoy would be. How should we template it?

On the other hand, CAREN is already a highly opinionated and has a ton of dependencies adding another isn't that big of a deal IMO

[dkoshkin]

Did you observe a cert error when testing? From CAAPH code it looks like its always setting the insecure flag.

[dkoshkin]

Is it not possible to get this from {{ .Release.Namespace }}? This won't work if helm install --namespace is set to anything other than caren-system right?

[faiq]

values.yaml doesn't use templates :( helm/helm#2492

[dkoshkin]

I ran make dev with this PR but removed the above trust-manager config and it seems to work correctly and already deploys the trust-manager chart into the correct namespace:

kubectl get pods -n caren-system
NAME                                                      READY   STATUS    RESTARTS   AGE
cluster-api-runtime-extensions-nutanix-7f5dc9ffdd-gxl8r   1/1     Running   0          65s
mindthegap-c57f655d5-k9sp4                                1/1     Running   0          88s
trust-manager-865cff7789-c2vc6                            1/1     Running   0          88s

The chart handles this correctly https://github.com/cert-manager/trust-manager/blob/2cb20f7d742d44a9eb3f2ebf62c415fffb34aa57/deploy/charts/trust-manager/templates/_helpers.tpl#L55

I think we're good to remove this and not hardcode the namespace?

[faiq]

the issue is with referencing the secret, where the pod was scheduled was never the issue

[faiq]

this comment shows why we need to set trust namespace cert-manager/trust-manager#120 (comment)

[dkoshkin]

Adding the namespace here directly makes it more difficult to reuse. See error message:

$ kubectl apply -f https://raw.githubusercontent.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/40853b83071e4804c5253ddf5981b77a1deb7b29/charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/aws-cluster-class.yaml -n kube-system

the namespace from the provided object "caren-system" does not match the namespace "kube-system". You must pass '--namespace=caren-system' to perform this operation.

[faiq]

this can be an issue down the line for sure 🤔

[dkoshkin]

This won't work if deployed in a different namespace?

[faiq]

that is correct. it needs to be deployed in that namespace, this is a limitation of trust-manager needing trust namespace set. this comment explains it

cert-manager/trust-manager#120 (comment)

if helm was able to use templating in values.yaml we could get around this

[jimmidyson]

I've pushed kubernetes-sigs/cluster-api-addon-provider-helm#238 to fix this issue in CAAPH without requiring trust-manager - I think this approach is very clean and easy to work with and as such feel we should rework this PR based on that approach.