Skip to content

Conversation

@gsilva00
Copy link

@gsilva00 gsilva00 commented Nov 3, 2025

Closes #526.

With the proposed changes it is now possible to pass secrets into the build context in a way that they won't be stored in the final image (equivalent to podman build --secret ...). They are temporarily mounted during the build process, in Containerfile RUN --mount=type=secret,id=<some_id> instructions, where the <some_id> matches the id specified in the corresponding --secret option of the podman build command.

Changes:

  • Mapped the --secrets argument into the expected manifest query parameter when making requests;
  • Updated documentation for the client.images.build function to include the added field. Description text extracted from the Podman CLI itself (with some additional info about the possibility of passing env variables)
  • Updated existing unit tests to ensure that --secret argument is mapped into the expected query parameter;
  • Implemented new integration test to ensure that a secret file is correctly passed and used by the build context, as well as testing that the file does not remain in the built image;

Signed-off-by: Guilherme Silva <gmpas4444@gmail.com>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 3, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gsilva00
Once this PR has been reviewed and has the lgtm label, please assign lsm5 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gsilva00
Copy link
Author

gsilva00 commented Nov 3, 2025

Hi,

Now that most pipeline checks have finished, I'm stumped on the results of the failing ones:

  • The pre-commit check is strange. I installed its hooks and it passed locally (otherwise, I wouldn't have been able to commit), and I also ran it manually. The test attribute in /tests/coverage_integration also seems to be set, despite the error message.
  • The integration test fails in not raising an exception when the secret file is not found. However, when I run it locally, it raises the exception as expected. Given that ls /non_existent_file should always fail and return a non-zero code, I'm not sure why it passes in the pipeline.

Looking forward to your feedback!

EDIT: I've also noticed that I left a temporary assert in the integration test, that I should've replaced with a proper unittest assert. However, I thought it best to wait on pipeline feedback to not clutter the PR with very small commits, nor trigger it multiple times.

@inknos
Copy link
Contributor

inknos commented Nov 3, 2025

The pre-commit check is strange. I installed its hooks and it passed locally (otherwise, I wouldn't have been able to commit), and I also ran it manually. The test attribute in /tests/coverage_integration also seems to be set, despite the error message.

Weird indeed. Locally, it's a green check for me

The integration test fails in not raising an exception when the secret file is not found. However, when I run it locally, it raises the exception as expected. Given that ls /non_existent_file should always fail and return a non-zero code, I'm not sure why it passes in the pipeline.

this however fails, consistently with CI.

I'll take a quick look now to see if I catch the issue, and if not I'll come back tomorrow with a fresh mind

@inknos
Copy link
Contributor

inknos commented Nov 3, 2025

update:

podman run -it --rm -v $(pwd):/devel:Z fedora:43
cd /devel
dnf install -y pre-commit
pre-commit run -a

reproduces the behavior of github.

The python env for that action (3.14) follows latest fedora (43) and probably tmt run into some issues since it's not up to date. the version in .pre-commit-config.yaml should be bumped to 1.60.0, and that fixes the issue

Edit: typo

Signed-off-by: Guilherme Silva <gmpas4444@gmail.com>
Signed-off-by: Guilherme Silva <gmpas4444@gmail.com>
@gsilva00
Copy link
Author

gsilva00 commented Nov 5, 2025

Hi @inknos ,

About ContainerError not being raised for ls /run/secrets

I ran the tests on the same OSes as the testing farms (not exactly their setup, but the error showed up nonetheless).

It happened because the /run/secrets dir inside the built images isn't deleted after the image is built, so the ls command succeeds.
That's different from the behavior on my host. On my host, that folder is deleted, so the ls command fails as expected.
Still, the secret file is not present, so altering the test to check for the file presence instead of the folder presence worked on all tested OSes. It also makes the code more meaningful, as the important part is the file, not necessarily the folder.

I don't understand why this happens:

  • Both the host and the testing farms are using the same Podman API endpoint version (the default - v5.6.0).
  • I would think this difference in build behavior is the responsability of buildah, which AFAIK is the one handling the builds under the hood. So I went looking for differences related to this:
    • Some testing farms have older podman versions (v5.6.0) than my host (v5.6.2).
    • The behaviour could be different thanks to that (given that the API calls use the underlying host's installation, as the tests are starting a podman service connecting on a local unix socket, AFAIK).
    • I checked the podman releases and their respective buildah releases (between v5.6.0 and v5.6.2), but I couldn't find any mention of build-time secret mounts or changes to handling of temporary files/folders during builds.
    • However, other testing farms have a higher podman versions and it still fails, so I doesn't seem to be related to that.

I didn't explore further than this, but would like to understand the root cause of this issue better.

In any case, the test should now pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Pass --secret option when building an image

2 participants