Add support of storing the caBundle in a secret

[farcaller]

• For first time contributors, read Submitting a pull request
• All code is covered by unit and/or runtime tests where feasible.
• All commits contain a well written commit description including a title,
  description and a Fixes: #XXX line if the commit addresses a particular
  GitHub issue.
• If your commit description contains a Fixes: <commit-id> tag, then
  please add the commit author[s] as reviewer[s] to this issue.
• All commits are signed off. See the section Developer’s Certificate of Origin
• Provide a title or release-note blurb suitable for the release notes.
• Are you a user of Cilium? Please add yourself to the Users doc
• Thanks for contributing!

This allows to e.g. pull it from vault via external-secrets (as ES cannot write to a ConfigMap).

Allow to use a Secret for the caBundle

[sayboras]

There are a couple of CI failures, if you are taking a look at logs, there will be steps to rectify.

[sayboras]

👋 friendly ping if you have a chance to perform the above. Also, would you mind to rebase with main branch for latest CI configuration.

[farcaller]

@sayboras thanks for a ping! I got a bit distracted by my cilium setup breaking down in new and exciting ways but I'm back to this and I have a working solution that I just need to rewrite this nix into helm yamls: 😄

{
      patchCM = object:
        let
          volumes = object.spec.template.spec.volumes;
          projected = builtins.filter (v: builtins.hasAttr "projected" v) volumes;
          nonProjected = builtins.filter (v: builtins.hasAttr "projected" v == false) volumes;
          reprojected = builtins.map
            (p: lib.recursiveUpdate p (
              let
                caSources = builtins.filter (s: (s.configMap or { name = ""; }).name == "cilium-ca") p.projected.sources;
                nonCaSources = builtins.filter (s: (s.configMap or { name = ""; }).name != "cilium-ca") p.projected.sources;
                patchedSources = builtins.map
                  (s: {
                    secret = {
                      items = [{ key = "certificate"; path = (builtins.head s.configMap.items).path; }];
                      name = "cilium-ca";
                    };
                  })
                  caSources;
              in
              {
                projected.sources = nonCaSources ++ patchedSources;
              }
            ))
            projected;
        in
        (lib.recursiveUpdate object {
          spec.template.spec.volumes = nonProjected ++ reprojected;
        });

      patchOn = cond: object:
        if (cond object) then patchCM object
        else object;

      patchCiliumDS = patchOn (object: (object.kind == "DaemonSet" && object.metadata.name == "cilium"));
      patchApiServerDeployment = patchOn (object: (object.kind == "Deployment" && object.metadata.name == "clustermesh-apiserver"));
      patchHubbleRelayDeployment = patchOn (object: (object.kind == "Deployment" && object.metadata.name == "hubble-relay"));
      patchHubbleUIDeployment = patchOn (object: (object.kind == "Deployment" && object.metadata.name == "hubble-ui"));
}

[joamaki]

Heading on vacation so unassigned myself and picked @tommyp1ckles from sig-k8s by lottery (sort -R sig-k8s | head -1).

[joestringer]

cc @cilium/sig-hubble I think some of you folks have spent more time with Cilium's secret management previously and could potentially help advise on the solution here.

[gandro]

Also cc @giorio94 who added the CA bundle feature in #24862

To me, the overall motivation "this allows to e.g. pull it from vault via external-secrets (as ES cannot write to a ConfigMap)." sounds very reasonable. Since we have not shipped the CA bundle feature in a release yet, I'd even say let's always make it a secret if that makes the CA bundle easier to manage (and I'm not a big fan of having a resource either be a configmap or a secret based on a Helm value).

It was also the case that before we added the CA bundle, the CA was already part of a secret, so that should also not be a problem. But I think it would be interesting to know from @giorio94 if there is a particular reason why the CA bundle is a ConfigMap, or if the intent just was that "it's a public key, so it doesn't need to be a secret".

I'm marking this as a release blocker for now, since I think this is something that we can quite easily resolve now, where it will become harder to resolve once we ship the CA bundle feature in a stable release.

[giorio94]

It was also the case that before we added the CA bundle, the CA was already part of a secret, so that should also not be a problem. But I think it would be interesting to know from @giorio94 if there is a particular reason why the CA bundle is a ConfigMap, or if the intent just was that "it's a public key, so it doesn't need to be a secret".

No specific reason beside the fact that it is not a "secret" value as you mentioned, and to mimic the kubernetes behavior (specifically the kube-root-ca.crt ConfigMap, indeed also the default name was picket to look like that). But I don't personally have any objection to making that a secret if it simplifies the CABundle management process.

[gandro]

Please run make -C install/kubernetes, it generates the install/kubernetes/cilium/README.md which needs to be updated if you add or change Helm values.

[gandro]

Thanks @giorio94 for the quick response!

I did a quick look at trust-manager, and it seems to only support ConfigMaps unfortunately https://cert-manager.io/docs/projects/trust-manager/api-reference/#bundlespectarget

So maybe we do need to use flag that switches between ConfigMap and Secrets 😞

In that case I'll remove the release-blocker label again, since the change with useSecret can be done in a backwards compatible manner and I think this PR still needs a bit of iteration.

[giorio94]

I did a quick look at trust-manager, and it seems to only support ConfigMaps unfortunately https://cert-manager.io/docs/projects/trust-manager/api-reference/#bundlespectarget

Oh right, I actually forgot to double-check it. Thanks!

[sayboras]

/test

[sayboras]

The changes lgtm ✅

[kaworu]

Thanks for the patch @farcaller!

Couple of cosmetic comments, but other than that LGTM.

[kaworu]

Please squash all the PR's commits into one on the way!

[ti-mo]

One more: https://github.com/cilium/cilium/actions/runs/5354804806/jobs/9712313069?pr=25728

-     - Use a Secret instead of a ConfigMap
+     - Use a Secret instead of a ConfigMap.

[farcaller]

One more

Done (thought that one was supposed to be re-generated from make -C install/kubernetes)

[gandro]

/test

[qmonnet]

“docs” change looks good. Thanks!