OCPBUGS#11410: Add required Azure AD permission by xenolinux · Pull Request #65911 · openshift/openshift-docs

xenolinux · 2023-10-07T06:42:11Z

Version(s): 4.10+

Issue: https://issues.redhat.com/browse/OCPBUGS-11410

Link to docs preview: https://65911--docspreview.netlify.app/openshift-enterprise/latest/installing/installing_azure/installing-azure-account#installation-azure-permissions_installing-azure-account

QE review:

QE has approved this change.

Additional information: The "Required Azure roles" section or modules/installation-azure-permissions.adoc file will not be available in the preview. The file is currently not present in 4.14 and main. But it is being used from 4.10 - 4.13. Once this PR is merged, I will create a manual cherry-pick for 4.14.

ocpdocs-previewbot · 2023-10-07T06:52:33Z

🤖 Updated build preview is available at:
https://65911--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/28292

jinyunma · 2023-10-08T01:15:50Z

 * `Contributor`

-To set roles on the Azure portal, see the link:https://docs.microsoft.com/en-us/azure/role-based-access-control/role-assignments-portal[Manage access to Azure resources using RBAC and the Azure portal] in the Azure documentation.
+Your Active Directory (AD) must have the following permission:


How about to change Your Azure account on Azure Active Directory (AD) must have the following permission:?

jinyunma · 2023-10-08T01:17:26Z

From docs review link:

Link to docs preview: https://65911--docspreview.netlify.app/openshift-enterprise/latest/installing/installing_azure/installing-azure-account#installation-azure-permissions_installing-azure-account

seems that the section of Required Azure roles is deleted?

xenolinux · 2023-10-09T08:59:50Z

@mjpytlak I think the modules/installation-azure-permissions.adoc file was part of refactoring. I have made changes in modules/installation-creating-azure-service-principal.adoc. But I was wondering whether or not update the modules/installation-azure-permissions.adoc file. Can you take a look at this change, if it looks good?

mjpytlak · 2023-10-09T16:51:44Z

I think the modules/installation-azure-permissions.adoc file was part of refactoring. I have made changes in modules/installation-creating-azure-service-principal.adoc. But I was wondering whether or not update the modules/installation-azure-permissions.adoc file. Can you take a look at this change, if it looks good?

@xenolinux I do not recall modules/installation-azure-permissions.adoc being part of a refactor. Can you be more specific? The only recent refactor I can recall was around the installation configuration parameters module.

xenolinux · 2023-10-10T07:33:56Z

I think the modules/installation-azure-permissions.adoc file was part of refactoring. I have made changes in modules/installation-creating-azure-service-principal.adoc. But I was wondering whether or not update the modules/installation-azure-permissions.adoc file. Can you take a look at this change, if it looks good?

@xenolinux I do not recall modules/installation-azure-permissions.adoc being part of a refactor. Can you be more specific? The only recent refactor I can recall was around the installation configuration parameters module.

okay. I was looking in this PR #62875, here

mjpytlak · 2023-10-10T13:34:17Z

@xenolinux modules/installation-azure-permissions.adoc is no longer used in the doc set, so I suggest reverting your changes to the module. My PR (62875) captures the changes you made:

https://github.com/openshift/openshift-docs/pull/62875/files#diff-c82de66f7c129811e1de419d2c8b04c6cf8429d036db2acffc36f43e606fdcbaR9 addresses the need for the permissions to be granted to the Azure subscription.
https://github.com/openshift/openshift-docs/pull/62875/files#diff-c82de66f7c129811e1de419d2c8b04c6cf8429d036db2acffc36f43e606fdcbaR9 addresses the link to for assigning roles using the Azure portal.

My PR continues to use modules/installation-creating-azure-service-principal.adoc, which will pick up your change about needing the microsoft.directory/servicePrincipals/createAsOwner permission.

xenolinux · 2023-10-11T08:56:36Z

@xenolinux modules/installation-azure-permissions.adoc is no longer used in the doc set, so I suggest reverting your changes to the module. My PR (62875) captures the changes you made:
* https://github.com/openshift/openshift-docs/pull/62875/files#diff-c82de66f7c129811e1de419d2c8b04c6cf8429d036db2acffc36f43e606fdcbaR9 addresses the need for the permissions to be granted to the Azure subscription.

