-
Notifications
You must be signed in to change notification settings - Fork 2.1k
add local fork of github.com/docker/docker/builder/remotecontext #6190
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
Conversation
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
Signed-off-by: Daniel Nephin <dnephin@docker.com>
`docker build` accepts remote repositories using either the `git://` notation, or `git@`. Docker attempted to parse both as an URL, however, `git@` is not an URL, but an argument to `git clone`. Go 1.7 silently ignored this, and managed to extract the needed information from these remotes, however, Go 1.8 does a more strict validation, and invalidated these. This patch adds a different path for `git@` remotes, to prevent them from being handled as URL (and invalidated). A test is also added, because there were no tests for handling of `git@` remotes. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was only used inside gitutils, and is written specifically for the requirements there. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If the HEAD request fails, use a GET request to properly test if git server is smart-http. Signed-off-by: Andrew He <he.andrew.mail@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
gty-migrate-from-testify --ignore-build-tags Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Update gotestyourself to gotest.tools
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> (cherry picked from commit 723b107ca4fba14580a6cd971e63d8af2e7d2bbe) Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
gitutils: add validation for ref
``` builder/remotecontext/remote.go:48: G107: Potential HTTP request made with variable url (gosec) builder/remotecontext/git/gitutils.go:145: G107: Potential HTTP request made with variable url (gosec) builder/remotecontext/git/gitutils.go:147: G107: Potential HTTP request made with variable url (gosec) pkg/fileutils/fileutils_test.go:185: G303: File creation in shared tmp directory without using ioutil.Tempfile (gosec) pkg/tarsum/tarsum_test.go:7: G501: Blacklisted import `crypto/md5`: weak cryptographic primitive (gosec) pkg/tarsum/tarsum_test.go:9: G505: Blacklisted import `crypto/sha1`: weak cryptographic primitive (gosec) ``` Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Replace gometalinter with golangci-lint
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
urlutil.IsUrl() was merely checking if the url had a http(s):// prefix, which is just as well handled through using url.Parse() Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: gotestyourself/gotest.tools@v2.3.0...v3.0.1 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
replace pkg/symlink with github.com/moby/sys/symlink
Signed-off-by: Tibor Vass <tibor@docker.com>
The io/ioutil package has been deprecated in Go 1.16. This commit replaces the existing io/ioutil functions with their new definitions in io and os packages. Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
pkg/urlutil (despite its poorly chosen name) is not really intended as a generic utility to handle URLs, and should only be used by the builder to handle (remote) build contexts. - IsURL() only does a very rudimentary check for http(s):// prefixes, without any other validation, but due to its name may give incorrect expectations. - IsGitURL() is written specifically with docker build remote git contexts in mind, and has handling for backward-compatibility, where strings that are not URLs, but start with "github.com/" are accepted. Because of the above, this patch: - moves the package inside builder/remotecontext, close to where it's intended to be used (ideally this would be part of build/remotecontext itself, but this package imports many other dependencies, which would introduce those as extra dependencies in the CLI). - deprecates pkg/urlutil, but adds aliases as there are some external consumers. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Simplify some of the logic, and add documentation about the package, as well as warnings that this package should not be used as a general- purpose utility. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
pkg/urlutil: deprecate, and move to builder/remotecontext/urlutil
Older versions of Go don't format comments, so committing this as a separate commit, so that we can already make these changes before we upgrade to Go 1.19. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make the test more debuggable by logging all git command output and running each table-driven test case as a subtest. Signed-off-by: Cory Snider <csnider@mirantis.com>
Keep It Simple! Set the working directory for git commands by...setting the git process's working directory. Git commands can be run in the parent process's working directory by passing the empty string. Signed-off-by: Cory Snider <csnider@mirantis.com>
Prevent git commands we run from reading the user or system configuration, or cloning submodules from the local filesystem. Signed-off-by: Cory Snider <csnider@mirantis.com>
Member
Author
|
Fixed; had that done in my previous attempt, but should be good now |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
1d1d0b2 to
ed9ea2c
Compare
Adds a local fork of this package for use in the classic builder.
Code was taken at commit [d33d46d01656e1d9ee26743f0c0d7779f685dd4e][1].
Migration was done using the following steps:
# install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
brew install git-filter-repo
# create a temporary clone of docker
cd ~/Projects
git clone https://github.com/docker/docker.git build_context_temp
cd build_context_temp
# commit taken from
git rev-parse --verify HEAD
d33d46d01656e1d9ee26743f0c0d7779f685dd4e
git filter-repo --analyze
# remove all code, except for the remotecontext packages, and move to build/internal docs and previous locations of it
git filter-repo \
--path 'builder/remotecontext/git' \
--path 'builder/remotecontext/urlutil' \
--path-rename builder/remotecontext:cli/command/image/build/internal
# go to the target repository
cd ~/go/src/github.com/docker/cli
# create a branch to work with
git checkout -b fork_remotecontext
# add the temporary repository as an upstream and make sure it's up-to-date
git remote add build_context_temp ~/Projects/build_context_temp
git fetch build_context_temp
# merge the upstream code
git merge --allow-unrelated-histories --signoff -S build_context_temp/master
[1]: https://github.com/docker/docker/d33d46d01656e1d9ee26743f0c0d7779f685dd4e
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ed9ea2c to
b8c794b
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
b8c794b to
e346165
Compare
Member
Author
|
Looks like we may need to add some extra things in the dev container; |
0ccc247 to
016137d
Compare
gitutils_test.go:210: git-http-backend: git: 'http-backend' is not a git command. See 'git --help'. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
016137d to
b05aa46
Compare
Member
Author
|
Another flaky test; looks like we may not be matching errors correctly (or just race condition) 🤔 |
Member
Author
Benehiko
approved these changes
Jul 16, 2025
dmcgowan
approved these changes
Jul 16, 2025
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/cli
that referenced
this pull request
Aug 14, 2025
add local fork of github.com/docker/docker/builder/remotecontext (cherry picked from commit 8c317ad) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/cli
that referenced
this pull request
Aug 14, 2025
add local fork of github.com/docker/docker/builder/remotecontext (cherry picked from commit 8c317ad) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
24 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area/builder/classic-builder
area/builder
kind/refactor
PR's that refactor, or clean-up code
status/2-code-review
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.
add local fork of github.com/docker/docker/builder/remotecontext
Adds a local fork of this package for use in the classic builder.
Code was taken at commit d33d46d01656e1d9ee26743f0c0d7779f685dd4e.
Migration was done using the following steps:
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)