mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 61171: Enabled filtering of the 'GET_AGENTS' v1 API call.
Date Fri, 28 Jul 2017 18:56:19 GMT

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




src/common/http.cpp
Lines 1005-1009 (patched)
<https://reviews.apache.org/r/61171/#comment257340>

    Ah, looks like we need to make sure we're doing authorization on the `Resource.reservations`
field everywhere; see this case, for example: https://github.com/apache/mesos/blob/155285d3d5815aacda4d8fcc1c02c58ddb572a31/src/master/http.cpp#L514-L515
    
    Could you make sure there aren't any other cases like that, and follow up with a patch
to add this?



src/master/http.cpp
Lines 2524-2530 (patched)
<https://reviews.apache.org/r/61171/#comment257335>

    Could you just do:
    
    ```
      return AuthorizationAcceptor::create(
          principal,
          master->authorizer,
          authorization::VIEW_ROLE)
        .then(defer(master->self(),
    ```



src/tests/api_tests.cpp
Lines 1638-1639 (patched)
<https://reviews.apache.org/r/61171/#comment257319>

    s/are being filtered to unauthorized users/is returned to unauthorized users in response
to the GET_AGENTS call/



src/tests/api_tests.cpp
Lines 1642 (patched)
<https://reviews.apache.org/r/61171/#comment257320>

    Can probably omit this comment.



src/tests/api_tests.cpp
Lines 1675-1682 (patched)
<https://reviews.apache.org/r/61171/#comment257321>

    It would be nice to use the v1 RESERVE_RESOURCES call since we're also testing the v1
GET_AGENTS call, but I'm not going to open an issue since we mix v0/v1 in many other tests.
    
    Another option is to statically reserve the resources.
    
    Or leave as-is :)



src/tests/api_tests.cpp
Lines 1692-1693 (patched)
<https://reviews.apache.org/r/61171/#comment257323>

    I think this comment is a bit misleading - the reason that this principal can view these
reservations is because the authorizer's `permissive` bit is set to `true`, not because the
reservations are "its own". Maybe:
    
    "Default credential principal should be allowed to see all reservations, regardless of
role."



src/tests/api_tests.cpp
Lines 1704 (patched)
<https://reviews.apache.org/r/61171/#comment257328>

    Might as well inspect the total/used/offered resources as well, right? Here and below.



src/tests/api_tests.cpp
Lines 1724-1725 (patched)
<https://reviews.apache.org/r/61171/#comment257326>

    Maybe:
    
    "Principal 2 should not be able to see resources associated with any role."


- Greg Mann


