mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 59552: Add support for explicitly setting bounding capabilities.
Date Fri, 09 Jun 2017 23:07:11 GMT


> On June 9, 2017, 9:28 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp
> > Lines 99-103 (patched)
> > <https://reviews.apache.org/r/59552/diff/2/?file=1741704#file1741704line101>
> >
> >     In fact, i think the semantics should be:
> >     
> >     1) treat framework specificied capabilities as effective capabilities.
> >     2) in the future, allow frameworks to set bounding capabilities
> >     3) if effective capabilities is set and bounding capabilities is not set, set
bounding set to be the same as effective set (instead of "unbounded"). This matches the existing
semantics.
> >     4) treat operator flags as the default values if the framework does not specify
them (for both bounding and effective capabilities).
> >     5) if both effective and bounding capabilities are set, make sure the effective
is a subset of bounding
> >     6) if effective capabilities is not set, but bounding capabilities is set, set
effective capabilities to be the same as bounding capabilities
> >     
> >     Given that, I'd adjust the logic of this function to the following:
> >     
> >     ```
> >     Option<CapabilityInfo> effective;
> >     Option<CapabilityInfo> bounding;
> >     
> >     if (containerConfig.has_container_info() &&
> >         containerConfig.container_info().has_linux_info() &&
> >         containerConfig.container_info().linux_info().has_capability_info()) {
> >       effective = containerConfig.container_info().linux_info().capability_info();
> >     }
> >     
> >     // Framework can override the effective capabilities.
> >     if (effective.isNone() && flags.effective_capabilities.isSome()) {
> >       effective = flags.effective_capabilities.get();
> >     }
> >     
> >     if (flags.bounding_capabilities.isSome()) {
> >       bounding = flags.bounding_capabilities.get();
> >     }
> >     
> >     if (effective.isSome() && bounding.isSome()) {
> >       // Validate that effective is a subset of bounding.
> >     }
> >     
> >     if (effective.isSome() && bounding.isNone()) {
> >       bounding = effective.get();
> >     }
> >     
> >     if (effective.isNone() && bounding.isSome()) {
> >       effective = bounding.get();
> >     }
> >     
> >     ...
> >     ```
> >     
> >     The above logic is a bit easier to follow, comparing putting some logic in `mergeCapabilities`.
I'll just kill `mergeCapabilities` function.

Thinking some more about this one :)


> On June 9, 2017, 9:28 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Line 456 (original), 456-457 (patched)
> > <https://reviews.apache.org/r/59552/diff/2/?file=1741705#file1741705line456>
> >
> >     We probably should have a CHECK (or comment) here. both of them are either set,
or either not set. Maybe add a comment in the flags as well.

The effective and bounding capabilities are currently allowed to vary (see below).


> On June 9, 2017, 9:28 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 627 (patched)
> > <https://reviews.apache.org/r/59552/diff/2/?file=1741705#file1741705line627>
> >
> >     Since two flags are either both set or both not set. I would simply check both
are isSome() here.

They are actually allowed to vary. If the neither the framework nor the operator specified
effective capabilities, then `launchInfo.has_effective_capabilities()` would be false but
`launchInfo.has_bounding_capabilities()` could still be true.


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59552/#review176530
-----------------------------------------------------------


On June 5, 2017, 4:36 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59552/
> -----------------------------------------------------------
> 
> (Updated June 5, 2017, 4:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7476
>     https://issues.apache.org/jira/browse/MESOS-7476
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The linux/capabilities isolator implements the `--allowed_capabilities`
> option by granting all the allowed capabilities. This change explicitly
> populates the only the bounding capabilities in the case where
> `--bounding_capabilities` has been set but the task itself has not been
> granted any effective capabilities. This improves the security of tasks
> since it is now possible to configure the bounding set without actually
> granting privilege to the task.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 60d22aa877c1ab62a08222e5efe8800e337684da

>   src/slave/containerizer/mesos/launch.cpp f48d294a0a832dfe248c4a83849ee5a63cb76bce 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 40376a03fdb8f931f8d3f83b1c3fa6207e02c1d1

> 
> 
> Diff: https://reviews.apache.org/r/59552/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message