spiffe / helm-charts

Helm charts for SPIRE and other SPIFFE components.
Apache License 2.0
20 stars 22 forks source link

ClusterSPIFFEID CR is included when CRD is found in the chart #427

Open drewwells opened 1 year ago

drewwells commented 1 year ago

If you're doing a fresh install of spire, it attempts to install both a CRD and CR. This is not supported in our CICD workflow (or a good practice). Other projects that require this do so in multiple steps like defining a separate CRD chart or a separate CR chart. I solve for this by installing a separate CR chart.

Repo steps:

helm template --namespace spire-server --release-name spire --is-upgrade spiffe/spire --version v0.10.1 --validate
Error: unable to build kubernetes objects from release manifest: resource mapping not found for name: "spire-controller-manager-service-account-based" namespace: "spire-server" from "": no matches for kind "ClusterSPIFFEID" in version "spire.spiffe.io/v1alpha1"
ensure CRDs are installed first
kfox1111 commented 1 year ago

I usually helm install with the skip crds flag, and then load it from raw yaml. Would that work?

drewwells commented 1 year ago

Unfortunately, our CICD system would not be able to do this. We need a helm install --only-crds so helm template --validate does not fail. We normally break out CRDs into a separate chart like cert-manager does. I think the best option is to have federation CRs as examples or a separate chart. Right now I disable the CRs with spire-server.controllerManager.identities.enabled=false then install them in a separate chart internally.

kfox1111 commented 1 year ago

This is related to https://github.com/spiffe/helm-charts/issues/411 as well.

faisal-memon commented 1 year ago

@drewwells thanks for bringing this up. We definitely need to revisit how controller manager is integrated. Im not super familiiar with how cert manager works. Would that model work here?

kfox1111 commented 1 year ago

No, its a CRD + CR issue not a Certificate issue. CRD management with helm has always been... problematic.

Some of the gory details are outlined in hip 11. https://github.com/helm/community/blob/main/hips/hip-0011.md

drewwells commented 1 year ago

In layman's, you can read this https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-2-separate-charts

kfox1111 commented 1 year ago

Hmm... If we moved it to a separate chart, we could conditionally embed it, and have it standalone. Then folks could use it either way.

faisal-memon commented 1 year ago

@drewwells Plan we came up with is to create a sub chart with the crd and convert it to a template. Then eventually move it to a separate root level chart. Would appreciate your thoughts on this plan.

drewwells commented 1 year ago

As long as we can install it as if it were a chart with only crds in it, then that will work fine.

kfox1111 commented 1 year ago

Thats the plan.