Fixed in https://github.com/apache/mesos/commit/d2ab4b49d3cc0b86bacc5ec3400b46cfa70c3a7b
On Fri, Nov 18, 2016 at 4:48 AM, Benjamin Bannier < [email protected]> wrote: > Hi, > > This introduces a possibly uninitialized member `weight_info` which > Coverity immediately detected. I filed MESOS-6604 for that. Could you > please take that on @Alexander? > > > Cheers, > > Benjamin > > > On Nov 16, 2016, at 6:00 PM, [email protected] wrote: > > > > Enabled multiple field based authorization in the authorizer interface. > > > > Updates the authorizer interfaces and well as the local authorizer, > > such that all actions which were limited to use a _role_ or a > > _principal_ as an object, are able to use whole protobuf messages > > as objects. This change enables more sofisticated authorization > > mechanisms. > > > > Review: https://reviews.apache.org/r/52600/ > > > > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo > > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bc0e6d7b > > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bc0e6d7b > > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bc0e6d7b > > > > Branch: refs/heads/master > > Commit: bc0e6d7b0b367e5ff67dd5f395e1e06938b02399 > > Parents: 40c2e5f > > Author: Alexander Rojas <[email protected]> > > Authored: Tue Nov 15 19:04:25 2016 -0800 > > Committer: Adam B <[email protected]> > > Committed: Wed Nov 16 01:55:03 2016 -0800 > > > > ---------------------------------------------------------------------- > > include/mesos/authorizer/authorizer.hpp | 6 +- > > include/mesos/authorizer/authorizer.proto | 54 ++++++++++++ > > src/authorizer/local/authorizer.cpp | 115 > +++++++++++++++++++++---- > > 3 files changed, 157 insertions(+), 18 deletions(-) > > ---------------------------------------------------------------------- > > > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/ > include/mesos/authorizer/authorizer.hpp > > ---------------------------------------------------------------------- > > diff --git a/include/mesos/authorizer/authorizer.hpp > b/include/mesos/authorizer/authorizer.hpp > > index cb365c7..7217600 100644 > > --- a/include/mesos/authorizer/authorizer.hpp > > +++ b/include/mesos/authorizer/authorizer.hpp > > @@ -61,7 +61,9 @@ public: > > task_info(object.has_task_info() ? &object.task_info() : > nullptr), > > executor_info( > > object.has_executor_info() ? &object.executor_info() : > nullptr), > > - quota_info(object.has_quota_info() ? &object.quota_info() : > nullptr) {} > > + quota_info(object.has_quota_info() ? &object.quota_info() : > nullptr), > > + weight_info(object.has_weight_info() ? &object.weight_info() : > nullptr), > > + resource(object.has_resource() ? &object.resource() : nullptr) > {} > > > > const std::string* value; > > const FrameworkInfo* framework_info; > > @@ -69,6 +71,8 @@ public: > > const TaskInfo* task_info; > > const ExecutorInfo* executor_info; > > const quota::QuotaInfo* quota_info; > > + const WeightInfo* weight_info; > > + const Resource* resource; > > }; > > > > /** > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/ > include/mesos/authorizer/authorizer.proto > > ---------------------------------------------------------------------- > > diff --git a/include/mesos/authorizer/authorizer.proto > b/include/mesos/authorizer/authorizer.proto > > index b6a9f14..0696a62 100644 > > --- a/include/mesos/authorizer/authorizer.proto > > +++ b/include/mesos/authorizer/authorizer.proto > > @@ -46,11 +46,17 @@ message Object { > > optional TaskInfo task_info = 4; > > optional ExecutorInfo executor_info = 5; > > optional quota.QuotaInfo quota_info = 6; > > + optional WeightInfo weight_info = 7; > > + optional Resource resource = 8; > > } > > > > > > // List of authorizable actions supported in Mesos. > > +// NOTE: Values in this enum should be kept in > > +// numerical order to prevent accidental aliasing. > > enum Action { > > + option allow_alias = true; > > + > > // This must be the first enum value in this list, to > > // ensure that if 'type' is not set, the default value > > // is UNKNOWN. This enables enum values to be added > > @@ -58,19 +64,67 @@ enum Action { > > UNKNOWN = 0; > > > > // Actions named *_WITH_foo may set a foo in `Object.value`. > > + > > + // `REGISTER_FRAMEWORK` will have an object with `FrameworkInfo` set. > > + // The `_WITH_ROLE` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + REGISTER_FRAMEWORK = 1; > > REGISTER_FRAMEWORK_WITH_ROLE = 1; > > > > // `RUN_TASK` will have an object with `FrameworkInfo` and `TaskInfo` > set. > > RUN_TASK = 2; > > > > + // `TEARDOWN_FRAMEWORK` will have an object with `FrameworkInfo` set. > > + // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + TEARDOWN_FRAMEWORK = 3; > > TEARDOWN_FRAMEWORK_WITH_PRINCIPAL = 3; > > + > > + // `RESERVE_RESOURCES` will have an object with `Resource` set. > > + // The `_WITH_ROLE` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + RESERVE_RESOURCES = 4; > > RESERVE_RESOURCES_WITH_ROLE = 4; > > + > > + // `UNRESERVE_RESOURCES` will have an object with `Resource` set. > > + // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + UNRESERVE_RESOURCES = 5; > > UNRESERVE_RESOURCES_WITH_PRINCIPAL = 5; > > + > > + // `CREATE_VOLUME` will have an object with `Resource` set. > > + // The `_WITH_ROLE` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + CREATE_VOLUME = 6; > > CREATE_VOLUME_WITH_ROLE = 6; > > + > > + // `DESTROY_VOLUME` will have an object with `Resource` set. > > + // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + DESTROY_VOLUME = 7; > > DESTROY_VOLUME_WITH_PRINCIPAL = 7; > > + > > GET_ENDPOINT_WITH_PATH = 8; > > VIEW_ROLE = 9; > > + > > + // `UPDATE_WEIGHT` will have an object with `WeightInfo` set. > > + // The `_WITH_ROLE` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + UPDATE_WEIGHT = 10; > > UPDATE_WEIGHT_WITH_ROLE = 10; > > + > > + // `GET_QUOTA` will have an object with `QuotaInfo` set. > > + // The `_WITH_ROLE` alias is deprecated and will be removed after > > + // Mesos 1.2's deprecation cycle ends. The `value` field will continue > > + // to be set until that time. > > + GET_QUOTA = 11; > > GET_QUOTA_WITH_ROLE = 11; > > > > // `UPDATE_QUOTA` will have an object with a `QuotaInfo` set. > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/ > src/authorizer/local/authorizer.cpp > > ---------------------------------------------------------------------- > > diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/ > authorizer.cpp > > index f1dff65..77e05dd 100644 > > --- a/src/authorizer/local/authorizer.cpp > > +++ b/src/authorizer/local/authorizer.cpp > > @@ -221,15 +221,7 @@ public: > > } else { > > switch (action_) { > > // All actions using `object.value` for authorization. > > - case authorization::REGISTER_FRAMEWORK_WITH_ROLE: > > - case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL: > > - case authorization::RESERVE_RESOURCES_WITH_ROLE: > > - case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL: > > - case authorization::CREATE_VOLUME_WITH_ROLE: > > - case authorization::DESTROY_VOLUME_WITH_PRINCIPAL: > > - case authorization::GET_QUOTA_WITH_ROLE: > > case authorization::VIEW_ROLE: > > - case authorization::UPDATE_WEIGHT_WITH_ROLE: > > case authorization::GET_ENDPOINT_WITH_PATH: { > > // Check object has the required types set. > > CHECK_NOTNULL(object->value); > > @@ -239,6 +231,92 @@ public: > > > > break; > > } > > + case authorization::REGISTER_FRAMEWORK: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->framework_info) { > > + aclObject.add_values(object->framework_info->role()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > + case authorization::TEARDOWN_FRAMEWORK: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->framework_info) { > > + aclObject.add_values(object->framework_info->principal()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > + case authorization::CREATE_VOLUME: > > + case authorization::RESERVE_RESOURCES: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->resource) { > > + aclObject.add_values(object->resource->role()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > + case authorization::DESTROY_VOLUME: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->resource) { > > + aclObject.add_values( > > + object->resource->disk().persistence().principal()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > + case authorization::UNRESERVE_RESOURCES: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->resource) { > > + aclObject.add_values(object->resource->reservation(). > principal()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > + case authorization::GET_QUOTA: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->quota_info) { > > + aclObject.add_values(object->quota_info->role()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > + case authorization::UPDATE_WEIGHT: { > > + aclObject.set_type(mesos::ACL::Entity::SOME); > > + if (object->weight_info) { > > + aclObject.add_values(object->weight_info->role()); > > + } else if (object->value) { > > + aclObject.add_values(*(object->value)); > > + } else { > > + aclObject.set_type(mesos::ACL::Entity::ANY); > > + } > > + > > + break; > > + } > > case authorization::RUN_TASK: { > > aclObject.set_type(mesos::ACL::Entity::SOME); > > if (object->task_info && object->task_info->has_command() && > > @@ -253,6 +331,7 @@ public: > > } else { > > aclObject.set_type(mesos::ACL::Entity::ANY); > > } > > + > > break; > > } > > case authorization::ACCESS_MESOS_LOG: { > > @@ -497,7 +576,7 @@ private: > > vector<GenericACL> acls_; > > > > switch (action) { > > - case authorization::REGISTER_FRAMEWORK_WITH_ROLE: > > + case authorization::REGISTER_FRAMEWORK: > > foreach ( > > const ACL::RegisterFramework& acl, > acls.register_frameworks()) { > > GenericACL acl_; > > @@ -509,7 +588,7 @@ private: > > > > return acls_; > > break; > > - case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL: > > + case authorization::TEARDOWN_FRAMEWORK: > > foreach ( > > const ACL::TeardownFramework& acl, > acls.teardown_frameworks()) { > > GenericACL acl_; > > @@ -532,7 +611,7 @@ private: > > > > return acls_; > > break; > > - case authorization::RESERVE_RESOURCES_WITH_ROLE: > > + case authorization::RESERVE_RESOURCES: > > foreach (const ACL::ReserveResources& acl, > acls.reserve_resources()) { > > GenericACL acl_; > > acl_.subjects = acl.principals(); > > @@ -543,7 +622,7 @@ private: > > > > return acls_; > > break; > > - case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL: > > + case authorization::UNRESERVE_RESOURCES: > > foreach ( > > const ACL::UnreserveResources& acl, > acls.unreserve_resources()) { > > GenericACL acl_; > > @@ -555,7 +634,7 @@ private: > > > > return acls_; > > break; > > - case authorization::CREATE_VOLUME_WITH_ROLE: > > + case authorization::CREATE_VOLUME: > > foreach (const ACL::CreateVolume& acl, acls.create_volumes()) { > > GenericACL acl_; > > acl_.subjects = acl.principals(); > > @@ -566,7 +645,7 @@ private: > > > > return acls_; > > break; > > - case authorization::DESTROY_VOLUME_WITH_PRINCIPAL: > > + case authorization::DESTROY_VOLUME: > > foreach (const ACL::DestroyVolume& acl, acls.destroy_volumes()) { > > GenericACL acl_; > > acl_.subjects = acl.principals(); > > @@ -577,7 +656,7 @@ private: > > > > return acls_; > > break; > > - case authorization::GET_QUOTA_WITH_ROLE: > > + case authorization::GET_QUOTA: > > foreach (const ACL::GetQuota& acl, acls.get_quotas()) { > > GenericACL acl_; > > acl_.subjects = acl.principals(); > > @@ -629,7 +708,7 @@ private: > > > > return acls_; > > break; > > - case authorization::UPDATE_WEIGHT_WITH_ROLE: > > + case authorization::UPDATE_WEIGHT: > > foreach (const ACL::UpdateWeight& acl, acls.update_weights()) { > > GenericACL acl_; > > acl_.subjects = acl.principals(); > > @@ -845,7 +924,9 @@ process::Future<bool> LocalAuthorizer::authorized( > > request.object().has_task() || > > request.object().has_task_info() || > > request.object().has_executor_info() || > > - request.object().has_quota_info()))); > > + request.object().has_quota_info() || > > + request.object().has_weight_info() || > > + request.object().has_resource()))); > > > > typedef Future<bool> (LocalAuthorizerProcess::*F)( > > const authorization::Request&); > > > >
