config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #835

wking · 2017-05-18T06:17:35Z

This is the alternative to the recently-closed #810.

Roll back the genericization from #673. Lifting the restriction there seems to have been motivated by “Solaris supports capabilities”, but that was before the split into a capabilities object which happened in #675. It's not clear if Solaris supports ambient caps, or what Solaris API rlimits or noNewPrivileges were punting to. @jhowardmsft has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future. John's statement didn't directly address rlimits or noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms.

Also add some backticks, remove the hyphens in OPTIONAL) - the, standardize lines I touch to use “the process”, and use four-space indents here to keep Pandoc happy (see #495).

This PR will conflict with #809, and I'm happy to rebase if that one lands first.

dqminh · 2017-05-23T14:43:13Z

Looks conflicted with https://github.com/opencontainers/runtime-spec/pull/845/files ( rlimits is not linux-specific )

The changes to capabilities and nonewprivileges looks sane though.

wking · 2017-05-23T16:12:27Z

On Tue, May 23, 2017 at 07:43:14AM -0700, Daniel, Dao Quang Minh wrote: Looks conflicted with https://github.com/opencontainers/runtime-spec/pull/845/files ( rlimits is not linux-specific )

Do you have Solaris and/or Windows docs for what API they'll use to implement rlimits? #845 looks like a subset of the rejected #810, although as I mentioned in the topic post for this PR, I couldn't find anyone speaking directly to rlimits support in Windows/Solaris.

lowenna · 2017-05-23T16:23:34Z

Do you have Solaris and/or Windows docs for what API they'll use to
implement rlimits?

rlimits are meaningless on Windows certainly.

wking · 2017-05-23T16:36:23Z

On Tue, May 23, 2017 at 09:23:37AM -0700, John Howard wrote: > Do you have Solaris and/or Windows docs for what API they'll use to > implement rlimits? `rlimits` are meaningless on Windows certainly.

So I can reroll this to make rlimits Linux or Solaris, and just punt to POSIX for the Solaris values until we hear from some Solaris person with Solaris-specific docs?

lowenna · 2017-05-23T16:37:39Z

I can only comment on the windows part, but makes sense to me....

Roll back the genericization from 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673). Lifting the restriction there seems to have been motivated by "Solaris supports capabilities", but that was before the split into a capabilities object which happened in eb114f0 (Add ambient and bounding capability support, 2017-02-02, opencontainers#675). It's not clear if Solaris supports ambient caps, or what Solaris API noNewPrivileges were punting to [1]. And John Howard has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future [2]. He also confirmed that Windows does not support rlimits [3]. John's statement didn't directly address noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms. Also add some backticks, remove the hyphens in "OPTIONAL) - the", standardize lines I touch to use "the process" [4], and use four-space indents here to keep Pandoc happy (see 7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495). [1]: opencontainers#673 (comment) [2]: opencontainers#810 (comment) [3]: opencontainers#835 (comment) [4]: opencontainers#809 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

This property was initially Linux-specific. 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) removed the Linux restriction, but the rlimit concept is from POSIX and Windows doesn't support it [1]. This commit adds new subsections for the POSIX-specific and Linux-specific process entries (to match the approach we currently use for process.user), and punts to POSIX for the Solaris values and compliance testing approach. If/when we get a Solaris-specific doc for valid values, we can replace the POSIX punt there, but we probably want to continue punting to POSIX for getrlimit(3)-based compliance testing. [1]: opencontainers#835 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

This property was initially Linux-specific. 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) removed the Linux restriction, but the rlimit concept is from POSIX and Windows doesn't support it [1]. This commit adds new subsections for the POSIX-specific and Linux-specific process entries (to match the approach we currently use for process.user), and punts to POSIX for the Solaris values and compliance testing approach. If/when we get a Solaris-specific doc for valid values, we can replace the POSIX punt there, but we probably want to continue punting to POSIX for getrlimit(3)-based compliance testing. I've renamed the overly-specific LinuxRlimit to POSIXRlimit. We could use the generic Rlimit, but then we'd be stuck if/when Windows adds support for some rlimit-like thing that doesn't match up cleanly enough for us to use the POSIX structure. [1]: opencontainers#835 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

wking · 2017-05-23T17:44:36Z

Ok, I've rerolled this with 550db9f → b4d7fae, which makes rlimits Solaris/Linux-specific, adjusting from master where rlimits is also nominally legal on Windows.

It also renames the Go LinuxRlimit to POSIXRlimit, although I'm fine leaving the type LinuxRlimit or following #845 and using the generic Rlimit (since Windows will hopefully use similar semantics if they add a new feature and decide to call it “rlimit”).

When adding the POSIX link for rlimits, I also added wording for RFC 2119 compliance testing, styled after the approach I'd floated for devices in #829 and some wording we already use for valid capabilities values.

