podman build --remote URI Dockerfile should not be treated as file #20476
Conversation
openshift-ci
bot
added
release-note
approved
github-actions
bot
added
the
kind/api-change
edsantiago
changed the title
| for _, containerfile := range m { | ||
| containerFiles = append(containerFiles, filepath.Join(contextDirectory, filepath.Clean(filepath.FromSlash(containerfile)))) | ||
| if containerfile[0] == byte('/') { |
There was a problem hiding this comment.
Shouldn't this just check if containerfile is a valid URL using ParseRequestURI and treat every other value as a path ? I mean value could be a relative path which does not starts with /
There was a problem hiding this comment.
A path is treated as a valid URI. Perhaps I can do the same check as before if the parsed requests path equals the path, then it is a path.
| for _, containerfile := range m { | ||
| containerFiles = append(containerFiles, filepath.Join(contextDirectory, filepath.Clean(filepath.FromSlash(containerfile)))) | ||
| u, err := url.ParseRequestURI(containerfile) |
There was a problem hiding this comment.
what happens if there IS an error?
There was a problem hiding this comment.
We ignore it and treat it as a file path, which is status quo.
tmp/Dockerfile was causing an error, while
/tmp/Dockerfile passes. So we treat either case as a file.
| for _, containerfile := range m { | ||
| containerFiles = append(containerFiles, filepath.Join(contextDirectory, filepath.Clean(filepath.FromSlash(containerfile)))) | ||
| u, err := url.ParseRequestURI(containerfile) | ||
| if err != nil || u.Path == containerfile { |
There was a problem hiding this comment.
Can you add a comment? I am just on my first cup of coffee and it took much longer than I wished to understand what this condition does.
Is the u.Path == contianerfile really needed? AFAIK a URL needs a schema and /tmp/Dockerfile doesn't have one.
There was a problem hiding this comment.
Buildah does a strings.HasPrefix check for http{s}://. Should we do that here as well? This way, errors aren't ignored in case of syntax mistakes etc.
pkg/bindings/images/build.go
Outdated
| @@ -430,6 +430,11 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO | |||
|
|
|||
| dontexcludes := []string{"!Dockerfile", "!Containerfile", "!.dockerignore", "!.containerignore"} | |||
| for _, c := range containerFiles { | |||
| u, err := url.ParseRequestURI(c) | |||
Podman build --remote is translating https://path as if it was a file path. This change will leave it as a URL so it can be parsed on the server side. Fixed: containers#20475 Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
@vrothberg Mergeme... |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
github-actions
bot
added
the
locked - please file new issue/PR
Podman build --remote is translating https://path as if it was a file path. This change will leave it as a URL so it can be parsed on the server side.
Fixed: #20475
Does this PR introduce a user-facing change?