Remote build: nTar secrets with relative paths and ignore bypass#28356
Conversation
Honny1
force-pushed
the
fix-remote-build
branch
from
5f6bd39 to
c7b1275
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
| fsSep := string(filepath.Separator) | ||
| if strings.HasPrefix(path, firstSourceAbs+fsSep) { | ||
| name = filepath.ToSlash(strings.TrimPrefix(path, firstSourceAbs+fsSep)) | ||
| } else { | ||
| name = filepath.ToSlash(path) | ||
| } | ||
| skipExclude = true |
There was a problem hiding this comment.
it is really hard to reason about what this is doing and WHY it is needed, please add comments
also HasPrefix() followed by TrimPrefix() means you should be using CutPrefix() most likely
| os.Setenv("MYSECRET", secret) | ||
| defer os.Unsetenv("MYSECRET") |
There was a problem hiding this comment.
this could use GinkgoT().Setenv() to safe the unset call
|
|
||
| session := podmanTest.PodmanExitCleanly("build", "-f", "build/remote-secret-copy/Dockerfile", "--secret", "id=mysecret,env=MYSECRET", "build/remote-secret-copy") | ||
| Expect(session.OutputToString()).To(ContainSubstring(secret)) | ||
| }) |
There was a problem hiding this comment.
can we just not extend the test case above "podman build with a secret from file and verify if secret file is not leaked into image"? or at the very least I assume these two test cases are a superset of the above one so then why do we still need that one?
putting the critical file existence check into the containerfile hides the important check IMO and it is harder to get that sh -c 'test -z "$(find / -name '\''podman-build-secret*'\'' 2>/dev/null | head -n1)"' is the important thing here, in general that comamnd seem over complicated
There was a problem hiding this comment.
I have consolidated tests of the build with secrets.
When `podman-remote` tars the context, extra `podman-build-secret*` paths were either dropped by `.dockerignore` (containers#25314) or archived as absolute paths so `COPY . .` pulled host-shaped trees into the image (containers#28334). Use relative names under the primary context for extra sources and do not apply `.dockerignore` to those forced entries. Fixes: containers#25314 Fixes: containers#28334 Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Honny1
force-pushed
the
fix-remote-build
branch
from
c7b1275 to
7e29609
Compare
|
@Honny1 I think you may have some real errors here. |
|
Yes, |
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Honny1
force-pushed
the
fix-remote-build
branch
from
7e29609 to
765ffb9
Compare
|
PTAL @containers/podman-maintainers |
|
LGTM |
|
@containers/podman-maintainers PTAL |
|
@Luap99 Can you please do a final review and merge? |
|
/cherry-pick v5.8 |
5 participants
When
podman-remotetars the context, extrapodman-build-secret*paths were either dropped by.dockerignore(#25314) or archived as absolute paths soCOPY . .pulled host-shaped trees into the image (#28334).Use relative names under the primary context for extra sources and do not apply
.dockerignoreto those forced entries.Fixes: #25314
Fixes: #28334
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?