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

Replace msgraph client. #45485

Open
wants to merge 4 commits into
base: release/v2.9
Choose a base branch
from

Conversation

bigkevmcd
Copy link
Contributor

@bigkevmcd bigkevmcd commented May 14, 2024

Issue:

Fixes #45009

Problem

This replaces the manicminer/hamilton dependency with the upstream msgraph client functionality.

Solution

This is a replacement of one package with another.

Testing

This will require detailed login/user import testing.

Engineering Testing

Manual Testing

I've logged in, and things seem to work, user is imported etc.

Automated Testing

Tests are added, but they require upstream API tokens.

  • Test types added/modified:
    • Unit
    • Integration (Go Framework)
    • Integration (v2prov Framework)
    • Validation (Go Framework)
    • Other - Explain: EXPLAIN
    • None
    • REMOVE NOT APPLICABLE BULLET POINTS ABOVE
  • If "None" - Reason: EXPLAIN THE REASON
  • If "None" - GH Issue/PR: LINK TO GH ISSUE/PR TO ADD TESTS

Summary: TODO

QA Testing Considerations

Regressions Considerations

TODO

Existing / newly added automated tests that provide evidence there are no regressions:

  • TODO

@bigkevmcd bigkevmcd marked this pull request as ready for review May 15, 2024 08:04
Copy link

@andreas-kupries andreas-kupries left a comment

Choose a reason for hiding this comment

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

2 bits of nits seen.

pkg/auth/providers/azure/clients/ms_graph_client.go Outdated Show resolved Hide resolved
pkg/auth/providers/azure/clients/ms_graph_client.go Outdated Show resolved Hide resolved
@samjustus samjustus requested a review from enrichman May 15, 2024 15:02
Copy link
Contributor

@enrichman enrichman left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@bigkevmcd bigkevmcd closed this May 17, 2024
@bigkevmcd bigkevmcd deleted the replace-hamilton branch May 17, 2024 11:22
@bigkevmcd bigkevmcd restored the replace-hamilton branch May 17, 2024 11:22
@bigkevmcd bigkevmcd reopened this May 17, 2024
@bigkevmcd bigkevmcd force-pushed the replace-hamilton branch 3 times, most recently from 6e2d009 to 852b6f2 Compare May 23, 2024 12:39
bigkevmcd and others added 4 commits May 24, 2024 15:38
This replaces the manicminer/hamilton dependency with the upstream
msgraph client functionality.
Co-authored-by: Enrico Candino <enrico.candino@gmail.com>
Improve the log messages to standardise the prefixes and log out when we
receive a value of the wrong type.

Reduce the number of contexts being created, ideally we'd receive these
as part of the client API definition.
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.

[RFE] Migrate Microsoft Graph API client from manicminer/hamilton to microsoftgraph/msgraph-sdk-go
3 participants