* https://github.com/openshift/openshift-docs/pull/62875/files#diff-c82de66f7c129811e1de419d2c8b04c6cf8429d036db2acffc36f43e606fdcbaR9 addresses the link to for assigning roles using the Azure portal.
My PR continues to use modules/installation-creating-azure-service-principal.adoc, which will pick up your change about needing the microsoft.directory/servicePrincipals/createAsOwner permission.

I reverted the changes. Thanks @mjpytlak for the clarification.

xenolinux · 2023-10-11T09:01:22Z

@jinyunma Since there was a difficulty in creating a service principal due to lack of privileges. The current addition in the modules/installation-creating-azure-service-principal.adoc file will be sufficient right?

jinyunma · 2023-10-11T10:07:12Z

@jinyunma Since there was a difficulty in creating a service principal due to lack of privileges. The current addition in the modules/installation-creating-azure-service-principal.adoc file will be sufficient right?

I still could not find the section Required Azure roles in doc link https://65911--docspreview.netlify.app/openshift-enterprise/latest/installing/installing_azure/installing-azure-account#installation-azure-permissions_installing-azure-account.

hmm, I think it's better to add into that permission in this missed section Required Azure roles, which lists all role/permissions that Azure User Account requires.

xenolinux · 2023-10-11T10:15:40Z

@jinyunma Since there was a difficulty in creating a service principal due to lack of privileges. The current addition in the modules/installation-creating-azure-service-principal.adoc file will be sufficient right?

I still could not find the section Required Azure roles in doc link https://65911--docspreview.netlify.app/openshift-enterprise/latest/installing/installing_azure/installing-azure-account#installation-azure-permissions_installing-azure-account.

hmm, I think it's better to add into that permission in this missed section Required Azure roles, which lists all role/permissions that Azure User Account requires.

@jinyunma We are not using the "Azure required roles" section related file anymore. Instead we are using the modules/installation-creating-azure-service-principal.adoc file. Hence, as Mike suggested I reverted the changes from the "Azure required roles" section.

jinyunma · 2023-10-11T10:26:31Z

@jinyunma We are not using the "Azure required roles" section related file anymore. Instead we are using the modules/installation-creating-azure-service-principal.adoc file. Hence, as Mike suggested I reverted the changes from the "Azure required roles" section.

If so, I think below content is missing in modules/installation-creating-azure-service-principal.adoc file, because those roles are required on Azure User account.

OpenShift Container Platform needs a service principal so it can manage Microsoft Azure resources. Before you can create a service principal, your Azure account subscription must have the following roles:

    User Access Administrator

    Contributor

xenolinux · 2023-10-11T10:29:09Z

@jinyunma We are not using the "Azure required roles" section related file anymore. Instead we are using the modules/installation-creating-azure-service-principal.adoc file. Hence, as Mike suggested I reverted the changes from the "Azure required roles" section.

If so, I think below content is missing in modules/installation-creating-azure-service-principal.adoc file, because those roles are required on Azure User account.
OpenShift Container Platform needs a service principal so it can manage Microsoft Azure resources. Before you can create a service principal, your Azure account subscription must have the following roles:

    User Access Administrator

    Contributor

Those roles are added as a prerequisite:
* If you are not going to assign the ContributorandUser Administrator Accessroles to the service principal, you have created a custom role with the required Azure permissions.

jinyunma · 2023-10-11T10:35:11Z

Those roles are added as a prerequisite: * If you are not going to assign the ContributorandUser Administrator Accessroles to the service principal, you have created a custom role with the required Azure permissions.

Here mainly describes roles required for service principal which will be created by azure user account.

But in "Azure required roles" section, it lists the roles which azure user account requires.

xenolinux · 2023-10-11T11:06:37Z

@mjpytlak Sorry, I am missing something. Have we shifted this information[1] somewhere else or is being communicated in other way?
cc: @jinyunma

[1] The Azure account requires the following roles:

User Access Administrator
Contributor

mjpytlak · 2023-10-11T13:42:10Z

Sorry, I am missing something. Have we shifted this information[1] somewhere else or is being communicated in other way?

@xenolinux @jinyunma Yes my 4.14 PR [1] has this information:

[1] #62875

xenolinux · 2023-10-11T14:05:48Z

@jinyunma I think this change[1] will suffice, what do you think?

[1] https://github.com/openshift/openshift-docs/pull/65911/files#diff-121a73dacb3a5821e84ce668704e00f06cb68a1143f7fad6baee8c38a43ab983R18

