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

Update googleauth dependency #591

Merged
merged 1 commit into from
Mar 20, 2023
Merged

Conversation

dhstewart
Copy link
Contributor

@dhstewart dhstewart commented Dec 20, 2022

Why

I did some research on this issue #564

It looks like to get openid connect up to date we'll need to get to Faraday v2, but our current version of googleauth constrains below that.

So it seems that getting the googleauth dependency into v1 is a necessary first step to addressing the issue.

What

  • update the googleauth dependency (here's the the changelog)
  • ran tests locally, all passed

If a more incremental approach to dependency updates are desired, I'm happy to approach this differently.

Let me know what you think 👍

Eventually I'd like to see kubeclient get up to Faraday v2. Getting
the googleauth dependency into v1 is a necessary step.
@cben
Copy link
Collaborator

cben commented Dec 20, 2022

FYI, test_informer.rb is known flaky, we have a tracking issue for that, but for now its failures are not merge blockers.

UPDATE: flakes'd been fixed since then, closed & reopened to re-run CI with current master => all green now 🍏

@cben cben closed this Jan 15, 2023
@cben cben reopened this Jan 15, 2023
@cben
Copy link
Collaborator

cben commented Jan 15, 2023

Thanks! Sorry, forgot this for no good reason :-(

LGTM and reading their changelog, sounds "probably non-breaking". But I really don't know almost anything about google auth, and that's some ~30 releases they've had (most before adopting semver)...
@dhstewart have you done any actual testing against GCP?

@lucasmazza @timothysmith0609 @jeremywadsack can any of you comment on the safety of this upgrade, and/or test it?

@jeremywadsack
Copy link
Contributor

@cben The latest version of googleauth we are using in production is 0.17.1 right now and that works fine with kubeclient. I won't have time to test this in a real GKE cluster but that's certainly something that I'd expect before merging this, especially with a major version change.

@dhstewart If you just need Faraday v2, can you use the spec '~> 1.1', '>= 1.1.2'? That way it's not forcing potential updates on other libraries that might be pinned below 1.3 (for whatever reason).

@hmcguire-shopify
Copy link

can any of you comment on the safety of this upgrade, and/or test it?

We've been using googleauth 1.3.0 with the master branch of Kubeclient and haven't seen any issues.

@cben
Copy link
Collaborator

cben commented Mar 20, 2023

I think I'm gonna merge this and see if anybody complains. Could relax the constaint to 1.1+ later if needed.

#606 is going towards some "how exactly do these libraries cache/expire tokens" questions, and reliably answering that for a wide range of versions is harder; if we can get away with only supporting recent ones that's simpler 😅

@cben cben merged commit 6b40464 into ManageIQ:master Mar 20, 2023
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.

4 participants