-
Notifications
You must be signed in to change notification settings - Fork 1.4k
dockerfile (labs): implement ADD <git ref>
#2799
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ run: | |
|
|
||
| build-tags: | ||
| - dfrunsecurity | ||
| - dfaddgit | ||
|
|
||
| linters: | ||
| enable: | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| //go:build dfaddgit | ||
| // +build dfaddgit | ||
|
|
||
| package dockerfile2llb | ||
|
|
||
| const addGitEnabled = true |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| //go:build !dfaddgit | ||
| // +build !dfaddgit | ||
|
|
||
| package dockerfile2llb | ||
|
|
||
| const addGitEnabled = false |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| //go:build dfaddgit | ||
| // +build dfaddgit | ||
|
|
||
| package dockerfile | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
| "text/template" | ||
|
|
||
| "github.com/containerd/continuity/fs/fstest" | ||
| "github.com/moby/buildkit/client" | ||
| "github.com/moby/buildkit/frontend/dockerfile/builder" | ||
| "github.com/moby/buildkit/util/testutil/integration" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| var addGitTests = integration.TestFuncs( | ||
| testAddGit, | ||
| ) | ||
|
|
||
| func init() { | ||
| allTests = append(allTests, addGitTests...) | ||
| } | ||
|
|
||
| func testAddGit(t *testing.T, sb integration.Sandbox) { | ||
| f := getFrontend(t, sb) | ||
|
|
||
| gitDir, err := os.MkdirTemp("", "buildkit") | ||
| require.NoError(t, err) | ||
| defer os.RemoveAll(gitDir) | ||
| gitCommands := []string{ | ||
| "git init", | ||
| "git config --local user.email test", | ||
| "git config --local user.name test", | ||
| } | ||
| makeCommit := func(tag string) []string { | ||
| return []string{ | ||
| "echo foo of " + tag + " >foo", | ||
| "git add foo", | ||
| "git commit -m " + tag, | ||
| "git tag " + tag, | ||
| } | ||
| } | ||
| gitCommands = append(gitCommands, makeCommit("v0.0.1")...) | ||
| gitCommands = append(gitCommands, makeCommit("v0.0.2")...) | ||
| gitCommands = append(gitCommands, makeCommit("v0.0.3")...) | ||
| gitCommands = append(gitCommands, "git update-server-info") | ||
| err = runShell(gitDir, gitCommands...) | ||
| require.NoError(t, err) | ||
|
|
||
| server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(gitDir)))) | ||
| defer server.Close() | ||
| serverURL := server.URL | ||
| t.Logf("serverURL=%q", serverURL) | ||
|
|
||
| dockerfile, err := applyTemplate(` | ||
| FROM alpine | ||
|
|
||
| # Basic case | ||
| ADD {{.ServerURL}}/.git#v0.0.1 /x | ||
| RUN cd /x && \ | ||
| [ "$(cat foo)" = "foo of v0.0.1" ] | ||
|
|
||
| # Complicated case | ||
| ARG REPO="{{.ServerURL}}/.git" | ||
| ARG TAG="v0.0.2" | ||
| ADD --keep-git-dir=true --chown=4242:8484 ${REPO}#${TAG} /buildkit-chowned | ||
| RUN apk add git | ||
| USER 4242 | ||
| RUN cd /buildkit-chowned && \ | ||
| [ "$(cat foo)" = "foo of v0.0.2" ] && \ | ||
| [ "$(stat -c %u foo)" = "4242" ] && \ | ||
| [ "$(stat -c %g foo)" = "8484" ] && \ | ||
| [ -z "$(git status -s)" ] | ||
| `, map[string]string{ | ||
| "ServerURL": serverURL, | ||
| }) | ||
| require.NoError(t, err) | ||
| t.Logf("dockerfile=%s", dockerfile) | ||
|
|
||
| dir, err := integration.Tmpdir(t, | ||
| fstest.CreateFile("Dockerfile", []byte(dockerfile), 0600), | ||
| ) | ||
| require.NoError(t, err) | ||
| defer os.RemoveAll(dir) | ||
|
|
||
| c, err := client.New(sb.Context(), sb.Address()) | ||
| require.NoError(t, err) | ||
| defer c.Close() | ||
|
|
||
| _, err = f.Solve(sb.Context(), c, client.SolveOpt{ | ||
| LocalDirs: map[string]string{ | ||
| builder.DefaultLocalNameDockerfile: dir, | ||
| builder.DefaultLocalNameContext: dir, | ||
| }, | ||
| }, nil) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| func applyTemplate(tmpl string, x interface{}) (string, error) { | ||
| var buf bytes.Buffer | ||
| parsed, err := template.New("").Parse(tmpl) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| if err := parsed.Execute(&buf, x); err != nil { | ||
| return "", err | ||
| } | ||
| return buf.String(), nil | ||
| } |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| dfrunsecurity | ||
| dfrunsecurity dfaddgit |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make sure that
Is converted to direct
llb.Git(), notllb.Scratch().File(llb.Copy(llb.Git(), "/", "/")). Possibly this could be also optimized in the llb libarary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to work on this in this PR or a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be better in this PR. Separate commit is ok. If you want to do this in
llbclient level then it can be done in parallel on separate PR but we should avoid the possibility of having the git feature but not this optimization merged.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have that optimization for
httpsources?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an optimizer in the second commit