jinyunma · 2023-10-12T01:31:46Z

@mjpytlak @xenolinux sorry that didn't notice that installation-azure-permissions.adoc is removed from PR #62875 when reviewing it, which removed privileges required for user account on subscription.

For normal installation:

Azure account need to have roles "Contributor" and "User Access Administrator" on subscription, and permission "microsoft.directory/servicePrincipals/createAsOwner" on Azure AD.
Azure account create service principal or azure vm with managed identity
then assign system role "Contributor" and "User Access Administrator" for service principal or managed identity on subscription level.
continue the installation.

But from doc link in comments, I didn't find any description for azure account subscription roles in step1 which I described above, or I misunderstand something?

mjpytlak · 2023-10-12T13:55:15Z

But from doc link in comments, I didn't find any description for azure account subscription roles in step1 which I described above, or I misunderstand something?

@jinyunma I think you are missing something and we might be missing something. Also - We are now starting to have conversations about two separate PRs, which is making it difficult to close on this PR -- this is my fault, as I initiated the conversation by referencing my work. I would first like to understand what is needed to get this PR approved.

For this PR

While "Required Azure roles" does not appear in the 4.14 docs, it is still used in 4.10 up to and including 4.13, so I was mistaken in asking @xenolinux to remove it. To get approval on this PR, I believe the only thing you are asking for is that the "microsoft.directory/servicePrincipals/createAsOwner" permission on Azure AD be added to to this topic. Correct?

From PR #62875.

Answering your last set of questions about my 4.14 work on managed identities, but I would ask that we continue this conversation in a Slack DM to determine if an update is required for 4.14.

Required Azure permissions for installer-provisioned infrastructure clearly states that 1 option is to apply the "Contributor" and "User Access Administrator" role to grant the required permissions needed to install, and that "These permissions must be granted to the Azure subscription that is associated with the identity." This is only one option, in that assigning those roles might not work for organizations with more restrictive security policies.

Here is the currently published 4.13 docs with Required Azure roles (installation-azure-permissions.adoc). Note that the Required Azure permissions for installer-provisioned infrastructure (from my 4.14 PR) says the exact same thing as Required Azure roles. I simply moved the content to "Required Azure permissions for installer-provisioned infrastructure" to clarify that assigning these roles is the quickest way to get all of the permissions assigned.

Given that I am continuing to use file that is being updated here by this PR, I believe I should be OK, unless you would also like to see the "microsoft.directory/servicePrincipals/createAsOwner" permission on Azure AD explicitly referenced in the Required Azure permissions content.

abhatt-rh · 2023-10-17T15:01:58Z

Confirmed that the author has received an approval from the OCP DPM for merging to 4.10

abhatt-rh · 2023-10-17T15:21:49Z

/cherrypick enterprise-4.14

abhatt-rh · 2023-10-17T15:21:58Z

/cherrypick enterprise-4.13

abhatt-rh · 2023-10-17T15:22:04Z

/cherrypick enterprise-4.12

abhatt-rh · 2023-10-17T15:22:11Z

/cherrypick enterprise-4.11

abhatt-rh · 2023-10-17T15:22:20Z

/cherrypick enterprise-4.10

gabriel-rh · 2023-10-17T16:10:38Z

/cherrypick enterprise-4.14

openshift-cherrypick-robot · 2023-10-17T16:11:26Z

@gabriel-rh: new pull request created: #66385

Details

In response to this:

/cherrypick enterprise-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

abhatt-rh · 2023-10-17T16:13:39Z

/cherrypick enterprise-4.13

openshift-cherrypick-robot · 2023-10-17T16:14:24Z

@abhatt-rh: #65911 failed to apply on top of branch "enterprise-4.13":

Applying: OCPBUGS#11410: Add required Azure AD permission
Using index info to reconstruct a base tree...
M	modules/installation-azure-permissions.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-azure-permissions.adoc
CONFLICT (content): Merge conflict in modules/installation-azure-permissions.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OCPBUGS#11410: Add required Azure AD permission
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

abhatt-rh · 2023-10-17T16:15:25Z

/cherrypick enterprise-4.12

openshift-cherrypick-robot · 2023-10-17T16:16:10Z

@abhatt-rh: #65911 failed to apply on top of branch "enterprise-4.12":

