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.

Sign up for GitHub

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

Jump to bottom

remote,API: fix implementation of build with --userns=auto for API and remote use-cases. #15477

Merged
merged 1 commit into from
Aug 26, 2022
Merged

remote,API: fix implementation of build with --userns=auto for API and remote use-cases. #15477

merged 1 commit into from
Aug 26, 2022

Conversation

flouthoc
Copy link
Collaborator

podman-remote and Libpod API does not supports build with
--userns=auto since IDMappingOptions were not implemented for API
and bindings, following PR implements passing IDMappingOptions via
bindings to API.

Closes: #15476

remote,api: build with --userns=auto now works for API and remote use-cases

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 25, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2022
@flouthoc flouthoc mentioned this pull request Aug 25, 2022
Closed
@flouthoc flouthoc force-pushed the remote-build-idmappings branch from 35bd2da to dbac6d7 Compare August 25, 2022 07:49
@flouthoc
Copy link
Collaborator Author

Cirrus looks down i can't see the logs, its just showing No logs available!. Not sure if its cirrus or test actually failed now.

@flouthoc flouthoc force-pushed the remote-build-idmappings branch from dbac6d7 to 57a7fad Compare August 25, 2022 09:15
@flouthoc
Copy link
Collaborator Author

All the tests pass except packit:production-build:* I don't think its related to this PR and is failing for other PR's as well.

vrothberg
vrothberg reviewed Aug 25, 2022
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM
just one question on the test

mheon
mheon reviewed Aug 25, 2022
View reviewed changes
pkg/api/handlers/compat/images_build.go
@@ -101,6 +101,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
ForceRm bool `schema:"forcerm"`
From string `schema:"from"`
HTTPProxy bool `schema:"httpproxy"`
IDMappingOptions string `schema:"idmappingoptions"`
Copy link
Member

Choose a reason for hiding this comment

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

Don't the API docs need to be updated to include this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lot of build options are already hidden from swagger and only common ones are there, I think this is too complex to expose since it contains nested fields and might not be used too frequently by regular users. But if everybody agrees we can expose this, however there are other options which i think deserve more priority than this if we want to expose hidden options.

Again I am cool if everybody agrees that we should expose this.

@flouthoc
Copy link
Collaborator Author

@vrothberg @giuseppe @mheon @containers/podman-maintainers PTAL i have addressed the comments, let me know if anyone of them is a blocker.

@flouthoc flouthoc requested a review from rhatdan August 26, 2022 09:23
@flouthoc flouthoc force-pushed the remote-build-idmappings branch 4 times, most recently from 630b8b2 to f155bbf Compare August 26, 2022 11:09
@flouthoc
remote: fix implementation of build with --userns=auto for API
e00272c 
`podman-remote` and Libpod API does not supports build with
`--userns=auto` since `IDMappingOptions` were not implemented for API
and bindings, following PR implements passing `IDMappingOptions` via
bindings to API.

Closes: containers#15476

Signed-off-by: Aditya R <[email protected]>
@flouthoc flouthoc force-pushed the remote-build-idmappings branch from f155bbf to e00272c Compare August 26, 2022 11:24
vrothberg
vrothberg reviewed Aug 26, 2022
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2022
@mheon
Copy link
Member

mheon commented Aug 26, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2022
@openshift-merge-robot openshift-merge-robot merged commit 0c028cd into containers:main Aug 26, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vrothberg vrothberg vrothberg left review comments

@mheon mheon Awaiting requested review from mheon

@giuseppe giuseppe Awaiting requested review from giuseppe

@rhatdan rhatdan Awaiting requested review from rhatdan

Assignees

@vrothberg vrothberg

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

build from API or podman-remote build does not supports --userns=auto while it works with CLI
6 participants
@flouthoc @mheon @giuseppe @rhatdan @vrothberg @openshift-merge-robot