If we want to focus on just the capabilities change, I'm happy to spin off the first commit into its own PR, and we can get back to this PR and the rlimits change if/when that cap PR lands.

wking · 2017-05-25T18:00:37Z

This is a breaking change for Solaris and Windows (removing process.capabilities for those configs) and allows Windows runtimes (which have no API to implement the currently-required-in-master-for-them process.capabilities) and Solaris runtimes (which may not have an API to implement the currently-required-in-master-for-them process.capabilities.ambient) to be certified compliant. I think that's sufficient grounds to milestone it 1.0 freeze [1] unless we preview those platforms [2]. [1]: https://github.com/opencontainers/runtime-spec/milestone/15 [2]: #817 (comment)

lowenna · 2017-05-25T18:04:27Z

specs-go/config.go

-// LinuxRlimit type and restrictions
-type LinuxRlimit struct {
+// POSIXRlimit type and restrictions
+type POSIXRlimit struct {


Wondering if this should be moved out of the bunch of Linux structures which surround it so that there are POSIX, Linux, Solaris and Windows sections grouped together. Otherwise LGTM (not a maintainer)

Wondering if this should be moved out of the bunch of Linux structures which surround…

I don't care and am happy to do whatever maintainers want for type ordering.

Roll back the genericization from 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673). Lifting the restriction there seems to have been motivated by "Solaris supports capabilities", but that was before the split into a capabilities object which happened in eb114f0 (Add ambient and bounding capability support, 2017-02-02, opencontainers#675). It's not clear if Solaris supports ambient caps, or what Solaris API noNewPrivileges were punting to [1]. And John Howard has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future [2]. He also confirmed that Windows does not support rlimits [3]. John's statement didn't directly address noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms. Also add some backticks, remove the hyphens in "OPTIONAL) - the", standardize lines I touch to use "the process" [4], and use four-space indents here to keep Pandoc happy (see 7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495). [1]: opencontainers#673 (comment) [2]: opencontainers#810 (comment) [3]: opencontainers#835 (comment) [4]: opencontainers#809 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

This property was initially Linux-specific. 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) removed the Linux restriction, but the rlimit concept is from POSIX and Windows doesn't support it [1]. This commit adds new subsections for the POSIX-specific and Linux-specific process entries (to match the approach we currently use for process.user), and punts to POSIX for the Solaris values and compliance testing approach. If/when we get a Solaris-specific doc for valid values, we can replace the POSIX punt there, but we probably want to continue punting to POSIX for getrlimit(3)-based compliance testing. I've renamed the overly-specific LinuxRlimit to POSIXRlimit. We could use the generic Rlimit, but then we'd be stuck if/when Windows adds support for some rlimit-like thing that doesn't match up cleanly enough for us to use the POSIX structure. [1]: opencontainers#835 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

wking · 2017-06-01T15:09:17Z

Rebased around #846 with b4d7fae → cb8df7b.

crosbymichael · 2017-06-01T18:18:42Z

We should make this change less about linux && solaris and more about unix vs windows as most of the things we have generally apply to unixes and we only need to point out exceptions for windows

wking · 2017-06-01T22:41:37Z

We should make this change less about linux && solaris and more about unix vs windows…

I'll rebase this one after #838 lands, since I've covered the rest of the POSIX-ification there.

tianon · 2017-06-21T15:14:16Z

I agree with @crosbymichael -- using "POSIX" to mean "not Windows" is a bit off the mark, especially since Windows is largely POSIX-compliant in many areas (or at least has been in the past).

+1 for Unix or Unix-like instead

vbatts · 2017-06-29T17:33:43Z

@wking pls2rebase

vbatts · 2017-07-03T23:07:04Z

no idea when @wking will be back to rebase this. Wednesday the 5th is drawing close ...

dqminh · 2017-07-03T23:32:59Z

if @wking doesnt get to this tomorrow, i can carry this change.

Roll back the genericization from 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673). Lifting the restriction there seems to have been motivated by "Solaris supports capabilities", but that was before the split into a capabilities object which happened in eb114f0 (Add ambient and bounding capability support, 2017-02-02, opencontainers#675). It's not clear if Solaris supports ambient caps, or what Solaris API noNewPrivileges were punting to [1]. And John Howard has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future [2]. He also confirmed that Windows does not support rlimits [3]. John's statement didn't directly address noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms. Also add some backticks, remove the hyphens in "OPTIONAL) - the", standardize lines I touch to use "the process" [4], and use four-space indents here to keep Pandoc happy (see 7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495). [1]: opencontainers#673 (comment) [2]: opencontainers#810 (comment) [3]: opencontainers#835 (comment) [4]: opencontainers#809 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

This property was initially Linux-specific. 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) removed the Linux restriction, but the rlimit concept is from POSIX and Windows doesn't support it [1]. This commit adds new subsections for the POSIX-specific and Linux-specific process entries (to match the approach we currently use for process.user), and punts to POSIX for the Solaris values and compliance testing approach. If/when we get a Solaris-specific doc for valid values, we can replace the POSIX punt there, but we probably want to continue punting to POSIX for getrlimit(3)-based compliance testing. I've renamed the overly-specific LinuxRlimit to POSIXRlimit. We could use the generic Rlimit, but then we'd be stuck if/when Windows adds support for some rlimit-like thing that doesn't match up cleanly enough for us to use the POSIX structure. [1]: opencontainers#835 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

