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

support cluster credentials api #360

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

smiklos
Copy link

@smiklos smiklos commented Aug 5, 2024

The accessProfile API is going to be deprecated not to mention that there's no out of the box RBAC role that permits this API without adding Admin access to the cluster/namespace.

The newer API differentiates between admin and user credentials and is covered by all built in AKS RBAC roles.

HelmDeployV0 and HelmDeployV1 should be migrated to use this newer API to simplify working with AKS.

@smiklos smiklos requested review from manolerazvan and a team as code owners August 5, 2024 19:47
@smiklos
Copy link
Author

smiklos commented Aug 5, 2024 via email

@smiklos
Copy link
Author

smiklos commented Aug 7, 2024

Added some tests but as far as I can tell none of the L0-* tests are running under azure-arm-rest package by default.

@smiklos
Copy link
Author

smiklos commented Aug 8, 2024

@manolerazvan Could please review this?

@smiklos
Copy link
Author

smiklos commented Aug 9, 2024

@starkmsu, @qianz2, @DergachevE any chance you could take a look? this Is related to microsoft/azure-pipelines-tasks#16614

I'm trying to add support for the new APIs without removing the old one for now. If this gets merge we can change HElmDeploy and other tasks to use the newer APIs

Copy link
Contributor

@YevheniiKholodkov YevheniiKholodkov left a comment

Choose a reason for hiding this comment

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

Looks good to me. Approved.

@smiklos
Copy link
Author

smiklos commented Aug 22, 2024

Thanks for the review! Are we not concerned that the tests are not running for this package?

@manolerazvan
Copy link

We are trying to fix that part also #here

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.

4 participants