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

fix: managed identity not being enabled for nodes #220

Merged

Conversation

comtalyst
Copy link
Collaborator

@comtalyst comtalyst commented Mar 19, 2024

Fixes #197

Description
We currently hardcode managed identity enablement for AKS node bootstrapping script to be false, which is incorrect as we need it to be true per our managed identity support. This PR is a quick fix for this. See the linked issue for a consequence of this bug.

How was this change tested?

  • Manually in NAP environment by assigning acrpull role to the kubelet identity and compare the results before and after the fix. This role assignment should be equivalent to attach-acr.

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

- Fix an issue where provisioned node(s) do not have permission to pull images from Azure Container Registry (ACR) for pods even after integrating with the cluster

@comtalyst comtalyst requested a review from tallaxes March 19, 2024 23:58
@coveralls
Copy link

coveralls commented Mar 20, 2024

Pull Request Test Coverage Report for Build 8352364991

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.716%

Totals Coverage Status
Change from base Build 8352358991: 0.0%
Covered Lines: 35591
Relevant Lines: 36423

💛 - Coveralls

Copy link
Collaborator

@tallaxes tallaxes left a comment

Choose a reason for hiding this comment

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

Let's also make sure to develop E2E test to cover this

@tallaxes tallaxes added kind/bug Categorizes issue or PR as related to a bug. area/nap Issues or PRs related to Node Auto Provisioning (NAP) area/security Issues or PRs related to security labels Mar 20, 2024
@comtalyst
Copy link
Collaborator Author

Let's also make sure to develop E2E test to cover this

#221 will address this

@comtalyst comtalyst merged commit 5beadbd into main Mar 20, 2024
9 checks passed
@comtalyst comtalyst deleted the comtalyst/fix-managed-identity-not-enabled-for-kubelet branch March 20, 2024 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/nap Issues or PRs related to Node Auto Provisioning (NAP) area/security Issues or PRs related to security kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACR image pull failed for nodes provisioned by node auto provisioner
3 participants