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

Address cached references to old versions in the Go module proxy #17

Closed
mfridman opened this issue May 29, 2021 · 17 comments
Closed

Address cached references to old versions in the Go module proxy #17

mfridman opened this issue May 29, 2021 · 17 comments
Milestone

Comments

@mfridman
Copy link
Member

Unfortunately the Go module proxy (i.e., mirror) cached 2 versions:

https://pkg.go.dev/github.com/golang-jwt/jwt?tab=versions

When a user does a go get github.com/golang-jwt/jwt it will download the following version:

github.com/golang-jwt/jwt v3.2.0+incompatible // indirect

Whereas it should import the correct one (based on latest commit on main)

github.com/golang-jwt/jwt v0.0.0-20210529012641-6a07921e6808 // indirect

If we cannot get those 2 versions removed from the proxy, we'll have to go with plan B... add a /v3 and tag a v3.2.1

@oxisto
Copy link
Collaborator

oxisto commented May 29, 2021

Did you already file an issue via https://golang.org/s/pkgsite-feedback for that? Because I am not quite sure if removing it from the pkg site also removes it from the go mirror, etc.

What could also work is to retract the two versions: https://blog.golang.org/go116-module-changes#TOC_5. But I think this probably Is only going to work on Go 1.16.

Not a good start :(

@oxisto
Copy link
Collaborator

oxisto commented May 29, 2021

What could also work is to retract the two versions: https://blog.golang.org/go116-module-changes#TOC_5. But I think this probably Is only going to work on Go 1.16.

Not a good start :(

As can be seen in #23 module retraction is in no way backwards compatible...

@oxisto
Copy link
Collaborator

oxisto commented May 29, 2021

Sorry for the fragmented thoughts and posts here... With all the information available currently, I would propose the following action plan

Step 1: Remove go.mod again

Currently, we have no external dependencies in the library (this is also probably a good reason why this was so popular). So the benefit of having go modules is debatable and almost zero. With the exception of the annoying +incompatible branding on the version - which bugs me as well - but I think we could live with that for a moment. Go's way of enforcing SIV after a v2 has lead to so much debate and pain, but unfortunately the design seems frozen these days.

See #26

Step 2: Release a v3.2.1+incompatible

Mainly with #6 fixed, possibly also #15.
Optional: Re-release the old release tags, so that we have the complete version history

Step 3: Continue working on backwards-compatible stuff in the v3.x.x+incompatible (main) branch

Mainly things that are on the wish-list of the community, such as features that I have listed in #16 without breaking any thing.

Step 4: Prepare a v4 branch with Go modules active

This branch will introduce breaking changing to the API as well as the import path, as we will switch to github.com/golang-jwt/jwt/v4 and do a proper release with Go modules.

cc @mfridman @Waterdrips @lggomez

@mfridman
Copy link
Member Author

I created an issue, but it'll likely be closed. golang/go#46440

Since users would need to modify their import paths anyways couldn't we do the following:

  1. Re-release the old release tags, so that we have the complete version history
  2. Modify go.mod and suffix the module with /v3, adding SIV
  3. Release a new v3.2.1 and update README

Then v3 becomes the drop-in replacement for dgrijalva/jwt-go

github.com/golang-jwt/jwt/v3

@mfridman
Copy link
Member Author

Or, as mentioned above .. just drop module support altogether for v3.x.x+incompatible and add breaking changes with a /v4

@mfridman mfridman reopened this May 29, 2021
@oxisto
Copy link
Collaborator

oxisto commented May 29, 2021

Or, as mentioned above .. just drop module support altogether for v3.x.x+incompatible and add breaking changes with a /v4

Both ways work I guess. Somehow I have the feeling that going the v3.x.x+incompatible + v4SIV later way seems a little bit "cleaner", than having some v3 releases in the jwt module and some in the jwt/v3 module.

@mfridman
Copy link
Member Author

going the v3.x.x+incompatible + v4SIV later way seems a little bit "cleaner"

@oxisto Okay, that sounds like a plan.

A slight tangent, I always try and avoid SIV as much as possible. There was an interesting proposal here to make it optional: golang/go#44550

oxisto added a commit that referenced this issue May 29, 2021
As discussed in full length here (#17), we have run into issues that forces us to abandon go modules, at least for the `v3.x.x` releases. After this is merged in, we can release a `v3.2.1+incompatible` version, which contains a security fix. 

Afterwards, we will work on non-breaking quality of life fixes and then eventually run a `v4` version, which most likely will then support go modules and have a new SIV-style import path.
@oxisto oxisto added this to the v3.2.1 milestone May 29, 2021
@oxisto
Copy link
Collaborator

oxisto commented May 31, 2021

I created an issue, but it'll likely be closed. golang/go#46440

Since users would need to modify their import paths anyways couldn't we do the following:

  1. Re-release the old release tags, so that we have the complete version history
  2. Modify go.mod and suffix the module with /v3, adding SIV
  3. Release a new v3.2.1 and update README

Then v3 becomes the drop-in replacement for dgrijalva/jwt-go

github.com/golang-jwt/jwt/v3

Should we set ourselves a "deadline" how long we should wait for a reaction from the Go team?

@mfridman
Copy link
Member Author

Arbitrarily, say by Jun 2?

It's not a full blocker so its better to get a stable release with an updated README describing what this repo is and how one might migrate from dgrijalva/jwt-go -> golang-jwt/jwt

Even if we have to adopt SIV or continue with +incompatible its not the end of the world.


Another thought, the checksum db will always have those versions.. so now that I'm thinking about it, even if it is removed from the proxy it will likely cause issues if those versions are used again (but different hashes). So for the sake of accuracy and security maybe we continue with the exact versions from dgrijalva/jwt-go

@oxisto
Copy link
Collaborator

oxisto commented May 31, 2021

Arbitrarily, say by Jun 2?

It's not a full blocker so its better to get a stable release with an updated README describing what this repo is and how one might migrate from dgrijalva/jwt-go -> golang-jwt/jwt

Even if we have to adopt SIV or continue with +incompatible its not the end of the world.

Another thought, the checksum db will always have those versions.. so now that I'm thinking about it, even if it is removed from the proxy it will likely cause issues if those versions are used again (but different hashes). So for the sake of accuracy and security maybe we continue with the exact versions from dgrijalva/jwt-go

Sounds reasonable and yes the checksum thing might create a problem anyway, so I think going the v3.x.x+incompatible route for now is ok. Then we have time until Wednesday to create an appropriate README and upgrade guide (see #25).

@mfridman
Copy link
Member Author

mfridman commented Jun 4, 2021

Alright, unless any objections.. I say we continue with our plan assuming the latest tagged version is v3.2.0

To recap

  1. Drop the go.mod file
  2. Bring all the existing tags back from original repo to preserve history
  3. Tag latest commit with v3.2.1 notably addressing the main issue: Fix issue with MapClaims VerifyAudience []string #12

oxisto added a commit that referenced this issue Jun 4, 2021
* Removing `go.mod` for the v3-release branch

As discussed in full length here (#17), we have run into issues that forces us to abandon go modules, at least for the `v3.x.x` releases. After this is merged in, we can release a `v3.2.1+incompatible` version, which contains a security fix. 

Afterwards, we will work on non-breaking quality of life fixes and then eventually run a `v4` version, which most likely will then support go modules and have a new SIV-style import path.

* Cloning into $GOPATH for GitHub actions
@oxisto
Copy link
Collaborator

oxisto commented Jun 6, 2021

Alright, unless any objections.. I say we continue with our plan assuming the latest tagged version is v3.2.0

To recap

  1. Drop the go.mod file
  2. Bring all the existing tags back from original repo to preserve history
  3. Tag latest commit with v3.2.1 notably addressing the main issue: Fix issue with MapClaims VerifyAudience []string #12

Did you have a chance to look at #27? I am trying to put some additional documentation on this, afterwards we can release v3.2.1.

@mfridman
Copy link
Member Author

mfridman commented Jun 7, 2021

Will take a look today @oxisto .. would you happen to know how to "re-import" all the existing tags?

Since the git history is the same (and this repo is a superset of all the git history), I'd imagine pulling the old repo, pointing at the new location and just pushing the tag refs. But need to test this when I get a chance.

@oxisto
Copy link
Collaborator

oxisto commented Jun 7, 2021

Will take a look today @oxisto .. would you happen to know how to "re-import" all the existing tags?

Thanks!

Since the git history is the same (and this repo is a superset of all the git history), I'd imagine pulling the old repo, pointing at the new location and just pushing the tag refs. But need to test this when I get a chance.

That would have been my approach as well, yes.

@mfridman
Copy link
Member Author

mfridman commented Jun 8, 2021

@oxisto I've pushed all existing tags except the v4.0.0-preview1

Now this repo has a 1:1 mirror with all the existing tags from dgrijalva/jwt-go.

So once #27 is merged we can cut a v.3.2.1 release.

@oxisto
Copy link
Collaborator

oxisto commented Jun 8, 2021

@oxisto I've pushed all existing tags except the v4.0.0-preview1

Now this repo has a 1:1 mirror with all the existing tags from dgrijalva/jwt-go.

So once #27 is merged we can cut a v.3.2.1 release.

Done. I think you can go ahead and release it, we gave enough time for objections.

@mfridman
Copy link
Member Author

mfridman commented Jun 8, 2021

Thanks @oxisto

I don't have bandwidth at the moment to write a proper changelog release notes between v.3.2.0..v3.2.1

But the tag is there so this repo is patched and ready to be consumed afaics.

https://github.com/golang-jwt/jwt/releases/tag/v3.2.1

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

No branches or pull requests

2 participants