Aad-pod-identity: Improve Helm package

[piizei]

Currently Helm package does not offer lot of value over just providing the deployment-rbac.yaml. It should provide the needed files to work as a Helm repository and function like normal Helm package. Similar to https://github.com/Azure/application-gateway-kubernetes-ingress/tree/master/helm

[ritazh]

Hi @piizei thanks for the feedback. Once you fill in the missing values for the Azure identity you want to use in the values.yaml here: https://github.com/Azure/aad-pod-identity/blob/master/charts/aad-pod-identity/values.yaml#L8-L17, the chart will deploy this https://github.com/Azure/aad-pod-identity/blob/master/charts/aad-pod-identity/templates/identities.yaml, which results in an AzureIdentity and an AzureIdentityBinding in your cluster. Have you seen issues with this?

[kkmsft]

@piizei - was wondering if you have some thoughts on Rita's reply. Do you think this is still an issue ?

[piizei]

Point here was that the Helm package should be installable without having to clone the repository. I'm using this package as downstream dependency of another helm package, which is now very complicated.
As opposite to the example link from application gateway, where the package can be installed just by following:
helm repo add application-gateway-kubernetes-ingress https://azure.github.io/application-gateway-kubernetes-ingress/helm/
helm repo update
helm install -f myconfig.yaml application-gateway-kubernetes-ingress/ingress-azure

HTH,
Petteri

[ritazh]

Thanks @piizei I will look into this.

[jamesrcounts]

@piizei Suggestion would also support easier deployments from terraform. I had originally opened #147, then thought you guys had already done something similar to ingress-azure (which is very easy to deploy from terraform). But today, found out we still need some help to make this component as easy to install as the ingress.

[torresdal]

Piggibacking on this with a few other improvement suggestions:

• When you make changes to the chart, like changing image versions, a good practice is to also change the chart version in Chart.yaml.
• Expose configuration for image name and tag (user should be able to override image name and tag if needed)
• The README should provide a table with available configuration values from values.yaml
• The templates/infra.yaml file should probably be split into more resources (files) for clarity

It's great that you've created a chart for this, but to be perfectly honest it currently could need some work to get up to the quality with other helm charts we're used to in the kubernetes community.

We are also about to deploy a open source project (https://github.com/SparebankenVest/azure-key-vault-to-kubernetes) for syncing Azure Key Vault secrets to Kubernetes that will have a optional downstream dependency on this Helm chart, so it will be important to us that this chart is properly versioned and maintained going forward. We also second @piizei's comment about publishing this Helm chart outside this repo.

[ritazh]

Thanks for the feedback everyone. Will look at this issue soon. Sorry for the delay!

[torresdal]

Update: we changed the URL of our helm repo from charts.spv.no to charts.spvapi.no (updated below).

In the meantime I've published the current version of this chart in our own public chart repo. Feel free to use that until this is fixed (https://github.com/SparebankenVest/public-helm-charts/tree/master/stable/aad-pod-identity):

helm repo add spv-charts https://charts.spvapi.no
helm repo update

helm install  spv-charts/aad-pod-identity --values valuesFile.yaml

Going forward I would urge this project to submit its chart to the official Helm repo: https://github.com/helm/charts

[aramase]

Closing with https://github.com/Azure/aad-pod-identity/pull/373