Applying: OCPBUGS#11410: Add required Azure AD permission
Using index info to reconstruct a base tree...
M	modules/installation-azure-permissions.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-azure-permissions.adoc
CONFLICT (content): Merge conflict in modules/installation-azure-permissions.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OCPBUGS#11410: Add required Azure AD permission
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

abhatt-rh · 2023-10-17T16:16:24Z

/cherrypick enterprise-4.11

abhatt-rh · 2023-10-17T16:16:49Z

/cherrypick enterprise-4.10

openshift-cherrypick-robot · 2023-10-17T16:17:07Z

@abhatt-rh: #65911 failed to apply on top of branch "enterprise-4.11":

Applying: OCPBUGS#11410: Add required Azure AD permission
Using index info to reconstruct a base tree...
M	modules/installation-azure-permissions.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-azure-permissions.adoc
CONFLICT (content): Merge conflict in modules/installation-azure-permissions.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OCPBUGS#11410: Add required Azure AD permission
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2023-10-17T16:17:32Z

@abhatt-rh: #65911 failed to apply on top of branch "enterprise-4.10":

Applying: OCPBUGS#11410: Add required Azure AD permission
Using index info to reconstruct a base tree...
M	modules/installation-azure-permissions.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-azure-permissions.adoc
CONFLICT (content): Merge conflict in modules/installation-azure-permissions.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OCPBUGS#11410: Add required Azure AD permission
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 7, 2023

xenolinux force-pushed the azure-dir-permission branch 2 times, most recently from 37643c2 to f8a6cf9 Compare October 7, 2023 07:20

jinyunma reviewed Oct 8, 2023

View reviewed changes

xenolinux force-pushed the azure-dir-permission branch 3 times, most recently from c5401be to 73db305 Compare October 9, 2023 07:51

xenolinux force-pushed the azure-dir-permission branch from 73db305 to 7f52251 Compare October 11, 2023 08:35

openshift-ci Bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 11, 2023

xenolinux force-pushed the azure-dir-permission branch 2 times, most recently from 3304185 to 158b811 Compare October 11, 2023 08:37

abhatt-rh added branch/enterprise-4.11 branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 labels Oct 17, 2023

abhatt-rh added this to the Continuous Release milestone Oct 17, 2023

abhatt-rh added the branch/enterprise-4.10 label Oct 17, 2023

abhatt-rh merged commit 4e35358 into openshift:main Oct 17, 2023

openshift-cherrypick-robot mentioned this pull request Oct 17, 2023

[enterprise-4.14] OCPBUGS#11410: Add required Azure AD permission #66385

Merged

abhatt-rh removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Oct 18, 2023

Conversation

xenolinux commented Oct 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ocpdocs-previewbot commented Oct 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jinyunma Oct 8, 2023

Choose a reason for hiding this comment

Uh oh!

jinyunma commented Oct 8, 2023

Uh oh!

xenolinux commented Oct 9, 2023

Uh oh!

mjpytlak commented Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xenolinux commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjpytlak commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xenolinux commented Oct 11, 2023

Uh oh!

xenolinux commented Oct 11, 2023

Uh oh!

jinyunma commented Oct 11, 2023

Uh oh!

xenolinux commented Oct 11, 2023

Uh oh!

jinyunma commented Oct 11, 2023

Uh oh!

xenolinux commented Oct 11, 2023

Uh oh!

jinyunma commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xenolinux commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjpytlak commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xenolinux commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jinyunma commented Oct 12, 2023

Uh oh!

mjpytlak commented Oct 12, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

gabriel-rh commented Oct 17, 2023

Uh oh!

openshift-cherrypick-robot commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

openshift-cherrypick-robot commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

openshift-cherrypick-robot commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

abhatt-rh commented Oct 17, 2023

Uh oh!

xenolinux commented Oct 7, 2023 •

edited

Loading

ocpdocs-previewbot commented Oct 7, 2023 •

edited

Loading

mjpytlak commented Oct 9, 2023 •

edited

Loading

xenolinux commented Oct 10, 2023 •

edited

Loading

mjpytlak commented Oct 10, 2023 •

edited

Loading

jinyunma commented Oct 11, 2023 •

edited

Loading

xenolinux commented Oct 11, 2023 •

edited

Loading

mjpytlak commented Oct 11, 2023 •

edited

Loading

xenolinux commented Oct 11, 2023 •

edited

Loading

abhatt-rh commented Oct 17, 2023 •

edited

Loading