arttor / helmify

Creates Helm chart from Kubernetes yaml
MIT License
1.48k stars 136 forks source link

[serviceaccount] Annotations as Values Support #109

Closed onelapahead closed 1 year ago

onelapahead commented 1 year ago

I'd like to be able to include annotations on my operator's ServiceAccount as this is a fairly common need for a lot of apps using ServiceAccounts.

Felt like because ServiceAccounts might be special in this regard, I copied processer.ProcessObjMeta and modified it to include an annotations template and value.

Feels a bit hacky and open to feedback as to what the right approach should be.

onelapahead commented 1 year ago

Thanks for the steer @arttor, this PR should be good to go now in that it has the test_data and examples update you asked for.

arttor commented 1 year ago

@hfuss, I apologize for not bringing up this question earlier, but could you please explain the use case for the serviceAccount annotation and how it differs from other objects?

I've tried to do some research but there is not much information:

  1. k8s doc says that you can annotate a Secret to use it as api token for ServiceAccount
  2. azure-specific IAM annotations. AWS also uses annotation to map serviceAccount to IAM role.

There may be other annotation use cases beyond serviceAccount or IAM. Third-party Kubernetes operators rely on annotations for autoscaling and other tasks.

Unfortunately, Helm does not offer a way to add custom annotations to chart objects, and the chart maintainer may not be aware of which operators the chart users will use and which chart objects they will need to annotate. As a result, it is common to add annotations manually after running helm install because all annotations will be retained during the next helm upgrade.

onelapahead commented 1 year ago

Hey @arttor no worries, thanks for doing the initial research.

Specifically, my use case is I've written an operator which requires cloud access to either AWS or Azure. As you pointed out, being able to add IAM ServiceAccount annotations for those clouds simplifies how an operator / app needs to authenticate and assume roles.

Another use case for an app or an operator with a UI, might be wanting to create an OAuth client from a ServiceAccount within OpenShift: https://access.redhat.com/documentation/en-us/openshift_container_platform/4.3/html/authentication/using-service-accounts-as-oauth-client

In my use of open-source Helm charts such as External DNS, cert-manager, and External Secrets, it was very common for the chart maintainers to expose their ServiceAccount annotations for the same reason - allowing simpler cloud IAM authentication. Users of helmify writing cloud-aware operators might often want to offer the same IAM authentication methods for their charts

Your workaround to manually patch ServiceAccount annotations ourselves outside of Helm could introduce a lot of complexity if folks are trying to use Helm in a purely declarative / GitOps driven way such as ArgoCD or Flux.

arttor commented 1 year ago

Understood. What do you think about the following approach?

onelapahead commented 1 year ago

@arttor great feedback, took a stab at the functional option implementation as you described and ready for review.

Starefossen commented 1 year ago

Waiting for this 🙏🏻 This is need in order for service account to be able to link with Google Cloud Workload Identity.

kubectl annotate serviceaccount KSA_NAME \
    --namespace NAMESPACE \
    iam.gke.io/gcp-service-account=GSA_NAME@GSA_PROJECT.iam.gserviceaccount.com
Starefossen commented 1 year ago

Superseded by #115

arttor commented 1 year ago

closed with #115