Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update aad-pod-identity helm chart dependency to 4.1.1 #1583

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

babbageclunk
Copy link
Member

Closes #1364

What this PR does / why we need it:
Updates the aad-pod-identity dependency in our helm chart to 4.1.1. This corresponds to aad-pod-identity version 1.8.0. It's needed because the previous version 1.5.5 installs v1beta1 ClusterRoleBindings, which trigger deprecation warnings in k8s 1.19.11.

Unfortunately this isn't the only thing that causes deprecation warnings in 1.19.11. Updating the version of cert-manager we install in CI and point to in docs is easy. We also need to change the ValidatingWebhookConfigurations to v1 (which is blocked by controller-runtime at the moment)

Special notes for your reviewer:
To avoid picking up changes to CRDs after the helm chart was released (which won't be handled by the current 1.0.24492 operator image) I made the dependency changes and ran the make target against the last helm chart update commit d28ea4b, then created the commit on master. Maybe we should be tagging each chart commit with the chart version as well as tagging the release commits?

How does this PR make you feel:
gif

@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2021

Codecov Report

Merging #1583 (a5a042b) into master (c2e7189) will not change coverage.
The diff coverage is n/a.

❗ Current head a5a042b differs from pull request most recent head 92febf6. Consider uploading reports for the commit 92febf6 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1583   +/-   ##
=======================================
  Coverage   63.24%   63.24%           
=======================================
  Files         183      183           
  Lines       11914    11914           
=======================================
  Hits         7535     7535           
  Misses       3706     3706           
  Partials      673      673           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2e7189...92febf6. Read the comment docs.

appVersion: 1.0.24492
description: Deploy components and dependencies of azure-service-operator
home: https://github.com/Azure/azure-service-operator
sources:
- https://github.com/Azure/azure-service-operator
dependencies:
- name: aad-pod-identity
version: 1.5.5
version: 4.1.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a massive version bump!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gave me a fright at first as well but it's not quite as bad as it sounds. The aad-pod-identity chart version used to be in lock-step with the underlying package version but they floated it at some point - the actual aad-pod-identity version is going from 1.5.5 to 1.8.0.

Copy link
Member

@matthchr matthchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to fix the content of charts/azure-service-operator/values.yaml because AAD Pod Identity change the format to specify an identity from what we're using there to a new format. I filed a bug to them about difficulty doing that here, which they resolved a while back.

@babbageclunk babbageclunk force-pushed the update-aad-pod-identity-dep branch 2 times, most recently from 0cb2700 to fd00039 Compare June 22, 2021 03:22
@babbageclunk babbageclunk force-pushed the update-aad-pod-identity-dep branch 15 times, most recently from 4629294 to 5386c78 Compare June 27, 2021 23:01
This corresponds to aad-pod-identity version 1.8.0. It's needed
because the previous version 1.5.5 installs v1beta1
ClusterRoleBindings, which trigger deprecation warnings in k8s
1.19.11.
@babbageclunk babbageclunk force-pushed the update-aad-pod-identity-dep branch from 5386c78 to b482f05 Compare June 28, 2021 00:17
@babbageclunk babbageclunk merged commit 76d4bf8 into Azure:master Jun 28, 2021
@babbageclunk babbageclunk deleted the update-aad-pod-identity-dep branch June 28, 2021 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: AAD pod-identity failing to get identity
4 participants