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

Authentication changes with Octokit v4.22 causes failures with legacy Faraday versions #1392

Closed
matt-taylor opened this issue Jan 15, 2022 · 14 comments
Assignees
Labels
Type: Bug Something isn't working as documented

Comments

@matt-taylor
Copy link

Problem:

With Faraday v1.x, Authentication helpers were provided to help set the request headers properly. However, these Authentication helpers are deprecated with Faraday v2.x and the new Faraday authorization middleware must be used.

Octokit v4.22 introduced changes to allow Faraday auth middleware to work instead of explicitly setting headers. (#1359)
This means that Octokit v4.22 is reliant on at least Faraday v1.x. However, this is not reflected as a dependency in the Gemspec causing failures with downstream repos with a legacy Faraday Gem.

Question:

While I understand that most repos will keep up to date with the Faraday client, For repos that can not update to a more recent version of Faraday client, Octokit client v4.22 fails authentication. With v4.22 can we get the Gemspec updated, to reflect the minimum Faraday version required with the new authentication flow (Octokit v4.22 Gemspec) ? Thanks

@jhirbour
Copy link

Even a quick note about this is the release notes would have just saved me 2 hours of my life.

pulkit110 added a commit to gatemedia/github-cookbook that referenced this issue Jan 26, 2022
Octokit 4.22.0 not compatible with legacy faraday versions.

see octokit/octokit.rb#1391
see octokit/octokit.rb#1392
see octokit/octokit.rb#1388

fix gatemedia/gmp-os#218
binford2k added a commit to binford2k/puppetlabs-dsc_lite that referenced this issue Jan 27, 2022
Due to octokit/octokit.rb#1392, the changelog
generator cannot authenticate properly, which means that it rate limits
almost instantly and takes many hours to generate a changelog. This
should be reverted when that issue is resolved.
@timrogers timrogers added the bug label Apr 26, 2022
@trashypete
Copy link

trashypete commented Oct 11, 2022

@matt-taylor @G-Rath @JamieMagee
Are there any updates or potential fixes to this yet in the meanwhile?

Im using octokit (4.22.0) which is pulling in the incompatible version of faraday. Unfortunately, upgrading octokit is not a potential solution as it's an indirect dependency, i.e. a dependency of a dependency. I believe the spec change would help tremendously.

@jhirbour
Copy link

I'm working on shedding a gem from my app that's blocking me from upgrading faraday. I'll take a hack at this after I get the other blocker out of the way. Submit a PR etc.

@matt-taylor
Copy link
Author

@trashypete Unfortunately the Octokit team has yet to respond or fix the underlying problem....

My team and I was forced to pin Octokit in all of our services until we were able to upgrade our Faraday versions to a version compatible with the new Octokit version.

@nickfloyd nickfloyd self-assigned this Oct 12, 2022
@nickfloyd
Copy link
Contributor

Hey friends, apologies for the delay on this and on even getting a response to it; this one just fell through, and we missed it.

I'll be taking a look at this today to see if we can get this resolved for y'all. Also, if anyone else has time please feel free to work through it.

Again, really sorry for the pain that this has caused.

@trashypete
Copy link

@matt-taylor I ended up wrapping it in my own gem to coerce the proper version. @nickfloyd Thanks Nick!

@nickfloyd
Copy link
Contributor

Hey Friends,

Acknowledging that there is loads of history and context here and that I could easily be missing most of that despite all of the great info y'all have provided and referenced, I wanted to ask some questions to help clarify my understanding of the depth of this pain point.

I just don't want to jump in and start changing things without fully understanding what happened (so that we can avoid this type of thing in the future) and what we should do about it.


The problem

It seems like the root of the issue here is that back in January, with the v4.22.0 release Faraday 1.x features were added but we never updated the gemspec from '>= 0.9' to >=1 but we later released v4.23.0 with the correct version specified.

This is causing an issue because in the 4.22.0 changeset we introduced middleware that uses 1.x faraday features but dependency resolution some cases still tries to use 0.9 and not 1.x

Let me know if the above is correct.


Possible solutions

If the above is correct, I think I can see two possible solutions (one because there is mention of not being able to update to the latest version of octokit - but I'll mention it anyway).

Option1: Patch v4.22.0

  • I can make a new branch from 4.22.0 and update the gemspec to >=1.0 only, and ship that as a new gem @ 4.22.1 - which would be true to the implementation.
  • Mark v4.22.0 as deprecated/invalid in gems and in the release notes, citing this issue and directing users to 4.22.1

Option2: Ask users to update to v4.23.0

  • Mark v4.22.0 as deprecated/invalid in gems and in the release notes, citing this issue and directing users to 4.23.0 where this was corrected.

Let me know if what I am describing above regarding the problem and potential solutions is in line with what y'all are thinking or if I am way off and need some alignment on what is going on. Thanks also for your patience while we get this resolved!

@jhirbour
Copy link

To me the faraday changes are a big enough breaking change for lots of other gems a full version number bump makes sense...

That said I can't update until I can cull out the media-wiki gem from our systems... so it may take me a while to update.

@trashypete
Copy link

@nickfloyd yes you are correct. While option 1 would address my issues, it is a very particular edge case I’m not sure many people are experiencing, I have wrapped my dependency and isolated the issue. The more appropriate solution for the public would be option two as @jhirbour pointed out. Thank you for your efforts.

@nickfloyd
Copy link
Contributor

Thanks for the feedback y'all. I'll move on option 2 just to make sure others walking up to this have more clarity and direction. Thank you again for the help and patience.

@nickfloyd
Copy link
Contributor

For visibility:

Updated release notes on v4.22.0 - https://github.com/octokit/octokit.rb/releases/tag/v4.22.0
Community discussion on this issue: #1492

@nickfloyd
Copy link
Contributor

I will close this issue unless anyone feels we should continue the discussion. As per our discussion, this was resolved via release notes and community discussion - please feel free to re-open it if we have more to discuss/do around this.

@lysenkooo
Copy link

Spent whole day just to realize I need to downgrade 4.22.0 to 4.21.0 to fix auth issue.

@lukewduncan
Copy link

just ran into this issue as well. spent 3 hours before figuring out it was on the faraday track. i followed @lysenkooo and downgraded to version 4.21.0 and all is well.

pulkit110 added a commit to gatemedia/github-cookbook that referenced this issue Mar 19, 2024
Octokit 4.22.0 not compatible with legacy faraday versions.

see octokit/octokit.rb#1391
see octokit/octokit.rb#1392
see octokit/octokit.rb#1388

fix gatemedia/gmp-os#218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

7 participants