feat: Add gomod git auth #509
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Peter Engelbert <[email protected]>
The auth header needs to be: ``` Authorization: basic <token-hash> ``` Where `<token-hash>` is the Base64 encoding of the string `"x-access-token:<token>"` and `<token>` is the token as provided by the authenticator (usually github). Prior commits missed the `x-access-token` part. It still worked, but was not strictly correct. Some other minor implementation details were changed, such as when shell quotes are added. This commit also adds some tests, but they will not succeed. Certain files are embedded as strings, and those files were not checked in for security reasons. The test implementation is also not correct, so it needs to be changed anyway. Signed-off-by: Peter Engelbert <[email protected]>
The new test will generate LLB for the gomods and validate that the correct steps are taken. The test in this commit is really just a skeleton. It doesn't perform the proper checks, but it generates the LLB. This commit also removes the previous test. In a future commit, I will add a test for the `llb.StateOption` that modifies the global gitconfig in the gomod-fetching worker container. We can check the generated gitconfig for validity. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
This is needed because go calls git, which needs to know which username to use for which hosts. Signed-off-by: Peter Engelbert <[email protected]>
Previously, secrets could have leaked via the generated gitconfig entering the cache. This commit generates the gitconfig into a tmpfs mount so that it does not persist to the cache after it is used. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
MorrisLaw
reviewed
Jan 29, 2025
source_test.go
Outdated
| assert.Check(t, cmp.Len(secrets, 3), secrets) | ||
|
|
||
| for _, auth := range src.Generate[0].Gomod.Auth { | ||
| chk := auth.Token |
There was a problem hiding this comment.
super nit (feel free to ignore since it's a test and it's more of an opinionated style comment):
Suggested change
| chk := auth.Token | |
| var chk string | |
| switch { | |
| case auth.Token != "": | |
| chk = auth.Token | |
| case auth.Header != "": | |
| chk = auth.Header | |
| default: | |
| chk = auth.SSH | |
| } |
test/testenv/buildx.go
Outdated
| for _, kv := range kvs { | ||
| k := kv.k | ||
| v := kv.v | ||
| m[k] = []byte(v) |
There was a problem hiding this comment.
nit: just do:
Suggested change
| m[k] = []byte(v) | |
| m[kv.k] = []byte(kv.v) |
source_test.go
Outdated
| } | ||
|
|
||
| tfso := mnt.GetTmpfsOpt() | ||
| if tfso == nil { |
There was a problem hiding this comment.
test nit:
Suggested change
| if tfso == nil { | |
| if tfso == nil || tfso.Size == 0 { |
generator_gomod.go
Outdated
| @@ -32,7 +38,12 @@ func (s *Spec) HasGomods() bool { | |||
|
|
|||
| func withGomod(g *SourceGenerator, srcSt, worker llb.State, opts ...llb.ConstraintsOpt) func(llb.State) llb.State { | |||
| return func(in llb.State) llb.State { | |||
| workDir := "/work/src" | |||
| const ( | |||
| fourKB = 4 * (1 << 10) | |||
There was a problem hiding this comment.
Why not just hardcode 4096 here? It's more obvious and we won't need to unnecessarily compute this every time a worker is ran for a path.
There was a problem hiding this comment.
It's a const so it'll be computed once at compile time, but 4096 is a lot clearer so I'll use that instead.
Signed-off-by: Peter Engelbert <[email protected]>
Also, make it clear that certain lines should be added to the gitconfig generation script only when there is actually auth to deal with. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commit Descriptions