Make aad-pod-identity install optional when using Managed Identity. #1236
Conversation
kyschouv
requested review from
davefellows,
frodopwns,
jananivMS,
matthchr,
Porges,
theunrepentantgeek and
WilliamMortlMicrosoft
as code owners
|
|
charts/index.yaml
Outdated
| version: 0.1.0 | ||
| generated: "2020-07-08T10:09:00.877634-06:00" | ||
| - azure-service-operator-0.1.1.tgz | ||
| version: 0.1.1 |
There was a problem hiding this comment.
The CI/CD around the helm chart is a bit simple right now. It doesn't do anything with versioning. As such, the tar being used in the pipeline has the 10.1.0` version hardcoded.
If we want to adhere to semver in the helm chart we will need to ensure that our current usage of MCR allows storage of multiple versions. Also we need to decide if it's worth the trouble. Helm won't currently allow us to update or delete CRDs so the value proposition is dubious.
There was a problem hiding this comment.
Ah, sorry. I'm used to the stable repo's CI/CD where they lint the chart and reject it without a version bump. Want me to revert the version number and repack the chart?
|
Hey @kyschouv - I'll look through this PR and talk with @frodopwns about this. Sorry about the delay, we've gone through a bit of a maintainer shakeup and are still figuring out some new processes as new and tooling which has caused us to be slow on reviewing PRs. |
|
@kyschouv - I pushed a merge of master with your branch so the diff was cleaner (just FYI) |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
What this PR does / why we need it:
When using a Managed Identity for azure-service-operator, the chart currently automatically installs aad-pod-identity. However, a user may already have aad-pod-identity installed on their cluster, or may wish to manage the install of aad-pod-identity separately from azure-service-operator. This change to the helm chart adds a separate
installAadPodIdentityflag, which is used to control the installation of aad-pod-identity, and leaves theazureUseMIflag to enable the AzureIdentity binding.Special notes for your reviewer:
It would be great if the
aadpodidbindingin the deployment was configurable as well (so the user could deploy with whatever label they'd like), but the template that includes that looks to be auto-generated, and so I left it outside the scope of this change. This change will also unblock my current integration, so I didn't want to delay it.refs #1123