-
Notifications
You must be signed in to change notification settings - Fork 141
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
*: remove RHEL6 hack and loosen capability validation #359
base: master
Are you sure you want to change the base?
*: remove RHEL6 hack and loosen capability validation #359
Conversation
5c1e0ab
to
cc52997
Compare
generate/generate.go
Outdated
// This is an exact copy of "validate/validate.go".lastCap. | ||
func lastCap() capability.Cap { | ||
last := capability.CAP_LAST_CAP | ||
// hack for RHEL6 which has no /proc/sys/kernel/cap_last_cap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly only some versions don't have it. That was back-ported at some point.
I have moved them into common util package in #344 |
@Mashimiao Do you want to rebase that PR? In addition, this PR also removes the "validity" checks from |
On Tue, Apr 11, 2017 at 07:09:55AM -0700, Aleksa Sarai wrote:
In addition, this PR also removes the "validity" checks from
`generate` because generate really shouldn't be checking the
validity of it's arguments.
Why not? If you do:
$ oci-runtime-tool generate --cap-add does-not-exist
capability DOES-NOT-EXIST must start with CAP_
I think the error message is helpful. And in the more-specific case:
$ oci-runtime-tool generate --cap-add cap_does_not_exist
Invalid capability: CAP_DOES_NOT_EXIST
I think the error message is still helpful. You only get the
host-specific validation if you explicitly ask for it:
$ oci-runtime-tool --host-specific generate --cap-add cap_audit_read
…success, because my host has CAP_AUDIT_READ…
So what is the use-case for “I want to set a known-invalid
capability”?
|
As far as I'm aware the spec doesn't disallow you from defining your own "capabilities" that are not Linux capabilities. I could imagine that some cloud provider might require "capabilities" in order for you to access certain files or resources. Sure, there's no reason that we must not check the validity of arguments we are provided, but the reason why generate is separate from validate is so that generation of a config is not conflated with its validation. At least, as a user that's what I would expect. |
On Tue, Apr 11, 2017 at 10:08:29AM -0700, Aleksa Sarai wrote:
> So what is the use-case for “I want to set a known-invalid
> capability”?
As far as I'm aware the spec doesn't disallow you from defining your
own "capabilities" that are not Linux capabilities.
I agree that the spec is weak on this point [1]. I think the… uh…
“right” approach to that is to completely drop capability value
validation, and I've filed opencontainers/runtime-spec#766 as part of
that. But keeping these checks in validation and only removing them
from generation does not seem like a consistent position. Either we
can validate the cap values (in which case I think we want to validate
them in both places) or we can't (in which case I think we want to
validate them in neither place).
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0-rc5/config.md#L135-L136
|
cc52997
to
3d20a6a
Compare
Since opencontainers/runtime-spec#766 was merged, now the strings are completely arbitrary and there's no need to do any validation. I'll rebase and remove |
The RHEL6 hack for CAP_LAST_CAP was causing us some annoyances, with the inter-dependency of generate and validate only existing because of CapValid (which then resulted in a bunch of build-time dependencies that were never used by projects that vendored us). To fix this issue, drop CapValid entirely so we don't have to touch it anymore -- just assume that CAP_LAST_CAP works on all systems. And in the case of validation we match new changes in the spec where capabilities are now just plain strings (but for the HostSpecific case we still do validation). Signed-off-by: Aleksa Sarai <[email protected]>
3d20a6a
to
24e0035
Compare
The RHEL6 hack for CAP_LAST_CAP was causing us some annoyances, with the
inter-dependency of generate and validate only existing because of
CapValid (which then resulted in a bunch of build-time dependencies that
were never used by projects that vendored us).
To fix this issue, drop CapValid entirely so we don't have to touch it
anymore -- just assume that CAP_LAST_CAP works on all systems. And in
the case of validation we match new changes in the spec where
capabilities are now just plain strings (but for the HostSpecific case
we still do validation).
Fixes: 1a899a6 ("validate: optimize capabilites check")
Signed-off-by: Aleksa Sarai [email protected]