You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ma...@apache.org on 2015/01/13 20:24:35 UTC
incubator-aurora git commit: Remove dynamic command hooks and dynamic
hook policy.
Repository: incubator-aurora
Updated Branches:
refs/heads/master 78d2b5245 -> a350982ee
Remove dynamic command hooks and dynamic hook policy.
Reviewed at https://reviews.apache.org/r/29717/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/a350982e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/a350982e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/a350982e
Branch: refs/heads/master
Commit: a350982ee63b70eefdb182274946c2c3c8789509
Parents: 78d2b52
Author: Zameer Manji <zm...@twopensource.com>
Authored: Tue Jan 13 11:23:53 2015 -0800
Committer: -l <ma...@apache.org>
Committed: Tue Jan 13 11:23:53 2015 -0800
----------------------------------------------------------------------
docs/design/command-hooks.md | 173 +-------------
.../python/apache/aurora/client/cli/__init__.py | 8 +-
.../apache/aurora/client/cli/command_hooks.py | 178 +--------------
.../python/apache/aurora/client/cli/AuroraHooks | 46 ----
.../cli/hook_test_data/bad_syntax/AuroraHooks | 15 --
.../cli/hook_test_data/exec_error/AuroraHooks | 47 ----
.../aurora/client/cli/test_command_hooks.py | 224 +------------------
7 files changed, 13 insertions(+), 678 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/docs/design/command-hooks.md
----------------------------------------------------------------------
diff --git a/docs/design/command-hooks.md b/docs/design/command-hooks.md
index cc56218..3f3f70f 100644
--- a/docs/design/command-hooks.md
+++ b/docs/design/command-hooks.md
@@ -30,12 +30,9 @@ verb from being executed.
## Registering Hooks
-These hooks will be registered two ways:
-* Project hooks file. If a file named `AuroraHooks` is in the project directory
- where an aurora command is being executed, that file will be read,
- and its hooks will be registered.
-* Configuration plugins. A configuration plugin can register hooks using an API.
- Hooks registered this way are, effectively, hardwired into the client executable.
+These hooks will be registered via configuration plugins. A configuration plugin
+can register hooks using an API. Hooks registered this way are, effectively,
+hardwired into the client executable.
The order of execution of hooks is unspecified: they may be called in
any order. There is no way to guarantee that one hook will execute
@@ -49,22 +46,9 @@ because they will run for all configurations, whether or not they
specify any hooks in the configuration file.
In the implementation, hooks are registered in the module
-`apache.aurora.client.cli.command_hooks`, using the class `GlobalCommandHookRegistry`. A
-global hook can be registered by calling `GlobalCommandHookRegistry.register_command_hook`
-in a configuration plugin.
-
-### Hook Files
-
-A hook file is a file containing Python source code. It will be
-dynamically loaded by the Aurora command line executable. After
-loading, the client will check the module for a global variable named
-"hooks", which contains a list of hook objects, which will be added to
-the hook registry.
-
-A project hooks file will be named `AuroraHooks`,
-and will be located in either the directory where the command is being
-executed, or one of its parent directories, up to the nearest git/mercurial
-repository base.
+`apache.aurora.client.cli.command_hooks`, using the class
+`GlobalCommandHookRegistry`. A global hook can be registered by calling
+`GlobalCommandHookRegistry.register_command_hook` in a configuration plugin.
### The API
@@ -107,127 +91,6 @@ repository base.
def register_command_hook(self, hook):
pass
-## Skipping Hooks
-
-In a perfect world, hooks would represent a global property or policy
-that should always be enforced. Unfortunately, we don't live in a
-perfect world, which means that sometimes, every rule needs to get
-broken.
-
-For example, an organization could decide that every configuration
-must be checked in to source control before it could be
-deployed. That's an entirely reasonable policy. It would be easy to
-implement it using a hook. But what if there's a problem, and the
-source repos is down?
-
-The easiest solution is just to allow a user to add a `--skip-hooks`
-flag to the command-line. But doing that means that an organization
-can't actually use hooks to enforce policy, because users can skip
-them whenever they want.
-
-Instead, we'd like to have a system where it's possible to create
-hooks to enforce policy, and then include a way of building policy
-about when hooks can be skipped.
-
-I'm using sudo as a rough model for this. Many organizations need to
-give people the ability to run privileged commands, but they still
-want to have some control. Sudo allows them to specify who is allowed
-to run a privileged command; where they're allowed to run it; and what
-command(s) they're allowed to run. All of that is specified in a
-special system file located in `/etc/sudoers` on a typical unix
-machine.
-
-In a world of distributed systems, this approach has one grave
-weakness. An aurora client can be located on any machine that has
-network access to a Mesos/Aurora cluster. It can be run by a user in
-pretty much any way they want - from a machine they control, from a
-special chroot they created, etc. Relying an a file being in a special
-location on their machine isn't sufficient - it's too easy to
-maliciously or erroneously run a command in an environment with an
-invalid hooks exceptions file.
-
-Instead, we've got two basic choices: hook exception rules can be
-baked into the client executable, or they can be provided in a
-network location.
-
-### Specifying when hooks can be skipped
-
-#### Hooks File
-
-The module `apache.aurora.client.cli` contains a variable named
-`GLOBAL_HOOK_SKIP_RULES_URL`. In the default distribution of Aurora, tihs variable contains
-`None`. Users can modify this value for their local environments, providing
-a site specific URL. If users attempt to bypass command hooks, and this
-URL is not `None`, then the client will fetch the contents of that URL, and
-attempt to interpret it as a hooks exception file.
-
-The hooks exception file is written in JSON, with the following structure:
-
- { "rulename":
- {
- "hooks": [ "hook-name ", ... ],
- "users": [ string, ...],
- "commands": { "job": ["kill", "killall", ...], ... },
- "arg-patterns": [ "regexp-str", ... ]
- },
- ...
- }
-
-* `hooks` is a list of hook identifiers which can be skipped by a user
- that satisfies this rule. If omitted, then this rule applies to _all hooks_.
- (Omitting the `hooks` field is equivalent to giving it the value `['*']`.)
-* `users` is a list of user names, or glob expressions that range over user
- names. This rule gives permission to those users to skip hooks. If omitted,
- then this rule allows _any user_ to skip hooks that satisfy the rest of this rule.
- Note that this is _user_ names, not
- _role_ names: the rules specify users that are allowed to skip commands.
- Some users that are allowed to work with a role account may be allowed to
- skip, while others cannot.
-* `commands` is a map from nouns to lists of verbs. If a command `aurora n v`
- is being executed, this rule allows the hooks to be skipped if
- `v` is in `commands[n]`. If this is omitted, then it allows hooks to be skipped for all
- commands that satisfy the rest of the rule.
-* `arg_patterns` is a list of glob patterns ranging over parameters.
- If any of the parameters of the command match the parameters in this list,
- the hook can be skipped. If ommitted, then this applies regardless of arguments.
-
-For example, the following is a hook rules file which allows:
-* The user "root" to skip any hook.
-* Any user to skip hooks for test jobs.
-* A specific group of users to skip hooks for jobs in cluster `east`
-* Another group of users to skip hooks for `job kill` in cluster `west`.
-
- {
- "allow-admin": { "users": ["root"] },
- "allow-test": { "users": ["\*"], "arg-patterns": ["\*/\*/test/\*"] },
- "allow-east-users": { "users"=['john', 'mary', 'mike', 'sue'],
- "arg-patterns": ["east/\*/\*/\*"] },
- "allow-west-kills": { "users": ["anne", "bill", "chris"],
- "commands": { "job": ["kill"]}, "arg-patterns" = ["west/\*/\*/\*"] }
- }
-
-#### Programmatic Hooks Exceptions
-
-The `GlobalHooksRegistry` contains the method `add_hooks_exception`, which allows
-users to register local hooks exceptions using the `ConfigurationPlugin` mechanism.
-A hooks exception object implements the following interface:
-
- class HooksException(object):
- def allow_exception(self, hooks, role, noun, verb, args):
- """Params:
- - hooks: a list of hook-names that the user wants to skip. If this
- is ommitted, then this applies to all hooks.
- - role: the role requesting that hooks be skipped.
- - noun, verb: the noun and verb being executed.
- - the other command-line arguments.
- Returns: True if the user should be allowed to skip the requested hooks.
- """
- return False
-
-When a user supplies the `--skip-hooks` argument, `allow_exception` is invoked on
-each of the `HooksException` arguments. If _any_ of the hooks exception objects
-returns `True`, then the user will be permitted to skip the hooks.
-
### Skipping Hooks
To skip a hook, a user uses a command-line option, `--skip-hooks`. The option can either
@@ -237,27 +100,3 @@ specify specific hooks to skip, or "all":
without running any hooks.
* `aurora --skip-hooks=test,iq create east/bozo/devel/myjob` will create a job,
and will skip only the hooks named "test" and "iq".
-
-
-## Changes
-
-4/30:
-* Rule exceptions are defined in JSON, and they are specified to be loaded from
- a URL, not from a local file.
-* Rule exceptions specify users, not roles.
-
-4/27:
-Major changes between this and the last version of this proposal.
-* Command hooks can't be declared in a configuration file. There's a simple
- reason why: hooks run before a command's implementation is invoked.
- Config files are read during the commands invocation if necessary. If the
- hook is declared in the config file, by the time you know that it should
- have been run, it's too late. So I've removed config-hooks from the
- proposal. (API hooks defined by configs still work.)
-* Skipping hooks. We expect aurora to be used inside of large
- organizations. One of the primary use-cases of hooks is to create
- enforcable policy that are specific to an organization. If hooks
- can just be skipped because a user wants to skip them, it means that
- the policy can't be enforced, which defeats the purpose of having them.
- So in this update, I propose a mechanism, loosely based on a `sudo`-like
- mechanism for defining when hooks can be skipped.
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/src/main/python/apache/aurora/client/cli/__init__.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/__init__.py b/src/main/python/apache/aurora/client/cli/__init__.py
index 1211131..a595948 100644
--- a/src/main/python/apache/aurora/client/cli/__init__.py
+++ b/src/main/python/apache/aurora/client/cli/__init__.py
@@ -50,16 +50,10 @@ EXIT_COMMAND_FAILURE = 4
EXIT_INVALID_COMMAND = 5
EXIT_INVALID_PARAMETER = 6
EXIT_NETWORK_ERROR = 7
-EXIT_PERMISSION_VIOLATION = 8
EXIT_TIMEOUT = 9
EXIT_API_ERROR = 10
EXIT_UNKNOWN_ERROR = 20
-# A location where you can find a site-specific file containing
-# global hook skip rules. This can be something like a link into a file stored in a git
-# repos.
-GLOBAL_HOOK_SKIP_RULES_URL = None
-
__version__ = pkg_resources.resource_string(__name__, '.auroraversion')
@@ -251,7 +245,7 @@ class CommandLine(AbstractClass):
return self.nouns.keys()
def _setup(self, args):
- GlobalCommandHookRegistry.setup(GLOBAL_HOOK_SKIP_RULES_URL)
+ GlobalCommandHookRegistry.setup()
# Accessing registered_nouns has the side-effect of registering them, but since
# nouns is unused, we must disable checkstyle.
nouns = self.registered_nouns # noqa
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/src/main/python/apache/aurora/client/cli/command_hooks.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/command_hooks.py b/src/main/python/apache/aurora/client/cli/command_hooks.py
index 26ee85e..e95b1b1 100644
--- a/src/main/python/apache/aurora/client/cli/command_hooks.py
+++ b/src/main/python/apache/aurora/client/cli/command_hooks.py
@@ -17,96 +17,23 @@
from __future__ import print_function
-import getpass
-import logging
-import os
-import sys
from abc import abstractmethod, abstractproperty
-from fnmatch import fnmatch
-import requests
-from twitter.common.lang import AbstractClass, Compatibility
+from twitter.common.lang import AbstractClass
from apache.aurora.client.cli.options import CommandOption
-# Ugly workaround to avoid cyclic dependency.
-EXIT_PERMISSION_VIOLATION = 8
-
-
ALL_HOOKS = "all"
-class SkipHooksRule(object):
- """A rule that specifies when a user may skip a command hook."""
-
- @property
- def name(self):
- pass
-
- def allow_hook_skip(self, hook, user, noun, verb, args):
- """ Check if this rule allows a user to skip a specific hook.
- Params:
- - hook: the name of the hook that the user wants to skip.
- - user: the user requesting that hooks be skipped. (Note: user, not role!)
- - noun, verb: the noun and verb being executed.
- - args: the other command-line arguments.
- Returns: True if the user should be allowed to skip the requested hooks.
- """
- return False
-
-
-class JsonSkipHooksRule(SkipHooksRule):
- """An implementation for skip rules that are loaded from a json file.
- See the design doc for details on the format of the rules."""
-
- def __init__(self, name, json_rule):
- self.rulename = name
- self.hooks = json_rule.get("hooks", ["*"])
- self.users = json_rule.get("users", ["*"])
- self.commands = json_rule.get("commands", {})
- self.arg_patterns = json_rule.get("arg-patterns", None)
-
- @property
- def name(self):
- return self.rulename
-
- def allow_hook_skip(self, hook, user, noun, verb, args):
- def _hooks_match(hook):
- return any(fnmatch(hook, hook_pattern) for hook_pattern in self.hooks)
-
- def _commands_match(noun, verb):
- return noun in self.commands and verb in self.commands[noun]
-
- def _users_match(user):
- return any(fnmatch(user, user_pattern) for user_pattern in self.users)
-
- def _args_match(args):
- if self.arg_patterns is None:
- return True
- else:
- return any(fnmatch(arg, arg_pattern) for arg_pattern in self.arg_patterns
- for arg in args)
-
- return (_hooks_match(hook) and _commands_match(noun, verb) and _users_match(user) and
- _args_match(args))
-
-
class GlobalCommandHookRegistry(object):
- """Registry for command hooks.
- See docs/design/command-hooks.md for details on what hooks are, and how
- they work.
- """
+ """Registry for command hooks."""
COMMAND_HOOKS = []
- HOOKS_FILE_NAME = "AuroraHooks"
- SKIP_HOOK_RULES = {}
@classmethod
- def setup(cls, rules_url):
- """Initializes the hook registry, loading the project hooks and the skip rules."""
- if rules_url is not None:
- cls.load_global_hook_skip_rules(rules_url)
- cls.load_project_hooks()
+ def setup(cls):
+ pass
@classmethod
def get_options(cls):
@@ -117,95 +44,8 @@ class GlobalCommandHookRegistry(object):
" cannot be skipped, then the command will be aborted"))]
@classmethod
- def register_hook_skip_rule(cls, rule):
- """Registers a rule to allow users to skip certain hooks"""
- cls.SKIP_HOOK_RULES[rule.name] = rule
-
- @classmethod
- def register_json_hook_skip_rules(cls, json_rules):
- """Register a set of skip rules that are structured as JSON dictionaries."""
- for (name, rule) in json_rules.items():
- cls.register_hook_skip_rule(JsonSkipHooksRule(name, rule))
-
- @classmethod
- def load_global_hook_skip_rules(cls, url):
- """If the system uses a master skip rules file, loads the master skip rules JSON."""
- resp = requests.get(url)
- try:
- rules = resp.json()
- cls.register_json_hook_skip_rules(rules)
- except ValueError as e:
- logging.error("Client could not decode hook skip rules: %s" % e)
-
- @classmethod
- def load_hooks_file(cls, path):
- """Load a file containing hooks. If there are any errors compiling or executing the file,
- the errors will be logged, the hooks from the file will be skipped, but the execution of
- the command will continue.
- """
- with open(path, "r") as hooks_file:
- hooks_data = hooks_file.read()
- hooks_code = None
- try:
- hooks_code = compile(hooks_data, path, "exec")
- except (SyntaxError, TypeError) as e:
- logging.warn("Error compiling hooks file %s: %s" % (path, e))
- print("Error compiling hooks file %s: %s" % (path, e), file=sys.stderr)
- return {}
- hooks_environment = {}
- try:
- Compatibility.exec_function(hooks_code, hooks_environment)
- except Exception as e:
- # Unfortunately, exec could throw *anything* at all.
- logging.warn("Warning: error loading hooks file %s: %s" % (path, e))
- print("Warning: error loading hooks file %s: %s" % (path, e), file=sys.stderr)
- return {}
- for hook in hooks_environment.get("hooks", []):
- cls.register_command_hook(hook)
- return hooks_environment
-
- @classmethod
- def find_project_hooks_file(cls, dir):
- """Search for a file named "AuroraHooks" in current directory or
- one of its parents, up to the closest repository root. Only one
- file will be loaded, so creating an AuroraHooks file in a subdirecory will
- override and replace the one in the parent directory
- """
- def is_repos_root(dir):
- # a directory is a git root if it contains a directory named ".git".
- # it's an HG root if it contains a directory named ".hg"
- return any(os.path.isdir(os.path.join(dir, rootname)) for rootname in [".git", ".hg"])
-
- filepath = os.path.join(dir, cls.HOOKS_FILE_NAME)
- if os.path.exists(filepath):
- return filepath
- elif is_repos_root(dir):
- # This is a repository root, so stop looking.
- return None
- else:
- parent, _ = os.path.split(os.path.abspath(dir))
- # if we've reached the filesystem root, then parent==self.
- if parent == dir:
- return None
- else:
- return cls.find_project_hooks_file(parent)
- return None
-
- @classmethod
- def load_project_hooks(cls, dir="."):
- """Looks for a project hooks file. Project hooks will be in the current directory,
- or one of its parents. Stops looking at the root of an SCM repository.
- """
- hooks_file = cls.find_project_hooks_file(dir)
- if hooks_file is not None:
- return cls.load_hooks_file(hooks_file)
- else:
- return None
-
- @classmethod
def reset(cls):
"""For testing purposes, reset the list of registered hooks"""
- cls.SKIP_HOOK_RULES = {}
cls.COMMAND_HOOKS = []
@classmethod
@@ -216,13 +56,11 @@ class GlobalCommandHookRegistry(object):
verb in hook.get_verbs(noun)]
@classmethod
- def get_required_hooks(cls, context, skip_opt, noun, verb, user=None):
+ def get_required_hooks(cls, context, skip_opt, noun, verb):
"""Given a set of hooks that match a command, find the set of hooks that
must be run. If the user asked to skip a hook that cannot be skipped,
raise an exception.
"""
- if user is None:
- user = getpass.getuser()
selected_hooks = cls.get_command_hooks_for(noun, verb)
# The real set of required hooks is the set of hooks that match the command
# being executed, minus the set of hooks that the user both wants to skip,
@@ -232,12 +70,6 @@ class GlobalCommandHookRegistry(object):
selected_hook_names = [hook.name for hook in selected_hooks]
desired_skips = set(selected_hook_names if skip_opt == ALL_HOOKS else skip_opt.split(","))
desired_skips = desired_skips & set(selected_hook_names)
- for desired_skip in desired_skips:
- if not any(rule.allow_hook_skip(desired_skip, user, noun, verb, context.args)
- for name, rule in cls.SKIP_HOOK_RULES.items()):
- logging.info("Hook %s cannot be skipped by user %s" % (desired_skip, user))
- raise context.CommandError(EXIT_PERMISSION_VIOLATION,
- "Hook %s cannot be skipped by user %s" % (desired_skip, user))
selected_hook_names = set(selected_hook_names) - desired_skips
return [hook for hook in selected_hooks if hook.name in selected_hook_names]
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/src/test/python/apache/aurora/client/cli/AuroraHooks
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/AuroraHooks b/src/test/python/apache/aurora/client/cli/AuroraHooks
deleted file mode 100644
index 73a7644..0000000
--- a/src/test/python/apache/aurora/client/cli/AuroraHooks
+++ /dev/null
@@ -1,46 +0,0 @@
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-from apache.aurora.client.cli.command_hooks import CommandHook
-
-class DynamicHook(CommandHook):
-
- def __init__(self, succeed):
- self.succeed = succeed
- self.ran_pre = False
- self.ran_post = False
-
- def name(self):
- return "dynamic-hook"
-
- def get_nouns(self):
- return ['job']
-
- def get_verbs(self, noun):
- if noun == 'job':
- return ['create', 'status' ]
- else:
- return []
-
- def pre_command(self, noun, verb, context, commandline):
- self.ran_pre = True
- if self.succeed:
- return 0
- else:
- return 1
-
- def post_command(self, noun, verb, context, commandline, result):
- self.ran_post = True
-
-hooks = [ DynamicHook(True) ]
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax/AuroraHooks
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax/AuroraHooks b/src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax/AuroraHooks
deleted file mode 100644
index 9a22159..0000000
--- a/src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax/AuroraHooks
+++ /dev/null
@@ -1,15 +0,0 @@
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-This is not valid python syntax.
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/src/test/python/apache/aurora/client/cli/hook_test_data/exec_error/AuroraHooks
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/hook_test_data/exec_error/AuroraHooks b/src/test/python/apache/aurora/client/cli/hook_test_data/exec_error/AuroraHooks
deleted file mode 100644
index 72f97bd..0000000
--- a/src/test/python/apache/aurora/client/cli/hook_test_data/exec_error/AuroraHooks
+++ /dev/null
@@ -1,47 +0,0 @@
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-from apache.aurora.client.cli.command_hooks import CommandHook
-
-class DynamicHook(CommandHook):
-
- def __init__(self, succeed):
- self.succeed = succeed
- self.ran_pre = False
- self.ran_post = False
-
- def name(self):
- return "dynamic-hook"
-
- def get_nouns(self):
- return ['job']
-
- def get_verbs(self, noun):
- if noun == 'job':
- return ['create', 'status' ]
- else:
- return []
-
- def pre_command(self, noun, verb, context, commandline):
- self.ran_pre = True
- if self.succeed:
- return 0
- else:
- return 1
-
- def post_command(self, noun, verb, context, commandline, result):
- self.ran_post = True
-
-
-hooks = [ DynamicHook(True), 1/0 ]
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a350982e/src/test/python/apache/aurora/client/cli/test_command_hooks.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_command_hooks.py b/src/test/python/apache/aurora/client/cli/test_command_hooks.py
index d3b3b9d..e8432a1 100644
--- a/src/test/python/apache/aurora/client/cli/test_command_hooks.py
+++ b/src/test/python/apache/aurora/client/cli/test_command_hooks.py
@@ -12,13 +12,9 @@
# limitations under the License.
#
-import contextlib
-
-import requests
-from mock import create_autospec, patch
+from mock import patch
from twitter.common.contextutil import temporary_file
-from apache.aurora.client.cli import EXIT_PERMISSION_VIOLATION
from apache.aurora.client.cli.client import AuroraCommandLine
from apache.aurora.client.cli.command_hooks import CommandHook, GlobalCommandHookRegistry
from apache.aurora.config import AuroraConfig
@@ -67,36 +63,6 @@ class HookForTesting(CommandHook):
self.ran_post = True
-class SecondHookForTesting(CommandHook):
- def __init__(self, succeed):
- self.succeed = succeed
- self.ran_pre = False
- self.ran_post = False
-
- @property
- def name(self):
- return "second"
-
- def get_nouns(self):
- return ["job"]
-
- def get_verbs(self, noun):
- if noun == "job":
- return ["kill", "killall", "create"]
- else:
- return []
-
- def pre_command(self, noun, verb, context, commandline):
- self.ran_pre = True
- if self.succeed:
- return 0
- else:
- return 1
-
- def post_command(self, noun, verb, context, commandline, result):
- self.ran_post = True
-
-
class TestClientCreateCommand(AuroraClientCommandTest):
@classmethod
@@ -133,10 +99,6 @@ class TestClientCreateCommand(AuroraClientCommandTest):
return cls.create_simple_success_response()
@classmethod
- def get_failed_createjob_response(cls):
- return cls.create_error_response()
-
- @classmethod
def assert_create_job_called(cls, mock_api):
# Check that create_job was called exactly once, with an AuroraConfig parameter.
assert mock_api.create_job.call_count == 1
@@ -151,9 +113,6 @@ class TestClientCreateCommand(AuroraClientCommandTest):
GlobalCommandHookRegistry.reset()
command_hook = HookForTesting(True)
GlobalCommandHookRegistry.register_command_hook(command_hook)
- self.generic_test_successful_hook(command_hook)
-
- def generic_test_successful_hook(self, command_hook):
mock_context = FakeAuroraCommandContext()
with patch("apache.aurora.client.cli.jobs.Job.create_context", return_value=mock_context):
mock_query = self.create_mock_query()
@@ -204,184 +163,3 @@ class TestClientCreateCommand(AuroraClientCommandTest):
assert api.create_job.call_count == 0
assert command_hook.ran_pre
assert not command_hook.ran_post
-
- def test_load_dynamic_hooks(self):
- GlobalCommandHookRegistry.reset()
- hook_locals = GlobalCommandHookRegistry.load_project_hooks(
- "./src/test/python/apache/aurora/client/cli")
- assert hook_locals["hooks"][0] in GlobalCommandHookRegistry.COMMAND_HOOKS
-
- def test_successful_dynamic_hook(self):
- GlobalCommandHookRegistry.reset()
- hook_locals = GlobalCommandHookRegistry.load_project_hooks(
- "./src/test/python/apache/aurora/client/cli")
- self.generic_test_successful_hook(hook_locals["hooks"][0])
-
- def test_dynamic_hook_syntax_error(self):
- with patch("logging.warn") as log_patch:
- GlobalCommandHookRegistry.reset()
- GlobalCommandHookRegistry.load_project_hooks(
- "./src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax")
- log_patch.assert_called_with("Error compiling hooks file "
- "./src/test/python/apache/aurora/client/cli/hook_test_data/bad_syntax/AuroraHooks: "
- "invalid syntax (AuroraHooks, line 15)")
-
- def test_dynamic_hook_exec_error(self):
- with patch("logging.warn") as log_patch:
- GlobalCommandHookRegistry.reset()
- GlobalCommandHookRegistry.load_project_hooks(
- "./src/test/python/apache/aurora/client/cli/hook_test_data/exec_error")
- log_patch.assert_called_with("Warning: error loading hooks file "
- "./src/test/python/apache/aurora/client/cli/hook_test_data/exec_error/AuroraHooks: "
- "integer division or modulo by zero")
-
- def assert_skip_allowed(self, context, skip_opt, user, noun, verb, args):
- """Checks that a hook would be allowed to be skipped in a command invocation"""
- GlobalCommandHookRegistry.get_required_hooks(context, skip_opt, noun, verb, user)
-
- def assert_skip_forbidden(self, context, skip_opt, user, noun, verb, args):
- """Checks that a hook would NOT be allowed to be skipped in a command invocation"""
- try:
- # assertRaises doesn't work with classmethods.
- context.args = args
- GlobalCommandHookRegistry.get_required_hooks(context, skip_opt, noun, verb, user)
- self.fail("Should have thrown an error.")
- except context.CommandError:
- pass
-
- def test_json_skip_rules(self):
- """Load up a set of skips, specified in JSON, and then check a bunch of different
- cases to see that the skip rules work correctly.
- """
- mock_response = create_autospec(spec=requests.Response, instance=True)
- mock_context = FakeAuroraCommandContext()
- with patch("requests.get", return_value=mock_response):
- mock_response.json.return_value = {
- "a": {
- "users": ["bozo", "clown"],
- "commands": {"job": ["killall"]},
- "hooks": ["test_hook", "second"]
- },
- "b": {
- "commands": {"user": ["kick"]},
- }
- }
- GlobalCommandHookRegistry.reset()
- GlobalCommandHookRegistry.setup("http://foo.bar")
- command_hook_one = HookForTesting(False)
- GlobalCommandHookRegistry.register_command_hook(command_hook_one)
- command_hook_two = SecondHookForTesting(True)
- GlobalCommandHookRegistry.register_command_hook(command_hook_two)
- assert len(GlobalCommandHookRegistry.SKIP_HOOK_RULES) == 2
- # Should not be allowed: no skip rule permitting skipping hooks on job kill
- self.assert_skip_forbidden(mock_context, "all", "beezus", "job", "kill", ["a", "b", "c"])
- # Should not be allowed: there are hooks on "job killall", and beezus doesn't satisfy
- # their exception rules
- self.assert_skip_forbidden(mock_context, "all", "beezus", "job", "kill", ["a", "b", "c"])
- # Should be allowed: there's a rule allowing bozo to skip test_hook on job killall.
- self.assert_skip_allowed(mock_context, "test_hook", "bozo", "job", "killall", ["a", "b", "c"])
- # Should be allowed: since there's only one hook on killall, this is equivalent to
- # the previous.
- self.assert_skip_allowed(mock_context, "all", "bozo", "job", "killall", ["a", "b", "c"])
- # Should be allowed: there's a rule allowing anyone to skip any hook on "user kick".
- self.assert_skip_allowed(mock_context, "test_hook,something_else", "nobody", "user",
- "kick", ["a", "b", "c"])
- # should be allowed: there are no hooks in place for "user kick", so all is acceptable.
- self.assert_skip_allowed(mock_context, "all", "nobody", "user", "kick", ["a", "b", "c"])
-
- def test_skip_hooks_in_create(self):
- """Run a test of create, with a hook that should forbid running create, but
- with a user who's covered by one of the hook skip exception rules - so it
- should succeed.
- """
-
- GlobalCommandHookRegistry.reset()
- command_hook = HookForTesting(True)
- GlobalCommandHookRegistry.register_command_hook(command_hook)
- command_hook_two = SecondHookForTesting(False)
- GlobalCommandHookRegistry.register_command_hook(command_hook_two)
- mock_response = create_autospec(spec=requests.Response, instance=True)
- mock_context = FakeAuroraCommandContext()
-
- with contextlib.nested(
- patch("apache.aurora.client.cli.jobs.Job.create_context", return_value=mock_context),
- patch("requests.get", return_value=mock_response),
- patch("getpass.getuser", return_value="bozo")):
- mock_response.json.return_value = {
- "a": {
- "users": ["bozo", "clown"],
- "commands": {"job": ["killall", "create"]},
- "hooks": ["test_hook", "second"]
- },
- "b": {
- "commands": {"user": ["kick"]},
- }
- }
-
- mock_query = self.create_mock_query()
- mock_context.add_expected_status_query_result(
- self.create_mock_status_query_result(ScheduleStatus.INIT))
- mock_context.add_expected_status_query_result(
- self.create_mock_status_query_result(ScheduleStatus.RUNNING))
- mock_context.get_api("west").check_status.side_effect = (
- lambda x: self.create_mock_status_query_result(ScheduleStatus.RUNNING))
- api = mock_context.get_api("west")
- api.create_job.return_value = self.get_createjob_response()
- GlobalCommandHookRegistry.setup("http://foo.bar")
-
- with temporary_file() as fp:
- fp.write(self.get_valid_config())
- fp.flush()
- cmd = AuroraCommandLine()
- result = cmd.execute(["job", "create", "--skip-hooks=second", "--wait-until=RUNNING",
- "west/bozo/test/hello", fp.name])
- assert result == 0
- self.assert_create_job_called(api)
- self.assert_scheduler_called(api, mock_query, 1)
- assert command_hook.ran_pre
- assert command_hook.ran_post
-
- def test_cannot_skip_hooks_in_create(self):
- """This time, the hook shouldn't be skippable, because we use a username
- who isn't allowed by the hook exception rule.
- """
- GlobalCommandHookRegistry.reset()
- command_hook = HookForTesting(True)
- GlobalCommandHookRegistry.register_command_hook(command_hook)
- command_hook_two = SecondHookForTesting(False)
- GlobalCommandHookRegistry.register_command_hook(command_hook_two)
- mock_response = create_autospec(spec=requests.Response, instance=True)
- mock_context = FakeAuroraCommandContext()
-
- with contextlib.nested(
- patch("apache.aurora.client.cli.jobs.Job.create_context", return_value=mock_context),
- patch("requests.get", return_value=mock_response),
- patch("getpass.getuser", return_value="beezus")):
- mock_response.json.return_value = {
- "a": {
- "users": ["bozo", "clown"],
- "commands": {"job": ["killall", "create"]},
- "hooks": ["test_hook", "second"]
- },
- "b": {
- "commands": {"user": ["kick"]},
- }
- }
-
- mock_context.add_expected_status_query_result(
- self.create_mock_status_query_result(ScheduleStatus.INIT))
- mock_context.add_expected_status_query_result(
- self.create_mock_status_query_result(ScheduleStatus.RUNNING))
- api = mock_context.get_api("west")
- api.create_job.return_value = self.get_createjob_response()
- GlobalCommandHookRegistry.setup("http://foo.bar")
-
- with temporary_file() as fp:
- fp.write(self.get_valid_config())
- fp.flush()
- cmd = AuroraCommandLine()
- result = cmd.execute(["job", "create", "--skip-hooks=second", "--wait-until=RUNNING",
- "west/bozo/test/hello", fp.name])
- # Check that it returns the right error code, and that create_job didn't get called.
- assert result == EXIT_PERMISSION_VIOLATION
- assert api.create_job.call_count == 0