aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject incubator-aurora git commit: Remove dynamic command hooks and dynamic hook policy.
Date Tue, 13 Jan 2015 19:24:35 GMT
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 <zmanji@twopensource.com>
Authored: Tue Jan 13 11:23:53 2015 -0800
Committer: -l <maxim@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


Mime
View raw message