dqminh · 2017-07-05T10:33:14Z

I have opened #880 to replace this.

This property was initially Linux-specific. 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) removed the Linux restriction, but the rlimit concept is from POSIX and Windows doesn't support it [1]. This commit adds new subsections for the POSIX-specific and Linux-specific process entries (to match the approach we currently use for process.user), and punts to POSIX for the Solaris values and compliance testing approach. If/when we get a Solaris-specific doc for valid values, we can replace the POSIX punt there, but we probably want to continue punting to POSIX for getrlimit(3)-based compliance testing. I've renamed the overly-specific LinuxRlimit to POSIXRlimit. We could use the generic Rlimit, but then we'd be stuck if/when Windows adds support for some rlimit-like thing that doesn't match up cleanly enough for us to use the POSIX structure. [1]: opencontainers#835 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

This isn't a question of not being implemented yet. Windows does not, and is unlikely to ever, support rlimits [1]. And the spec only defines process.rlimits for POSIX platforms [2]. Folks writing Windows configs will only be setting process.rlimits as an out-of-spec extention. We, as runtime validators, are unlikely to write such configs. But if we use runtimetest in a container launched from such a config, we should silently ignore that out-of-spec extention, just like we silently ignore all other out-of-spec extentions, instead of logging a warning. [1]: opencontainers/runtime-spec#835 (comment) [2]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#posix-process Signed-off-by: W. Trevor King <wking@tremily.us>

wking mentioned this pull request May 23, 2017

specs-go: Rlimits is not Linux-specific #845

Closed

wking force-pushed the linux-only-capabilities-again branch from 550db9f to 15f5fd6 Compare May 23, 2017 17:34

wking force-pushed the linux-only-capabilities-again branch from 15f5fd6 to b4d7fae Compare May 23, 2017 17:39

lowenna reviewed May 25, 2017

View reviewed changes

wking added 2 commits June 1, 2017 08:05

wking force-pushed the linux-only-capabilities-again branch from b4d7fae to cb8df7b Compare June 1, 2017 15:09

mrunalp added this to the 1.0.0 milestone Jun 1, 2017

tianon mentioned this pull request Jun 21, 2017

config: Bump Hyper-V condition from root.path to root itself #838

Merged

dqminh mentioned this pull request Jul 5, 2017

rebase: config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #880

Merged

dqminh closed this Jul 5, 2017

wking deleted the linux-only-capabilities-again branch July 10, 2017 21:10

This was referenced Oct 10, 2017

validate: Soften unrecognized rlimit types to SHOULD violations opencontainers/runtime-tools#481

Merged

config: runtime SHOULD create mountpoints and cwd #928

Closed

wking mentioned this pull request Nov 3, 2017

runtimetest: Make validateRlimits silent on Windows opencontainers/runtime-tools#509

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #835

config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #835

wking commented May 18, 2017 •

edited

Loading

dqminh commented May 23, 2017

wking commented May 23, 2017 via email

lowenna commented May 23, 2017

wking commented May 23, 2017 via email

lowenna commented May 23, 2017

wking commented May 23, 2017 •

edited

Loading

wking commented May 25, 2017 via email

lowenna May 25, 2017

wking May 25, 2017

wking commented Jun 1, 2017

crosbymichael commented Jun 1, 2017

wking commented Jun 1, 2017

tianon commented Jun 21, 2017

vbatts commented Jun 29, 2017

vbatts commented Jul 3, 2017

dqminh commented Jul 3, 2017

dqminh commented Jul 5, 2017

config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #835

config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #835

Conversation

wking commented May 18, 2017 • edited Loading

dqminh commented May 23, 2017

wking commented May 23, 2017 via email

lowenna commented May 23, 2017

wking commented May 23, 2017 via email

lowenna commented May 23, 2017

wking commented May 23, 2017 • edited Loading

wking commented May 25, 2017 via email

lowenna May 25, 2017

Choose a reason for hiding this comment

wking May 25, 2017

Choose a reason for hiding this comment

wking commented Jun 1, 2017

crosbymichael commented Jun 1, 2017

wking commented Jun 1, 2017

tianon commented Jun 21, 2017

vbatts commented Jun 29, 2017

vbatts commented Jul 3, 2017

dqminh commented Jul 3, 2017

dqminh commented Jul 5, 2017

wking commented May 18, 2017 •

edited

Loading

wking commented May 23, 2017 •

edited

Loading