Support podman --remote when Containerfile is not in context directory#18577
Support podman --remote when Containerfile is not in context directory#18577openshift-merge-robot merged 1 commit intocontainers:mainfrom
Conversation
| m = []string{query.Dockerfile} | ||
| } | ||
| for _, containerfile := range m { | ||
| containerFiles = append(containerFiles, filepath.Join(contextDirectory, containerfile)) |
There was a problem hiding this comment.
Probably want to path.Clean() the containerfile value before joining, to strip out any weird prefixes like "../../" and friends.
pkg/bindings/images/build.go
Outdated
|
|
||
| name := filepath.ToSlash(strings.TrimPrefix(path, s+separator)) | ||
| if i > 0 { | ||
| name = path |
There was a problem hiding this comment.
This path isn't going to have the benefit of the ToSlash() change, and I think we always want to have passed the path through that.
There was a problem hiding this comment.
... and if path is a directory, this replacement will happen multiple times while we walk through it, which sounds wrong.
fb905d3 to
cfbf02e
Compare
|
@nalind @Luap99 @giuseppe @vrothberg @mheon @containers/podman-maintainers PTAL |
|
@containers/podman-maintainers PTAL |
pkg/bindings/images/build.go
Outdated
There was a problem hiding this comment.
This is a perfect example of why I ABSOLUTELY LOATHE one char variable names that aren't referenced within 3 or 4 lines. It took me almost 5 minutes to spot where 'd' was defined. For the love of all things holy, please add a few characters to this and any other 1 character variables that aren't referenced within a few lines of its definition.
Fixes: containers#18239 [NO NEW TESTS NEEDED] @test "podman build -f test" in test/system/070-build.bats Will test this. This was passing when run on a local system since the remote end was using the clients path to read the Containerfile The issue is it would not work in a podman machine since the Containerfile would/should be a different path. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lsm5, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
Backport #18577 to v4.4.1-rhel
Fixes: #18239
[NO NEW TESTS NEEDED]
@test "podman build -f test" in test/system/070-build.bats
Will test this. This was passing when run on a local system since the remote end was using the clients path to read the Containerfile The issue is it would not work in a podman machine since the Containerfile would/should be a different path.
Does this PR introduce a user-facing change?