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.

Sign up for GitHub

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

Jump to bottom

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

Closed
JonCrowther opened this issue Apr 2, 2024 · 1 comment · Fixed by #45485
Closed

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

JonCrowther opened this issue Apr 2, 2024 · 1 comment · Fixed by #45485
Assignees
@bigkevmcd
Labels
kind/enhancement Issues that improve or augment existing functionality team/collie the team that is responsible for auth and rbac within rancher
Milestone
v2.9-Next1

Comments

@JonCrowther
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently to interact with the Microsoft Graph API, Rancher uses this 3rd party Hamilton library. There are some security, performance and maintenance concerns. Additionally, past issues we've had with the Hamilton library have taken a while to get addressed.

Describe the solution you'd like
We would like to move to the officially supported Microsoft go-sdk.

Describe alternatives you've considered
We could continue to use the existing 3rd party library, and open issues in that repo as needed. Historically that has worked for smaller changes, but if there's a significant change/feature we need, it can be very slow to get those changes into the Hamilton library.

Additional context
There is ongoing work for our Azure AD support, so the earlier we are able to migrate, the less challenging the migration will be.
@JonCrowther JonCrowther added kind/enhancement Issues that improve or augment existing functionality team/collie the team that is responsible for auth and rbac within rancher labels Apr 2, 2024
@bigkevmcd
Copy link
Contributor

bigkevmcd commented Apr 8, 2024 •
edited

Migrating to the Azure SDK for Go

Using https://github.com/microsoftgraph/msgraph-sdk-go

Rancher currently has two clients for Azure, the now deprecated AzureADGraphClient and the MSGraphClient.

I don't believe we know how many users use the deprecated client, and I can't find the deprecation timeline for it.

Azure SDK AzureClient

We would need to implement AzureClient but using the Microsoft SDK client.

From a technical PoV this is simply going through the code in the current MS Graph Client looking up the methods in the msgraph-sdk GoDoc and translating the calls that we use (based on the hamilton package) into calls to the Microsoft package.

It's complicated by the documentation for the package, which is not particularly helpful.

There are some useful examples in the package documentation and snippets.

Before reimplementing the client

  • Do we want to fully deprecate the old client? Can we?
  • Does this replace the new client?
  • Do we want more than one deprecated client?

The AzureClient interface has eight methods, and we have two existing implementations, so it shouldn't be particularly hard to migrate, we need to figure out how we deal with the deprecation elements.

@bigkevmcd bigkevmcd self-assigned this Apr 9, 2024
@samjustus samjustus added this to the v2.9-Next1 milestone Apr 16, 2024
@bigkevmcd bigkevmcd mentioned this issue May 14, 2024
Merged
@aruiz14 aruiz14 mentioned this issue May 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bigkevmcd bigkevmcd

Labels
kind/enhancement Issues that improve or augment existing functionality team/collie the team that is responsible for auth and rbac within rancher
Projects
None yet
Milestone
v2.9-Next1
Development

Successfully merging a pull request may close this issue.

Replace msgraph client. bigkevmcd/rancher
3 participants
@bigkevmcd @JonCrowther @samjustus