-
Notifications
You must be signed in to change notification settings - Fork 359
updating the RBAC +ABAC scenario of ACR #510
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
base: main
Are you sure you want to change the base?
updating the RBAC +ABAC scenario of ACR #510
Conversation
the acrpull wont work if container is configured for ABAC the identity should have the **Container Registry Repository Contributor** role
@sanjayananthamurthy : Thanks for your contribution! The author(s) and reviewer(s) have been notified to review your proposed change. |
Learn Build status updates of commit 18d03ea: 💡 Validation status: suggestions
articles/machine-learning/concept-endpoints-online-auth.md
For more details, please refer to the build report. Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them. |
Can you review the proposed changes? Note: The new alt text needs to be updated to match Microsoft standards. Important: When the changes are ready for publication, adding a #label:"aq-pr-triaged" |
#sign-off |
Invalid command: '#sign-off'. Only the assigned author of one or more file in this PR can sign off. @s-polly |
Learn Build status updates of commit 40380d4: ✅ Validation status: passed
For more details, please refer to the build report. |
Hi @sanjayananthamurthy - In the public repo, pull requests should be signed off by the author, another member of the content team, or a PM. @dem108 - Could you take a look? Thanks! |
> [!IMPORTANT] | ||
> If you configure your Container registry to use **[RBAC Registry + ABAC Repository Permissions](/azure/container-registry/container-registry-rbac-abac-repository-permissions?tabs=azure-portal)** | ||
> | ||
>  |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This image doesn't exist. Please replace it with a valid one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dem108 took this from https://learn.microsoft.com/en-us/azure/container-registry/media/container-registry-rbac-abac-repository-permissions/rbac-abac-repository-permissions-02-update-registry.png can you please help with the relative path for this image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sanjayananthamurthy - The image is in a different repository, and it would need to be moved to azure-ai-docs if you want to use it here. However, since you're already referencing the article that includes the image, you might want to consider omitting it here.
Co-authored-by: SeokJin Han <[email protected]>
Learn Build status updates of commit efb7e7f: ✅ Validation status: passed
For more details, please refer to the build report. |
This pull request has been inactive for at least 14 days. If you are finished with your changes, don't forget to sign off. See the contributor guide for instructions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the documentation for Azure Machine Learning online endpoints authentication to clarify role requirements when using RBAC + ABAC (Attribute-Based Access Control) permissions with Azure Container Registry.
Key changes:
- Added important notice about ABAC-enabled container registries
- Clarified that traditional ACR roles (AcrPull, AcrPush, AcrDelete) are not honored in ABAC-enabled registries
- Specified that Container Registry Repository Contributor role is required for endpoint identities
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
> If you configure your Container registry to use **[RBAC Registry + ABAC Repository Permissions](/azure/container-registry/container-registry-rbac-abac-repository-permissions?tabs=azure-portal)** | ||
> | ||
>  | ||
> | ||
>In this case, some existing role assignments aren't honored or will have different effects, because a different set of ACR built-in roles apply to ABAC-enabled registries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IMPORTANT callout block has an incomplete sentence structure. Line 119 starts with 'If you configure...' but doesn't have a complete conditional statement. Consider restructuring to either complete the conditional or change to a declarative statement like 'When you configure your Container registry to use..., the following considerations apply:'
> If you configure your Container registry to use **[RBAC Registry + ABAC Repository Permissions](/azure/container-registry/container-registry-rbac-abac-repository-permissions?tabs=azure-portal)** | |
> | |
>  | |
> | |
>In this case, some existing role assignments aren't honored or will have different effects, because a different set of ACR built-in roles apply to ABAC-enabled registries. | |
> When you configure your Container registry to use **[RBAC Registry + ABAC Repository Permissions](/azure/container-registry/container-registry-rbac-abac-repository-permissions?tabs=azure-portal)**, the following considerations apply: | |
> | |
>  | |
> | |
> In this case, some existing role assignments aren't honored or will have different effects, because a different set of ACR built-in roles apply to ABAC-enabled registries. |
Copilot uses AI. Check for mistakes.
> For example, the **AcrPull**, **AcrPush**, and **AcrDelete** roles aren't honored in an ABAC-enabled registry. | ||
> Instead, in ABAC-enabled registries, use the `Container Registry Repository Reader`, `Container Registry Repository Writer`, and `Container Registry Repository Contributor` roles to grant either registry-wide or repository-specific image permissions. | ||
> | ||
> Ensure that the SAI or the UAI of your endpoint has the **Container Registry Repository Contributor** role assigned on the Container registry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The role names should be consistently formatted. The three role names are enclosed in backticks while the role name in line 128 uses bold formatting. Consider using consistent formatting (either all backticks or all bold) for better readability.
> Ensure that the SAI or the UAI of your endpoint has the **Container Registry Repository Contributor** role assigned on the Container registry. | |
> Ensure that the SAI or the UAI of your endpoint has the `Container Registry Repository Contributor` role assigned on the Container registry. |
Copilot uses AI. Check for mistakes.
the acrpull wont work if container is configured for ABAC the identity should have the Container Registry Repository Contributor role