Skip to content

Commit

Permalink
resolve merge conflicts.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joe Stubbs committed Oct 28, 2019
2 parents f67e620 + 6e72b02 commit 9b17acc
Show file tree
Hide file tree
Showing 12 changed files with 354 additions and 31 deletions.
12 changes: 9 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
# Change Log
All notable changes to this project will be documented in this file.

## 1.5.0 - 2019-09-16 (target 2019-11-01)
## 1.5.0 - 2019-10-29
### Added
- Added an endpoint `PUT /actors/aliases/{alias}` for updating the
definition of an alias. Requires `UPDATE` permission for the alias as well as for the actor to which the alias should be defined.

### Changed
- Fixed a bug where nonces defined for aliases would not be honored when using the alias in the URL (they were only honored when using the actor id assigned to the alias).
- Fixed issue where autoscaler did not properly scale down worker pools for actors with the `sync` hint. They are now scaled down to 1.
- The permission check on all on all `/aliases/{alias}` endpoints has been updated to require UPDATE on the associated `actor_id`.
- Fixed issue where the actor's token attribute was not being processed correctly causing tokens to be generated even for actors for which the attribute was false.
- Fixed issue where hypyerlinks in response model for executions were not generated correctly, showing the actor's internal database id instead of the human readable id.

- Fixed error messaging when using a nonce and the API endpoint+HTTP verb combination do not exist.
- The admin role is now recognized when checking access to certain objects in some edge cases, including when a nonce is used.

### Removed
- No change.
- It is no longer possible to create an alias nonce for permission levels UPDATE.


