Skip to content
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

fix(KFLUXBUGS-1328): pull secrets in rpm-ostree #1054

Closed
wants to merge 1 commit into from

Conversation

scoheb
Copy link
Contributor

@scoheb scoheb commented Jun 5, 2024

  • copy functionality used in buildah-remote task

- copy functionality used in buildah-remote task

Signed-off-by: Scott Hebert <[email protected]>
@scoheb scoheb requested review from stuartwdouglas and chmeliik June 5, 2024 13:31
Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 👍

@@ -173,6 +174,7 @@ spec:
-e COMMIT_SHA="$COMMIT_SHA" \
--rm \
-v "$BUILD_DIR/workspaces/source:$(workspaces.source.path):Z" \
-v "$BUILD_DIR/.docker/:/root/.docker:Z" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this one, is it really necessary? It seems to expose the pull secret to the container itself, but I don't think we need that here FWIW.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch, this is a different use case than the buildah-remote task. This one wants to use the credentials to pull the BUILDER_IMAGE itself, and there's no need to pass the credentials to that image again

I think we need to rsync the docker credentials to the $HOME of the VM user

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to either sync the credentials to the $HOME/.docker dir, or explicitly tell podman which credentials file to use.

@chmeliik chmeliik self-requested a review June 5, 2024 14:12
@cgwalters
Copy link
Contributor

There's a more medium term thing here that I think we can actually kill the rpm-ostree task in favor of buildah-remote.

@@ -173,6 +174,7 @@ spec:
-e COMMIT_SHA="$COMMIT_SHA" \
--rm \
-v "$BUILD_DIR/workspaces/source:$(workspaces.source.path):Z" \
-v "$BUILD_DIR/.docker/:/root/.docker:Z" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to either sync the credentials to the $HOME/.docker dir, or explicitly tell podman which credentials file to use.

@lmilbaum
Copy link

This PR is a blocker for a downstream MR. Would you be able to prioritize this?

@chmeliik
Copy link
Contributor

looks abandoned, can we close this?

@scoheb
Copy link
Contributor Author

scoheb commented Aug 28, 2024

Yep. We can close.

@chmeliik chmeliik closed this Aug 28, 2024
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.

5 participants