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

Add support for Caps Options. #17

Merged
merged 1 commit into from
Nov 5, 2017
Merged

Add support for Caps Options. #17

merged 1 commit into from
Nov 5, 2017

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Nov 3, 2017

Still has security options handling, which should be merged first

@mheon
Copy link
Member

mheon commented Nov 3, 2017

Vendor grabbed too much here. Don't want to update kube and CNI vendors with this. Can you redo the vendor and commit only what we actually need?

@rhatdan
Copy link
Member Author

rhatdan commented Nov 3, 2017

mheon, I think we could actually drop some of these vendors. But I can do that.

@rhatdan
Copy link
Member Author

rhatdan commented Nov 3, 2017

BTW This is working. :^) Very Cool.

@mheon
Copy link
Member

mheon commented Nov 3, 2017

@rhatdan I think we should hold off until libkpod disappears. We can probably drop half our vendors then

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably 098389d) made this pull request unmergeable. Please resolve the merge conflicts.

@rhatdan rhatdan changed the title [WIP] Add support for Caps Options. Add support for Caps Options. Nov 3, 2017
@rhatdan
Copy link
Member Author

rhatdan commented Nov 3, 2017

@mheon @baude @umohnani8 PTAL

@mheon
Copy link
Member

mheon commented Nov 3, 2017

LGTM pending tests

configSpec.Process.Capabilities.Bounding = caplist
configSpec.Process.Capabilities.Permitted = caplist
configSpec.Process.Capabilities.Inheritable = caplist
configSpec.Process.Capabilities.Effective = caplist
Copy link
Member

Choose a reason for hiding this comment

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

Would you consider plopping this into a function that returned a spec.Process.Capablilties type? I did that for some of the larger things like volumes and keeps this clean

Copy link
Member

Choose a reason for hiding this comment

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

oh, and if you do, some unittests would be really handy. you could come back around this, because I have a PR that initiates the tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@rhatdan rhatdan force-pushed the caps branch 4 times, most recently from 4b325d6 to a02b2f2 Compare November 4, 2017 08:00
Had to revendor in docker/docker again, which dropped a bunch of packages

Signed-off-by: Daniel J Walsh <[email protected]>
@baude
Copy link
Member

baude commented Nov 5, 2017

@rhatdan ready for merge ?

@rhatdan
Copy link
Member Author

rhatdan commented Nov 5, 2017

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 619637a has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 619637a with merge 5d94d81...

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@rhatdan
Copy link
Member Author

rhatdan commented Nov 5, 2017

No idea what the homu test is about?

@rhatdan
Copy link
Member Author

rhatdan commented Nov 5, 2017

I don't see anything failing, so I am going to merge.

@rhatdan rhatdan merged commit dbd524e into containers:master Nov 5, 2017
wking added a commit to wking/libpod that referenced this pull request May 11, 2018
This directory just had Markdown and vendor.conf.  I'm not sure why we
have it in our version control, maybe old versions of vndr kept it?
Or maybe folk dropped it into vendor/ by hand without using vndr?  The
history of that vendored directory is:

* 619637a (Handle Linux Capabilities from command line, 2017-11-03,
  containers#17) added the three files to our version control.
* c344fe6 (Update vendoring, 2017-11-22, containers#60) bumped hack/README.md.
* af64e10 (Vendor in lots of kubernetes stuff to shrink image size,
  2018-03-26, containers#554) bumped hack/README.md.
* 27107fd (Vendor in latest containers/image and contaners/storage,
  2018-04-18, containers#509) removed the files.
* a824186 (Use buildah commit and bud in podman, 2018-04-25, containers#681)
  added the files back.
* I'm removing them again in this commit.

With this commit,

  $ vndr github.com/docker/docker

becomes a no-op.

Signed-off-by: W. Trevor King <[email protected]>
rh-atomic-bot pushed a commit that referenced this pull request May 11, 2018
This directory just had Markdown and vendor.conf.  I'm not sure why we
have it in our version control, maybe old versions of vndr kept it?
Or maybe folk dropped it into vendor/ by hand without using vndr?  The
history of that vendored directory is:

* 619637a (Handle Linux Capabilities from command line, 2017-11-03,
  #17) added the three files to our version control.
* c344fe6 (Update vendoring, 2017-11-22, #60) bumped hack/README.md.
* af64e10 (Vendor in lots of kubernetes stuff to shrink image size,
  2018-03-26, #554) bumped hack/README.md.
* 27107fd (Vendor in latest containers/image and contaners/storage,
  2018-04-18, #509) removed the files.
* a824186 (Use buildah commit and bud in podman, 2018-04-25, #681)
  added the files back.
* I'm removing them again in this commit.

With this commit,

  $ vndr github.com/docker/docker

becomes a no-op.

Signed-off-by: W. Trevor King <[email protected]>

Closes: #752
Approved by: baude
baude referenced this pull request in baude/podman Aug 31, 2019
pkg/ns: use unix.Setns() instead of open coding it
jwhonce referenced this pull request in jwhonce/podman Nov 8, 2019
pod create and container create
@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 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants