Fix CLI output, multiple configs, improve tests

mozilla_aws_cli/cli.py

@@ -31,6 +31,8 @@
 logger.setLevel(logging.ERROR)
 logging.getLogger('urllib3').propagate = False
 
+VALID_OUTPUT_OPTIONS = ("envvar", "shared", "awscli")
+
 
 def validate_arn(ctx, param, value):
     # arn:aws:iam::account-id:role/role-name
@@ -45,8 +47,10 @@ def validate_arn(ctx, param, value):
         return value
 
 
-def validate_awscli_exists(ctx, param, value):
-    if value.lower() == 'awscli' and not find_executable('aws'):
+def validate_output(ctx, param, value):
+    if value is None:
+        pass
+    elif value.lower() == 'awscli' and not find_executable('aws'):
         raise click.BadParameter('AWS CLI is not detected on local system.')
 
     return value
@@ -79,17 +83,24 @@ def validate_config_file(ctx, param, filenames):
         # Override the --config file contents with the mozilla_aws_cli_config
         # module contents
         for key in mozilla_aws_cli_config.config:
-            config.set('DEFAULT', key, mozilla_aws_cli_config.config[key])
+            if key in config.defaults() and config.defaults()[key] != mozilla_aws_cli_config.config[key]:
+                raise click.BadOptionUsage(None, "setting for `{}` exists in both global module and local config file".format(key))
+
+            config.defaults()[key] = mozilla_aws_cli_config.config[key]
 
     missing_settings = (
         {'client_id', 'idtoken_for_roles_url', 'well_known_url'} - set(config.defaults().keys()))
 
     if missing_settings:
-        raise click.BadParameter(
-            '{} setting{} are missing from the config files {}'.format(
-                ', '.join(missing_settings),
+        message = '{} setting{} are missing from config files: {}'.format(
+                ', '.join(["`{}`".format(setting) for setting in missing_settings]),
                 's' if len(missing_settings) > 1 else '',
-                " ".join(filenames)))
+                " ".join(filenames))
+        raise click.BadOptionUsage(None, message)
+
+    if config.defaults().get("output", "envvar") not in VALID_OUTPUT_OPTIONS:
+        raise click.BadParameter('{}'.format(config.defaults()["output"]),
+                                 param_hint="`output` in config file")
 
     return config.defaults()
 
@@ -122,10 +133,9 @@ def validate_disable_caching(ctx, param, disabled):
 @click.option(
     "-o",
     "--output",
-    default="envvar",
-    type=click.Choice(["envvar", "shared", "awscli"]),
+    type=click.Choice(VALID_OUTPUT_OPTIONS),
     help="How to output the AWS API keys",
-    callback=validate_awscli_exists
+    callback=validate_output
 )
 @click.option(
     "-r",
@@ -140,6 +150,11 @@ def main(batch, config, no_cache, output, role_arn, verbose, web_console):
     if verbose:
         logger.setLevel(logging.DEBUG)
 
+    # The output setting "envvar" if not specified on the command line or in a
+    # config file
+    if output is None:
+        output = config.get("output", "envvar")
+
     config["openid-configuration"] = requests.get(config["well_known_url"]).json()
     config["jwks"] = requests.get(config["openid-configuration"]["jwks_uri"]).json()
 

tests/test_federated_aws_cli.py

@@ -3,57 +3,76 @@
 
 """Tests for `mozilla_aws_cli` package."""
 
-import os
+import uuid
+
 from click.testing import CliRunner
 from mozilla_aws_cli import cli
 
 
 def test_command_line_interface():
     """Test the CLI."""
     runner = CliRunner()
-    help_result = runner.invoke(cli.main, ['--help'])
+    help_result = runner.invoke(cli.main, ["--help"])
     assert help_result.exit_code == 0
-    assert 'Show this message and exit.' in help_result.output
+    assert "Show this message and exit." in help_result.output
 
 
 def test_parse_config():
-    good_arn = 'arn:aws:iam::123456789012:role/MyRole'
-    bad_arn = 'bogus'
-    good_config_content = '''[DEFAULT]
+    good_config_content = """[DEFAULT]
 well_known_url = http://auth.example.com/.well-known/openid-configuration
 client_id = abcdefghijklmnopqrstuvwxyz012345
-idtoken_for_roles_url = https://example.com/roles'''
+idtoken_for_roles_url = https://example.com/roles"""
+
+    bad_arn = "bogus"
+    bad_output_setting = good_config_content + "\noutput = blahblahblah"
+    good_arn = "arn:aws:iam::123456789012:role/MyRole"
+    missing_config_setting = """[DEFAULT]
+well_known_url = http://auth.example.com/.well-known/openid-configuration
+    """
+
     cases = {
-        'malformed ini': {
-            'filename': os.path.expanduser(os.path.join("~", ".malformed")),
-            'content': '- a z=x:\ny: "${z}"',
-            'args': ['-c', os.path.expanduser(os.path.join("~", ".malformed")), '-r', good_arn]
+        "malformed ini": {
+            "content": "- a z=x:\ny: \"${z}\"",
+            "args": ["-r", good_arn]
+        },
+        "missing config setting": {
+            "content": missing_config_setting,
+            "args": ["-r", good_arn]
         },
-        'missing config setting': {
-            'filename': os.path.expanduser(os.path.join("~", ".missing_config_setting")),
-            'content': '[DEFAULT]\nwell_known_url = http://auth.example.com/.well-known/openid-configuration\n',
-            'args': ['-c', os.path.expanduser(os.path.join("~", ".missing_config_setting")), '-r', good_arn]
+        "bad role arn": {
+            "content": good_config_content,
+            "args": ["-r", bad_arn]
+        },
+        "bad output setting": {
+            "content": bad_output_setting,
+            "args": ["-r", good_arn]
         },
-        'bad role arn': {
-            'filename': os.path.expanduser(os.path.join("~", ".good_config")),
-            'content': good_config_content,
-            'args': ['-c', os.path.expanduser(os.path.join("~", ".good_config")), '-r', bad_arn]
-        }
     }
 
     runner = CliRunner()
     with runner.isolated_filesystem():
         results = {}
         for case in cases:
-            with open(cases[case]['filename'], 'w') as f:
-                f.write(cases[case]['content'])
-            results[case] = runner.invoke(cli.main, cases[case]['args'])
+            args = cases[case]["args"]
+
+            # Use a random filename, if "-c" isn"t specified in the arguments above
+            filename = str(uuid.uuid4())
+            if "-c" not in args:
+                args += ["-c", filename]
+
+            with open(filename, "w") as f:
+                f.write(cases[case]["content"])
+
+            results[case] = runner.invoke(cli.main, args)
+
+        assert results["malformed ini"].exit_code != 0
+        assert "is not a valid INI" in results["malformed ini"].output
 
-        assert results['malformed ini'].exit_code != 0
-        assert 'is not a valid INI' in results['malformed ini'].output
+        assert results["missing config setting"].exit_code != 0
+        assert "settings are missing from config files" in results["missing config setting"].output
 
-        assert results['missing config setting'].exit_code != 0
-        assert 'settings are missing from the config file' in results['missing config setting'].output
+        assert results["bad role arn"].exit_code != 0
+        assert "is not a valid ARN" in results["bad role arn"].output
 
-        assert results['bad role arn'].exit_code != 0
-        assert 'is not a valid ARN' in results['bad role arn'].output
+        assert results["bad output setting"].exit_code != 0
+        assert "`output` in config file" in results["bad output setting"].output