On July 28, 2017, 11:57 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61171/
> -----------------------------------------------------------
> 
> (Updated July 28, 2017, 11:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, Quinn Leng, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7416
>     https://issues.apache.org/jira/browse/MESOS-7416
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enables filtering of the results of calls to the 'GET_AGENTS' v1
> API. It filters the contents of different resources entries based
> on the 'VIEW_ROLE' permissions of the principal doing the request
> based on resource roles, allocation roles and reservations.
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp ba8dda18a02f51d1a28e719f06ee4b51573dfbec 
>   src/common/http.cpp dfd5f335e8a3745d047d4f9f5e8c821b2c22da5a 
>   src/common/protobuf_utils.hpp 80d2edd452f3ffa38c40f9a21f8489799065c401 
>   src/common/protobuf_utils.cpp 49d3a229925f4aa107e3e5f762936c16318aeadb 
>   src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
>   src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
>   src/tests/api_tests.cpp 1d5b080c809248bdf4c76ddad382d714692c804b 
> 
> 
> Diff: https://reviews.apache.org/r/61171/diff/3/
> 
> 
> Testing
> -------
> 
> ```shell
> make check
> ```
> 
> Manual test:
> 
> ```shell
> mkdir -p /tmp/mesos/master
> mkdir -p /tmp/mesos/agent
> 
> # Create credentials
> cat <<EOF > /tmp/mesos/credentials.txt
> hal-9000 dave
> glados potato
> skynet connor
> EOF
> 
> # Create ACLs
> cat <<EOF > /tmp/mesos/acls.json
> {
>   "permissive": true,
>   "view_roles" : [
>    {
>      "principals" : { "type" : "ANY" },
>      "roles" : { "values" : ["*"] }
>    },
>    {
>      "principals" : { "values" : ["hal-9000"] },
>      "roles" : { "values" : ["space-odyssey"] }
>    },
>    {
>      "principals" : { "values" : ["hal-9000"] },
>      "roles" : { "type" : "NONE" }
>    },
>    {
>      "principals" : { "values" : ["glados"] },
>      "roles" : { "values" : ["portal"] }
>    },
>    {
>      "principals" : { "values" : ["glados"] },
>      "roles" : { "type" : "NONE" }
>    },
>    {
>      "principals" : { "values" : ["skynet"] },
>      "roles" : { "values" : ["terminator"] }
>    },
>    {
>      "principals" : { "values" : ["skynet"] },
>      "roles" : { "type" : "NONE" }
>    }
>   ]
> }
> EOF
> 
> # Launch Master with some predefined roles.
> ./bin/mesos-master.sh \
>     --work_dir=/tmp/mesos/master \
>     --log_dir=/tmp/mesos/master/log \
>     --authenticate_http \
>     --credentials=/tmp/mesos/credentials.txt \
>     --authenticate_http_frameworks \
>     --http_framework_authenticators=basic \
>     --http_authenticators=basic \
>     --authenticate_http_readonly \
>     --acls=/tmp/mesos/acls.json \
>     --roles="space-odyssey,portal,terminator" &
>     
> # Launch Agent with static reservations for all roles.
> sudo ./bin/mesos-agent.sh \
>     --master=127.0.0.1:5050 \
>     --work_dir=/tmp/mesos/agent \
>     --authenticate_http_readwrite \
>     --http_authenticators=basic \
>     --http_credentials=/tmp/mesos/credentials.txt \
>     --acls=/tmp/mesos/acls.json \
>     --resources='cpus(space-odyssey):2;cpus(portal):2;cpus(*):4;mem(space-odyssey):250;mem(portal):250;mem(*):10360;ports(space-odyssey):[31000-32000];ports(portal):[32001-33000];ports(*):[33001-35000];disk(space-odyssey):250;disk(portal):250;disk(*):1000'
&
>     
> # Launch test framework.
> ./src/mesos-execute \
>     --master=127.0.0.1:5050 \
>     --command='while true; do echo "Hello World"; sleep 5; done;' \
>     --resources="cpus:1;mem:128;disk:32;ports:[31002-31003]" \
>     --role=space-odyssey \
>     --name=hello-discovery \
>     --principal=hal-9000 \
>     --secret=dave &
>     
> # Create a dynamic reservation.    
> cat > /tmp/resources.json <<EOM
> [
>   {
>     "name": "cpus",
>     "type": "SCALAR",
>     "scalar": { "value": 2 },
>     "role": "terminator",
>     "reservation": {
>       "principal": "skynet"
>     }
>   },
>   {
>     "name": "mem",
>     "type": "SCALAR",
>     "scalar": { "value": 250 },
>     "role": "terminator",
>     "reservation": {
>       "principal": "skynet"
>     }
>   },
>   {
>     "name": "disk",
>     "type": "SCALAR",
>     "scalar": { "value": 250 },
>     "role": "terminator",
>     "reservation": {
>       "principal": "skynet"
>     }
>   },
>   {
>     "name": "ports",
>     "type": "RANGES",
>     "ranges": {
>       "range": [
>         { 
>           "begin": 33001,
>           "end": 34000
>         }
>       ]
>     },
>     "role": "terminator",
>     "reservation": {
>       "principal": "skynet"
>     }
>   }
> ]
> EOM
> 
> http \
>     -a skynet:connor \
>     -f POST \
>     127.0.0.1:5050/master/reserve \
>     slaveId=${SLAVE_ID} \
>     resources=@/tmp/resources.json
>     
>     
> # Create some quota.
> cat > /tmp/quota.json <<EOM
> {
>   "role": "portal",
>   "guarantee": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": { "value": 2 }
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": { "value": 250 }
>     },
>     {
>       "name": "disk",
>       "type": "SCALAR",
>       "scalar": { "value": 250 }
>     }
>   ]
> }
> EOM
> 
> http \
>     -a glados:potato \
>     POST \
>     127.0.0.1:5050/master/quota \
>     @/tmp/quota.json
>     
>     
> # Query the master with all users and check
> # that only the information of his role is
> # available.
> http -a glados:potato -v -j -f POST 127.0.0.1:5050/api/v1 type=GET_AGENTS
> 
> http -a skynet:connor -v -j -f POST 127.0.0.1:5050/api/v1 type=GET_AGENTS
> 
> http -a hal-9000:dave -v -j -f POST 127.0.0.1:5050/api/v1 type=GET_AGENTS
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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