## 1.4.0 - 2019-09-16
Expand Down
28 changes: 24 additions & 4 deletions actors/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ def check_nonce():
any privileged action cannot be taken via a nonce.
"""
logger.debug("top of check_nonce")
# first check whether the request is even valid -
if hasattr(request, 'url_rule'):
logger.debug("request.url_rule: {}".format(request.url_rule))
if hasattr(request.url_rule, 'rule'):
logger.debug("url_rule.rule: {}".format(request.url_rule.rule))
else:
logger.info("url_rule has no rule.")
raise ResourceError(
"Invalid request: the API endpoint does not exist or the provided HTTP method is not allowed.", 405)
else:
logger.info("Request has no url_rule")
raise ResourceError(
"Invalid request: the API endpoint does not exist or the provided HTTP method is not allowed.", 405)
try:
nonce_id = request.args['x-nonce']
except KeyError:
Expand Down Expand Up @@ -273,7 +286,7 @@ def check_privileged():
data = request.get_json()
if not data:
data = request.form
# various APIs (e.g., the state api) allow an arbitary JSON serializable objects which won't have a get method:
# various APIs (e.g., the state api) allow an arbitrary JSON serializable objects which won't have a get method:
if not hasattr(data, 'get'):
return True
if not codes.PRIVILEGED_ROLE in g.roles:
Expand Down Expand Up @@ -312,12 +325,16 @@ def check_privileged():
logger.debug("not trying to use privileged options.")
return True

def check_permissions(user, identifier, level):
def check_permissions(user, identifier, level, roles=None):
"""Check the permissions store for user and level. Here, `identifier` is a unique id in the
permissions_store; e.g., actor db_id or alias_id.
"""
logger.debug("Checking user: {} permissions for identifier: {}".format(user, identifier))
# get all permissions for this actor
# first, if roles were passed, check for admin role -
if roles:
if codes.ADMIN_ROLE in roles:
return True
# get all permissions for this actor -
permissions = get_permissions(identifier)
for p_user, p_name in permissions.items():
# if the actor has been shared with the WORLD_USER anyone can use it
Expand Down Expand Up @@ -354,7 +371,10 @@ def get_db_id():
logger.error("Unrecognized request -- could not find the actor id. path_split: {}".format(path_split))
raise PermissionsException("Not authorized.")
logger.debug("path_split: {}".format(path_split))
actor_identifier = path_split[idx]
try:
actor_identifier = path_split[idx]
except IndexError:
raise ResourceError("Unable to parse actor identifier: is it missing from the URL?", 404)
logger.debug("actor_identifier: {}; tenant: {}".format(actor_identifier, g.tenant))
try:
actor_id = Actor.get_actor_id(g.tenant, actor_identifier)
Expand Down
2 changes: 2 additions & 0 deletions actors/codes.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ def __repr__(self):

PERMISSION_LEVELS = (NONE.name, READ.name, EXECUTE.name, UPDATE.name)

ALIAS_NONCE_PERMISSION_LEVELS = (NONE.name, READ.name, EXECUTE.name)

# role set by agaveflask in case the access_control_type is none
ALL_ROLE = 'ALL'

Expand Down
81 changes: 76 additions & 5 deletions actors/controllers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from auth import check_permissions, get_tas_data, tenant_can_use_tas, get_uid_gid_homedir, get_token_default
from channels import ActorMsgChannel, CommandChannel, ExecutionResultsChannel, WorkerChannel
from codes import SUBMITTED, COMPLETE, SHUTTING_DOWN, PERMISSION_LEVELS, READ, UPDATE, EXECUTE, PERMISSION_LEVELS, PermissionLevel
from codes import SUBMITTED, COMPLETE, SHUTTING_DOWN, PERMISSION_LEVELS, ALIAS_NONCE_PERMISSION_LEVELS, READ, UPDATE, EXECUTE, PERMISSION_LEVELS, PermissionLevel
from config import Config
from errors import DAOError, ResourceError, PermissionsException, WorkerException
from models import dict_to_camel, display_time, is_hashid, Actor, Alias, Execution, ExecutionsSummary, Nonce, Worker, get_permissions, \
Expand Down Expand Up @@ -138,7 +138,7 @@ def check_metrics(self, actor_ids, inbox_lengths, cmd_length):
except:
hints = []
for hint in hints:
if hint == actor.SYNC_HINT:
if hint == Actor.SYNC_HINT:
is_sync_actor = True
break
metrics_utils.scale_down(actor_id, is_sync_actor)
Expand Down Expand Up @@ -333,6 +333,10 @@ def post(self):
logger.debug("did not find actor: {}.".format(dbid))
raise ResourceError(
"No actor found with id: {}.".format(actor_id), 404)
# update 10/2019: check that use has UPDATE permission on the actor -
if not check_permissions(user=g.user, identifier=dbid, level=codes.UPDATE):
raise PermissionsException(f"Not authorized -- you do not have access to {actor_id}.")

# supply "provided" fields:
args['tenant'] = g.tenant
args['db_id'] = dbid
Expand All @@ -358,9 +362,66 @@ def get(self, alias):
logger.debug("did not find alias with id: {}".format(alias))
raise ResourceError(
"No alias found: {}.".format(alias), 404)
logger.debug("found actor {}".format(alias))
logger.debug("found alias {}".format(alias))
return ok(result=alias.display(), msg="Alias retrieved successfully.")

def validate_put(self):
logger.debug("top of validate_put")
try:
data = request.get_json()
except:
data = None
if data and 'alias' in data or 'alias' in request.form:
logger.debug("found alias in the PUT.")
raise DAOError("Invalid alias update description. The alias itself cannot be updated in a PUT request.")
parser = Alias.request_parser()
logger.debug("got the alias parser")
# remove since alias is only required for POST, not PUT
parser.remove_argument('alias')
try:
args = parser.parse_args()
except BadRequest as e:
msg = 'Unable to process the JSON description.'
if hasattr(e, 'data'):
msg = e.data.get('message')
raise DAOError("Invalid alias description. Missing required field: {}".format(msg))
return args

def put(self, alias):
logger.debug("top of PUT /actors/aliases/{}".format(alias))
alias_id = Alias.generate_alias_id(g.tenant, alias)
try:
alias_obj = Alias.from_db(alias_store[alias_id])
except KeyError:
logger.debug("did not find alias with id: {}".format(alias))
raise ResourceError("No alias found: {}.".format(alias), 404)
logger.debug("found alias {}".format(alias_obj))
args = self.validate_put()
actor_id = args.get('actor_id')
if Config.get('web', 'case') == 'camel':
actor_id = args.get('actorId')
dbid = Actor.get_dbid(g.tenant, actor_id)
# update 10/2019: check that use has UPDATE permission on the actor -
if not check_permissions(user=g.user, identifier=dbid, level=codes.UPDATE, roles=g.roles):
raise PermissionsException(f"Not authorized -- you do not have UPDATE "
f"access to the actor you want to associate with this alias.")
logger.debug(f"dbid: {dbid}")
# supply "provided" fields:
args['tenant'] = alias_obj.tenant
args['db_id'] = dbid
args['owner'] = alias_obj.owner
args['alias'] = alias_obj.alias
args['alias_id'] = alias_obj.alias_id
args['api_server'] = alias_obj.api_server
logger.debug("Instantiating alias object. args: {}".format(args))
new_alias_obj = Alias(**args)
logger.debug("Alias object instantiated; updating alias in alias_store. "
"alias: {}".format(new_alias_obj))
alias_store[alias_id] = new_alias_obj
logger.info("alias updated for actor: {}.".format(dbid))
set_permission(g.user, new_alias_obj.alias_id, UPDATE)
return ok(result=new_alias_obj.display(), msg="Actor alias updated successfully.")

def delete(self, alias):
logger.debug("top of DELETE /actors/aliases/{}".format(alias))
alias_id = Alias.generate_alias_id(g.tenant, alias)
Expand All @@ -370,6 +431,13 @@ def delete(self, alias):
logger.debug("did not find alias with id: {}".format(alias))
raise ResourceError(
"No alias found: {}.".format(alias), 404)

# update 10/2019: check that use has UPDATE permission on the actor -
# TODO - check: do we want to require UPDATE on the actor to delete the alias? Seems like UPDATE
# on the alias iteself should be sufficient...
# if not check_permissions(user=g.user, identifier=alias.db_id, level=codes.UPDATE):
# raise PermissionsException(f"Not authorized -- you do not have UPDATE "
# f"access to the actor associated with this alias.")
try:
del alias_store[alias_id]
# also remove all permissions - there should be at least one permissions associated
Expand Down Expand Up @@ -435,10 +503,11 @@ def validate_post(self):
msg = e.data.get('message')
raise DAOError("Invalid nonce description: {}".format(msg))
# additional checks

if 'level' in args:
if not args['level'] in PERMISSION_LEVELS:
if not args['level'] in ALIAS_NONCE_PERMISSION_LEVELS:
raise DAOError("Invalid nonce description. "
"The level attribute must be one of: {}".format(PERMISSION_LEVELS))
"The level attribute must be one of: {}".format(ALIAS_NONCE_PERMISSION_LEVELS))
if Config.get('web', 'case') == 'snake':
if 'max_uses' in args:
self.validate_max_uses(args['max_uses'])
Expand Down Expand Up @@ -614,9 +683,11 @@ def get(self):
return ok(result=actors, msg="Actors retrieved successfully.")

def validate_post(self):
logger.debug("top of validate post in /actors")
parser = Actor.request_parser()
try:
args = parser.parse_args()
logger.debug(f"initial actor args from parser: {args}")
if args['queue']:
queues_list = Config.get('spawner', 'host_queues').replace(' ', '')
valid_queues = queues_list.split(',')
Expand Down
2 changes: 1 addition & 1 deletion actors/metrics_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ def scale_down(actor_id, is_sync_actor=False):
if len(workers) == 1 and is_sync_actor:
logger.debug("only one worker, on sync actor. checking worker idle time..")
try:
sync_max_idle_time = int(Config.get('worker', 'sync_max_idle_time'))
sync_max_idle_time = int(Config.get('workers', 'sync_max_idle_time'))
except Exception as e:
logger.error(f"Got exception trying to read sync_max_idle_time from config; e:{e}")
sync_max_idle_time = DEFAULT_SYNC_MAX_IDLE_TIME
Expand Down
2 changes: 1 addition & 1 deletion actors/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ def set_logs(cls, exc_id, logs):
max_log_length = DEFAULT_MAX_LOG_LENGTH
if len(logs) > DEFAULT_MAX_LOG_LENGTH:
logger.info("truncating log for execution: {}".format(exc_id))
logs = logs[:max_log_length]
logs = logs[:max_log_length] + " LOG LIMIT EXCEEDED; this execution log was TRUNCATED!"
start_timer = timeit.default_timer()
if log_ex > 0:
logger.info("Storing log with expiry. exc_id: {}".format(exc_id))
Expand Down
72 changes: 67 additions & 5 deletions docs/specs/openapi_v3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,38 @@ paths:
content:
application/json:
schema:
allOf:
- $ref: '#/components/schemas/BasicResponse'
# allOf:
# - $ref: '#/components/schemas/BasicResponse'
properties:
result:
$ref: '#/components/schemas/Actor'
put:
tags:
- Actors
summary: Update an actor
description: Update an actor's definition.
operationId: updateActor
parameters:
- name: actor_id
in: path
description: Unique ID of the actor
required: true
schema:
type: string
requestBody:
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/NewActor'
responses:
200:
description: OK
content:
application/json:
schema:
# allOf:
# - $ref: '#/components/schemas/BasicResponse'
properties:
result:
$ref: '#/components/schemas/Actor'
Expand Down Expand Up @@ -745,6 +775,37 @@ paths:
properties:
result:
$ref: '#/components/schemas/Alias'
put:
tags:
- Actors
- Aliases
summary: Update an actor alias
description: Update an alias definition.
operationId: updateActorAlias
parameters:
- name: alias
in: path
description: Unique alias of the actor
required: true
schema:
type: string
requestBody:
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/NewAlias'
responses:
200:
description: OK
content:
application/json:
schema:
allOf:
- $ref: '#/components/schemas/BasicResponse'
properties:
result:
$ref: '#/components/schemas/Alias'
delete:
tags:
- Actors
Expand Down Expand Up @@ -898,8 +959,8 @@ components:

Actor:
type: object
allOf:
- $ref: '#/components/schemas/ArrayOfActorMounts'
# allOf:
# - $ref: '#/components/schemas/ArrayOfActorMounts'
properties:
id:
type: string
Expand All @@ -922,6 +983,8 @@ components:
link:
type: string
description: Actor identifier of actor to link this actor's events too. May be an actor id or an alias. Cycles not permitted.
mounts:
$ref: '#/components/schemas/ArrayOfActorMounts'
owner:
type: string
description: The user who created this actor.
Expand Down Expand Up @@ -1187,4 +1250,3 @@ components:




4 changes: 2 additions & 2 deletions local-dev.conf
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ dd: unix://var/run/docker.sock
init_count: 1

# set whether autoscaling is enabled
autoscaling = false
autoscaling = true

# max length of time, in seconds, an actor container is allowed to execute before being killed.
# set to -1 for indefinite execution time.
Expand Down Expand Up @@ -134,7 +134,7 @@ auto_remove: true
# Whether the workers should have OAuth clients generated for them:
generate_clients: False
# specifiy client generation is available for a specific tenant -
DEV-STAGING_generate_clients: True
# DEV-STAGING_generate_clients: True

# a list of mounts to make for every actor container, separated by comma (,) characters.
# Mounts should be of the form <absolute_host_path>:<absolute_container_path>:<mode>
Expand Down
6 changes: 6 additions & 0 deletions samples/mem_limit/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Image: abacosamples/mem_limit
from abacosamples/py3_base

ADD actor.py /actor.py

CMD ["python", "/actor.py"]
Empty file added samples/mem_limit/README.rst
Empty file.
Loading

0 comments on commit 9b17acc

Please sign in to comment.