OCPBUGS-4549: azure: replace deprecated AD Graph API #6614

r4f4 · 2022-11-22T22:25:57Z

Use the new MS Graph API instead.

Please note that Applications.ReadWrite.OwnedBy permissions must be granted in Azure for MSGraph.

https://issues.redhat.com/browse/CORS-1897

r4f4 · 2022-11-22T22:29:19Z

Depends on #6003

This is an alternative to #5714. In this PR, only the new MSGraph calls are tried with no fallback to the deprecated AD Graph APIs (which are removed here).

r4f4 · 2022-11-22T22:30:41Z

/hold

r4f4 · 2022-11-22T23:05:54Z

Error strings from msgraph sdk are not very helpful. They just say error status code received from the API. So we might want to unwrap the ODataError before logging it.

microsoftgraph/msgraph-sdk-go#218

patrickdillon · 2022-11-30T21:15:17Z

/retest
/approve
LGTM but let's see what the tests say and obviously we need to wait for #6003

But I think we should do this over #5714

openshift-ci · 2022-11-30T21:21:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [patrickdillon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

r4f4 · 2022-12-06T14:14:54Z

/hold cancel

r4f4 · 2022-12-06T14:16:38Z

/retitle OCPBUGS-4549: azure: replace deprecated AD Graph API

r4f4 · 2022-12-06T14:37:20Z

/retitle OCPBUGS-4549: azure: replace deprecated AD Graph API

openshift-ci-robot · 2022-12-06T14:37:29Z

@r4f4: This pull request references Jira Issue OCPBUGS-4549, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Use the new MS Graph API instead.

Please note that Applications.ReadWrite.OwnedBy permissions must be granted in Azure for MSGraph.

azure-deprecation/dashboard#60

https://issues.redhat.com/browse/CORS-1897

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.

r4f4 · 2022-12-06T14:41:56Z

/test golint

patrickdillon · 2022-12-07T20:44:39Z

ug, was coming to LGTM but we already hit another conflict :(

Use the new MS Graph API instead. Please note that `Applications.ReadWrite.OwnedBy` permissions must be granted in Azure for MSGraph. azure-deprecation/dashboard#60 https://issues.redhat.com/browse/CORS-1897

Errors returned by the new Azure SDK are not very helpful. So before returning any error back to the user, we try to extract a more friendly "code: message" format from them. Please note that we do that at the very end so that unwraping and checking for other non-ODataError types is still possible during the function call chain. microsoftgraph/msgraph-sdk-go#218

patrickdillon · 2022-12-07T21:50:58Z

/lgtm

openshift-ci-robot · 2022-12-08T08:19:30Z

/retest-required

Remaining retests: 0 against base HEAD 01adff5 and 1 for PR HEAD 20a288b in total

openshift-ci-robot · 2022-12-08T15:39:30Z

/retest-required

Remaining retests: 0 against base HEAD 254a2b7 and 0 for PR HEAD 20a288b in total

openshift-ci-robot · 2022-12-08T16:42:15Z

/hold

Revision 20a288b was retested 3 times: holding

r4f4 · 2022-12-09T10:00:47Z

/override ci/prow/golint
It's timing out. It should be better when #6601 merges. I ran it locally and there were no problems reported.

openshift-ci · 2022-12-09T10:01:02Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/golint

Details

In response to this:

/override ci/prow/golint
It's timing out. It should be better when #6601 merges. I ran it locally and there were no problems reported.

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.

r4f4 · 2022-12-09T10:01:49Z

/hold cancel

openshift-ci-robot · 2022-12-09T14:38:34Z

/retest-required

Remaining retests: 0 against base HEAD 40cf6fe and 2 for PR HEAD 20a288b in total

openshift-ci-robot · 2022-12-09T18:38:25Z

/retest-required

Remaining retests: 0 against base HEAD 6de3bb4 and 1 for PR HEAD 20a288b in total

r4f4 · 2022-12-09T18:39:51Z

/override ci/prow/e2e-gcp-ovn
Not affected by the changes in this PR

openshift-ci · 2022-12-09T18:40:42Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-gcp-ovn

Details

In response to this:

/override ci/prow/e2e-gcp-ovn
Not affected by the changes in this PR

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.

patrickdillon · 2022-12-09T20:40:58Z

/skip

r4f4 · 2022-12-09T22:24:19Z

/override ci/prow/e2e-gcp-ovn
Not affected by the PR

openshift-ci · 2022-12-09T22:24:46Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-gcp-ovn

Details

In response to this:

/override ci/prow/e2e-gcp-ovn
Not affected by the PR

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-robot · 2022-12-10T02:38:27Z

/retest-required

Remaining retests: 0 against base HEAD 3c693eb and 0 for PR HEAD 20a288b in total

openshift-ci-robot · 2022-12-10T03:38:27Z

/hold

Revision 20a288b was retested 3 times: holding

r4f4 · 2022-12-10T09:28:32Z

/override ci/prow/golint
/hold cancel

openshift-ci · 2022-12-10T09:29:02Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/golint

Details

In response to this:

/override ci/prow/golint
/hold cancel

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-robot · 2022-12-10T09:38:17Z

/retest-required

Remaining retests: 0 against base HEAD 3c693eb and 2 for PR HEAD 20a288b in total

r4f4 · 2022-12-10T18:15:37Z

/override ci/prow/e2e-gcp-ovn
Not affected and timing out

openshift-ci · 2022-12-10T18:16:07Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-gcp-ovn

Details

In response to this:

/override ci/prow/e2e-gcp-ovn
Not affected and timing out

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 · 2022-12-10T18:16:08Z

@r4f4: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-azurestack	`20a288b`	link	false	`/test e2e-azurestack`
ci/prow/e2e-aws-ovn-disruptive	`20a288b`	link	false	`/test e2e-aws-ovn-disruptive`
ci/prow/okd-scos-e2e-aws-ovn	`20a288b`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/okd-e2e-aws-ovn-upgrade	`20a288b`	link	false	`/test okd-e2e-aws-ovn-upgrade`
ci/prow/e2e-libvirt	`20a288b`	link	false	`/test e2e-libvirt`
ci/prow/e2e-azure-ovn-shared-vpc	`20a288b`	link	false	`/test e2e-azure-ovn-shared-vpc`
ci/prow/golint	`20a288b`	link	true	`/test golint`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-ci-robot · 2022-12-10T18:20:39Z

@r4f4: All pull requests linked via external trackers have merged:

openshift/installer#6614

Jira Issue OCPBUGS-4549 has been moved to the MODIFIED state.

Details

In response to this:

Use the new MS Graph API instead.

Please note that Applications.ReadWrite.OwnedBy permissions must be granted in Azure for MSGraph.

azure-deprecation/dashboard#60

https://issues.redhat.com/browse/CORS-1897

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 do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2022

openshift-ci bot requested review from jhixson74 and m1kola November 22, 2022 22:34

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 30, 2022

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2022

r4f4 force-pushed the azure-servp-3 branch from ced300b to aa238f0 Compare December 6, 2022 14:14

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2022

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2022

openshift-ci bot changed the title ~~azure: replace deprecated AD Graph API~~ OCPBUGS-4549: azure: replace deprecated AD Graph API Dec 6, 2022

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Dec 6, 2022

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Dec 6, 2022

r4f4 mentioned this pull request Dec 6, 2022

[release-4.12] OCPBUGS-4542: Azure: use azidentity with an adapter #6671

Closed

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2022

r4f4 added 2 commits December 7, 2022 21:49

azure: replace deprecated AD Graph API

b360ef5

Use the new MS Graph API instead. Please note that `Applications.ReadWrite.OwnedBy` permissions must be granted in Azure for MSGraph. azure-deprecation/dashboard#60 https://issues.redhat.com/browse/CORS-1897

azure: update mod and vendor

8411482

r4f4 force-pushed the azure-servp-3 branch from aa238f0 to 8411482 Compare December 7, 2022 20:57

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2022

patrickdillon mentioned this pull request Dec 8, 2022

Azure: use the new Graph API to delete Service Principals (if possible) #5714

Closed

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2022

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 9, 2022

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 10, 2022

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 10, 2022

openshift-merge-robot merged commit 780b29a into openshift:master Dec 10, 2022

r4f4 mentioned this pull request Dec 19, 2022

OCPBUGS-4549: destroy: azure: handle nil responses from msgraph sdk #6717

Merged

openshift-ci-robot mentioned this pull request Feb 11, 2023

OCPBUGS-4549: azure: fix MS Graph calls on Gov cloud #6844

Merged

OCPBUGS-4549: azure: replace deprecated AD Graph API #6614

OCPBUGS-4549: azure: replace deprecated AD Graph API #6614

Uh oh!

Conversation

r4f4 commented Nov 22, 2022

Uh oh!

r4f4 commented Nov 22, 2022

Uh oh!

r4f4 commented Nov 22, 2022

Uh oh!

r4f4 commented Nov 22, 2022

Uh oh!

patrickdillon commented Nov 30, 2022

Uh oh!

openshift-ci bot commented Nov 30, 2022

Uh oh!

r4f4 commented Dec 6, 2022

Uh oh!

r4f4 commented Dec 6, 2022

Uh oh!

r4f4 commented Dec 6, 2022

Uh oh!

openshift-ci-robot commented Dec 6, 2022

Uh oh!

r4f4 commented Dec 6, 2022

Uh oh!

patrickdillon commented Dec 7, 2022

Uh oh!

patrickdillon commented Dec 7, 2022

Uh oh!

openshift-ci-robot commented Dec 8, 2022

Uh oh!

openshift-ci-robot commented Dec 8, 2022

Uh oh!

openshift-ci-robot commented Dec 8, 2022

Uh oh!

r4f4 commented Dec 9, 2022

Uh oh!

openshift-ci bot commented Dec 9, 2022

Uh oh!

r4f4 commented Dec 9, 2022

Uh oh!

openshift-ci-robot commented Dec 9, 2022

Uh oh!

openshift-ci-robot commented Dec 9, 2022

Uh oh!

r4f4 commented Dec 9, 2022

Uh oh!

openshift-ci bot commented Dec 9, 2022

Uh oh!

patrickdillon commented Dec 9, 2022

Uh oh!

r4f4 commented Dec 9, 2022

Uh oh!

openshift-ci bot commented Dec 9, 2022

Uh oh!

openshift-ci-robot commented Dec 10, 2022

Uh oh!

openshift-ci-robot commented Dec 10, 2022

Uh oh!

r4f4 commented Dec 10, 2022

Uh oh!

openshift-ci bot commented Dec 10, 2022

Uh oh!

openshift-ci-robot commented Dec 10, 2022

Uh oh!

r4f4 commented Dec 10, 2022

Uh oh!

openshift-ci bot commented Dec 10, 2022

Uh oh!

openshift-ci bot commented Dec 10, 2022

Uh oh!

openshift-ci-robot commented Dec 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development