-
Notifications
You must be signed in to change notification settings - Fork 787
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
COPY does not expand ENV nor ARG variables in --chown #2192
COPY does not expand ENV nor ARG variables in --chown #2192
Comments
This works for me with the latest
|
Maybe I am seeing a different root cause, but technically speaking, the expansion still seems broken (even when using the latest master).
I even tried setting |
Funny thing ... this actually seems to work with an older Buildah :D, so it might very well have seemed working when you originally closed this.
The problem is, there it seems to expand the variable to an empty string... (I've added the
So, I've added an extra
|
So, to sum up: With 1.14.3, With 1.15.0-dev, nothing's expanded, and it always blows up, no matter whether one uses |
@petr-motejlek the way ARG variables in Dockerfiles are handled changed recently. In short, if the ARG statement is before the FROM statement, then it is no longer used after the FROM statement unless it is reestablished after the FROM statement. i.e.
The above will now end up with a file in the container named To make this "work", you now need to adjust the Dockerfile slightly and establish the ARG variable again after the FROM statement. You don't need to reset the ARG.
Does this fit your scenario? We also had issues prior with environment variables in a |
Hey, Tom.
When I started using Docker heavily about 2 years back, it had already been doing that, so that's not recent in my eyes :D. However, how does this relate to the issue I am reporting, though? My original Dockerfile's do indeed use "this" (having a global ARG before FROM and then "pulling" that ARG into the FROM), but the reproduction scenario above has FROM first, then ARG, so the feature you describe does not seem to apply. Or, did I miss anything? |
@petr-motejlek let me re-look. I could have sworn your Dockerfile had an ARG statement at the top, I think my covid19 bunkering has caused me to lose a step. |
@petr-motejlek first off, thx for the re-ping. I'm also seeing the chown command not using the username as it should and I've verified that it does work on older versions and Docker for that matter. I'm not sure what the issue is, but something is off for sure. |
Any chance this could actually be re-opened, as futher investigation is happening? |
Yes, I'll reopen |
OK, I don't have a root cause yet, but I've found the issue. The problem happens if you use the We recently had a fix for values set by the |
OK, I think I've found the issue down in openshift/builder which we use to do the Dockerfile processing. I've got a rough fix put together, will clean it up and get it out for review tomorrow. The root of the issue is the code looks through the declared environment variables via the |
If the Dockerfile has a `ARG CHOWN_VAL=6173:6173`, then a `COPY --chown=${CHOWN_VAL} srcFile destFile` will fail. However if the Dockerfile has a `ENV CHOWN_VAL=6173:6173` then it succeeds. The dispatchCopy() function was only going through the list of Environment variables to resolve a Variable instead of the list of Args and Environment variables. If there's a trailing equal `=` sign in the arg, assume that we'd a variable that did not resolve and try to resolve from the list of args. At some point I'd like to look into converting the b.Env to a Map from a slice like b.Args, but didn't want to run this down now. Fixes: containers/buildah#2192 and probably containers/buildah#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
If the Dockerfile has a `ARG CHOWN_VAL=6173:6173`, then a `COPY --chown=${CHOWN_VAL} srcFile destFile` will fail. However if the Dockerfile has a `ENV CHOWN_VAL=6173:6173` then it succeeds. The dispatchCopy() and add() functions were only going through the list of Environment variables to resolve a Variable instead of the list of Args and Environment variables. If there's a trailing equal `=` sign in the arg, assume that we'd a variable that did not resolve and try to resolve from the list of args. At some point I'd like to look into converting the b.Env to a Map from a slice like b.Args, but didn't want to run this down now. Fixes: containers/buildah#2192 and probably containers/buildah#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
If the Dockerfile has a `ARG CHOWN_VAL=6173:6173`, then a `COPY --chown=${CHOWN_VAL} srcFile destFile` will fail. However if the Dockerfile has a `ENV CHOWN_VAL=6173:6173` then it succeeds. The dispatchCopy() and add() functions were only going through the list of Environment variables to resolve a Variable instead of the list of Args and Environment variables. Now add the definitions from the ARG statements to the ones from the ENV statements and use both when resolving variables. At some point I'd like to look into converting the b.Env to a Map from a slice like b.Args, but didn't want to run this down now. Fixes: containers/buildah#2192 and probably containers/buildah#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
If the Dockerfile has a `ARG CHOWN_VAL=6173:6173`, then a `COPY --chown=${CHOWN_VAL} srcFile destFile` will fail. However if the Dockerfile has a `ENV CHOWN_VAL=6173:6173` then it succeeds. The dispatchCopy() and add() functions were only going through the list of Environment variables to resolve a Variable instead of the list of Args and Environment variables. Now add the definitions from the ARG statements to the ones from the ENV statements and use both when resolving variables. At some point I'd like to look into converting the b.Env to a Map from a slice like b.Args, but didn't want to run this down now. Fixes: containers/buildah#2192 and probably containers/buildah#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
If the Dockerfile has a `ARG CHOWN_VAL=6173:6173`, then a `COPY --chown=${CHOWN_VAL} srcFile destFile` will fail. However if the Dockerfile has a `ENV CHOWN_VAL=6173:6173` then it succeeds. The dispatchCopy() and add() functions were only going through the list of Environment variables to resolve a Variable instead of the list of Args and Environment variables. Now add the definitions from the ARG statements to the ones from the ENV statements and use both when resolving variables. At some point I'd like to look into converting the b.Env to a Map from a slice like b.Args, but didn't want to run this down now. Fixes: containers/buildah#2192 and probably containers/buildah#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
Bump imagebuilder to the latest upstream to get the ARG fix from openshift/imagebuilder#159 Also touched up the install.md with vendoring from a commitId so I won't have to run all around looking for it next time. Fixes: containers#2192 and probably containers#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
Bump imagebuilder to the latest upstream to get the ARG fix from openshift/imagebuilder#159 Also touched up the install.md with vendoring from a commitId so I won't have to run all around looking for it next time. Fixes: containers#2192 and probably containers#2345 Signed-off-by: TomSweeneyRedHat <[email protected]>
2373: Bump imagebuilder for ARG fix r=giuseppe a=TomSweeneyRedHat Bump imagebuilder to the latest upstream to get the ARG fix from openshift/imagebuilder#159 Also touched up the install.md with vendoring from a commitId so I won't have to run all around looking for it next time. Fixes: #2192 and probably #2345 Signed-off-by: TomSweeneyRedHat <[email protected]> <!-- Thanks for sending a pull request! Please make sure you've read and understood our contributing guidelines (https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring that all your commits are signed with `git commit -s`. --> #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind api-change > /kind bug > /kind cleanup > /kind deprecation > /kind design > /kind documentation > /kind failing-test > /kind feature > /kind flake > /kind other #### What this PR does / why we need it: #### How to verify it #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: TomSweeneyRedHat <[email protected]>
2373: Bump imagebuilder for ARG fix r=giuseppe a=TomSweeneyRedHat Bump imagebuilder to the latest upstream to get the ARG fix from openshift/imagebuilder#159 Also touched up the install.md with vendoring from a commitId so I won't have to run all around looking for it next time. Fixes: #2192 and probably #2345 Signed-off-by: TomSweeneyRedHat <[email protected]> <!-- Thanks for sending a pull request! Please make sure you've read and understood our contributing guidelines (https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring that all your commits are signed with `git commit -s`. --> #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind api-change > /kind bug > /kind cleanup > /kind deprecation > /kind design > /kind documentation > /kind failing-test > /kind feature > /kind flake > /kind other #### What this PR does / why we need it: #### How to verify it #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: TomSweeneyRedHat <[email protected]>
2373: Bump imagebuilder for ARG fix r=giuseppe a=TomSweeneyRedHat Bump imagebuilder to the latest upstream to get the ARG fix from openshift/imagebuilder#159 Also touched up the install.md with vendoring from a commitId so I won't have to run all around looking for it next time. Fixes: #2192 and probably #2345 Signed-off-by: TomSweeneyRedHat <[email protected]> <!-- Thanks for sending a pull request! Please make sure you've read and understood our contributing guidelines (https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring that all your commits are signed with `git commit -s`. --> #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind api-change > /kind bug > /kind cleanup > /kind deprecation > /kind design > /kind documentation > /kind failing-test > /kind feature > /kind flake > /kind other #### What this PR does / why we need it: #### How to verify it #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: TomSweeneyRedHat <[email protected]>
We recently made a couple of commits to the imagebuilder project to address containers#2345 and containers#2192. Tests were created in that project, but I thought it best to add some regression tests here in Buildah too. FYI: @petr-motejlek and @tachoknight Signed-off-by: TomSweeneyRedHat <[email protected]>
We recently made a couple of commits to the imagebuilder project to address containers#2345 and containers#2192. Tests were created in that project, but I thought it best to add some regression tests here in Buildah too. FYI: @petr-motejlek and @tachoknight Signed-off-by: TomSweeneyRedHat <[email protected]>
2418: buildah, bud: support --jobs=N for parallel execution r=rhatdan a=giuseppe <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> /kind feature #### What this PR does / why we need it: it enables running multi stages Containerfiles in parallel #### How to verify it buildah bud --jobs=$(nproc) ..... #### Which issue(s) this PR fixes: None #### Special notes for your reviewer: #### Does this PR introduce a user-facing change? ```release-note add support for running multi stages Containerfiles in parallel ``` 2422: Add bud regression tests r=rhatdan a=TomSweeneyRedHat We recently made a couple of commits to the imagebuilder project to address #2345 and #2192. Tests were created in that project, but I thought it best to add some regression tests here in Buildah too. FYI: @petr-motejlek and @tachoknight Signed-off-by: TomSweeneyRedHat <[email protected]> <!-- Thanks for sending a pull request! Please make sure you've read and understood our contributing guidelines (https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring that all your commits are signed with `git commit -s`. --> #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind api-change > /kind bug > /kind cleanup > /kind deprecation > /kind design > /kind documentation > /kind failing-test > /kind feature > /kind flake > /kind other #### What this PR does / why we need it: #### How to verify it #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: Giuseppe Scrivano <[email protected]> Co-authored-by: TomSweeneyRedHat <[email protected]>
2422: Add bud regression tests r=rhatdan a=TomSweeneyRedHat We recently made a couple of commits to the imagebuilder project to address #2345 and #2192. Tests were created in that project, but I thought it best to add some regression tests here in Buildah too. FYI: @petr-motejlek and @tachoknight Signed-off-by: TomSweeneyRedHat <[email protected]> <!-- Thanks for sending a pull request! Please make sure you've read and understood our contributing guidelines (https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring that all your commits are signed with `git commit -s`. --> #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind api-change > /kind bug > /kind cleanup > /kind deprecation > /kind design > /kind documentation > /kind failing-test > /kind feature > /kind flake > /kind other #### What this PR does / why we need it: #### How to verify it #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: TomSweeneyRedHat <[email protected]>
What version has the fix for this? |
I believe V1.16.0 and later, from scanning through the git log. |
Actually made Buildah v1.15.0, so definiely v1.16 and beyond. |
Description
I have a Dockerfile that appears to do what I expect it to without raising any errors. Buildah is unable to do the same.
It would appear that the
--chown
parameter for theCOPY
instrcution is not expanded (to its corresponding ENV or ARG value) in Buildah, but it is expanded in regulardocker build
(and actually also in BuildKit builds).Steps to reproduce the issue:
docker build .
Describe the results you received:
I received very bad results :).
Describe the results you expected:
I expected buildah to actually succeed at creating the image. I am not sure how, if at all, this is related to #1957 or #1994
Output of
rpm -q buildah
orapt list buildah
:Output of
buildah version
:Output of
cat /etc/*release
:Output of
uname -a
:Output of
cat /etc/containers/storage.conf
:(I did not touch this file)
The text was updated successfully, but these errors were encountered: