diff --git a/.github/workflows/tests-integration.yaml b/.github/workflows/tests-integration.yaml index 5fafd25..bc58897 100644 --- a/.github/workflows/tests-integration.yaml +++ b/.github/workflows/tests-integration.yaml @@ -51,6 +51,9 @@ jobs: integration_tests_cli_refarch: runs-on: ubuntu-latest + env: + TERRAFORM_VERSION: 1.9.8 + OPENTOFU_VERSION: 1.9.1 strategy: max-parallel: 1 matrix: @@ -79,6 +82,18 @@ jobs: export PATH="${PATH}:${HOME}/.poetry/bin" poetry install --with=dev + - name: Install terraform and tofu + run: | + echo "[INFO] Installing terraform ${TERRAFORM_VERSION} and tofu ${OPENTOFU_VERSION}" + curl -fsSL -o /tmp/terraform.zip \ + "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip" + curl -fsSL -o /tmp/tofu.zip \ + "https://github.com/opentofu/opentofu/releases/download/v${OPENTOFU_VERSION}/tofu_${OPENTOFU_VERSION}_linux_amd64.zip" + sudo unzip -q -o /tmp/terraform.zip terraform -d /usr/local/bin + sudo unzip -q -o /tmp/tofu.zip tofu -d /usr/local/bin + terraform version + tofu version + - name: Build Leverage CLI run: | echo "[INFO] Building Leverage CLI" @@ -131,6 +146,10 @@ jobs: aws configure set output json --profile bb-apps-devstg-devops aws configure set role_arn arn:aws:iam::${{ secrets.AWS_DEVSTG_ACCOUNT_ID }}:role/DeployMaster --profile bb-apps-devstg-devops aws configure set source_profile bb-deploymaster --profile bb-apps-devstg-devops + aws configure set region us-east-1 --profile bb-security-oaar + aws configure set output json --profile bb-security-oaar + aws configure set role_arn arn:aws:iam::${{ secrets.AWS_SECURITY_ACCOUNT_ID }}:role/DeployMaster --profile bb-security-oaar + aws configure set source_profile bb-deploymaster --profile bb-security-oaar cat << EOF > ~/.aws/credentials [bb-deploymaster] aws_access_key_id = ${{ secrets.AWS_ACCESS_KEY_ID }} diff --git a/leverage/leverage.py b/leverage/leverage.py index 214b6b9..cae0d48 100644 --- a/leverage/leverage.py +++ b/leverage/leverage.py @@ -6,7 +6,7 @@ from leverage import __version__, conf from leverage._internals import pass_state -from leverage.path import NotARepositoryError, PathsHandler +from leverage.path import NotARepositoryError, build_paths_and_environment, is_project_yaml_only_bootstrap from leverage.modules import aws, credentials, run, project, tofu, terraform, tfautomv, kubectl @@ -34,11 +34,14 @@ def leverage(context, state, verbose): if context.invoked_subcommand == project.name: return - state.paths = PathsHandler(state.config) - state.environment = { - "AWS_SHARED_CREDENTIALS_FILE": str(state.paths.aws_credentials_file), - "AWS_CONFIG_FILE": str(state.paths.aws_config_file), - } + # `credentials configure` can legitimately run right after `project init` and before + # `project create`: only project.yaml exists, so no project name can be resolved yet and + # PathsHandler would abort. The `credentials` group callback derives the project name from + # project.yaml and builds state.paths itself in that case. + if context.invoked_subcommand == credentials.name and is_project_yaml_only_bootstrap(): + return + + state.paths, state.environment = build_paths_and_environment(state.config) # Add modules to leverage diff --git a/leverage/modules/credentials.py b/leverage/modules/credentials.py index 0de0dfd..14dc38b 100644 --- a/leverage/modules/credentials.py +++ b/leverage/modules/credentials.py @@ -16,13 +16,14 @@ from questionary import Choice from click.exceptions import Exit -from leverage import logger +from leverage import conf, logger from leverage._utils import ExitError from leverage.modules.runner import Runner from leverage._internals import State, pass_runner, pass_paths, pass_state from leverage.path import ( NotARepositoryError, PathsHandler, + build_paths_and_environment, get_global_config_path, get_project_root_or_current_dir_path, ) @@ -265,6 +266,20 @@ def credentials(state): if short_name is None or not re.match("^[a-z]{2,4}$", short_name): logger.error("Invalid or missing project short name in project.yaml file.") raise Exit(1) + + if not build_env.exists(): + # Completes this branch's own docstring promise, mirroring the common.tfvars branch + # below. Guarded so a mature project that still has project.yaml on disk never has + # its real build.env clobbered (project create never deletes project.yaml). + logger.info("Writing project short name to build.env.") + build_env.write_text(f"PROJECT={short_name}\nTF_IMAGE_TAG=1.1.9") + + if state.paths is None: + # Upstream (leverage.py) skipped PathsHandler because only project.yaml existed. + # build.env now has a project name (or already did) - reload config from disk and + # build real, project-scoped paths here instead of leaving state.paths/environment None. + state.config = conf.load() + state.paths, state.environment = build_paths_and_environment(state.config) elif not build_env.exists(): # project_config is not empty # and build.env does not exist @@ -372,7 +387,7 @@ def _profile_is_configured(awscli: Runner, profile: str): Returns: bool: Whether the profile was already configured or not. """ - exit_code, _, _ = awscli.exec("configure", "list", "--profile", profile) + exit_code, _, _ = awscli.exec("configure", "list", "--profile", profile, raises=False) return not exit_code @@ -446,7 +461,7 @@ def configure_credentials( values = {"aws_access_key_id": key_id, "aws_secret_access_key": secret_key} for key, value in values.items(): - exit_code, output, _ = awscli.exec("configure", "set", key, value, "--profile", profile) + exit_code, output, _ = awscli.exec("configure", "set", key, value, "--profile", profile, raises=False) if exit_code: raise ExitError(exit_code, f"AWS CLI error: {output}") @@ -467,7 +482,7 @@ def _credentials_are_valid(awscli: Runner, profile: str): Returns: bool: Whether the credentials are valid. """ - error_code, output, _ = awscli.exec("sts", "get-caller-identity", "--profile", profile) + error_code, output, _ = awscli.exec("sts", "get-caller-identity", "--profile", profile, raises=False) return error_code != 255 and "InvalidClientTokenId" not in output @@ -482,7 +497,9 @@ def _get_management_account_id(awscli: Runner, profile: str): Returns: str: Management account id. """ - exit_code, caller_identity, _ = awscli.exec("sts", "get-caller-identity", "--output", "json", "--profile", profile) + exit_code, caller_identity, _ = awscli.exec( + "sts", "get-caller-identity", "--output", "json", "--profile", profile, raises=False + ) if exit_code: raise ExitError(exit_code, f"AWS CLI error: {caller_identity}") @@ -502,7 +519,7 @@ def _get_organization_accounts(awscli: Runner, profile: str, project_name: str): dict: Mapping of organization accounts names to ids. """ exit_code, organization_accounts, _ = awscli.exec( - "organizations", "list-accounts", "--output", "json", "--profile", profile + "organizations", "list-accounts", "--output", "json", "--profile", profile, raises=False ) if exit_code: @@ -530,7 +547,9 @@ def _get_mfa_serial(awscli: Runner, profile: str): Returns: str: MFA device serial. """ - exit_code, mfa_devices, _ = awscli.exec("iam", "list-mfa-devices", "--output", "json", "--profile", profile) + exit_code, mfa_devices, _ = awscli.exec( + "iam", "list-mfa-devices", "--output", "json", "--profile", profile, raises=False + ) if exit_code: raise ExitError(exit_code, f"AWS CLI error: {mfa_devices}") mfa_devices = json.loads(mfa_devices) @@ -558,7 +577,7 @@ def configure_profile(awscli: Runner, profile: str, values: dict): """ logger.info(f"\tConfiguring profile [bold]{profile}[/bold]") for key, value in values.items(): - exit_code, output, _ = awscli.exec("configure", "set", key, value, "--profile", profile) + exit_code, output, _ = awscli.exec("configure", "set", key, value, "--profile", profile, raises=False) if exit_code: raise ExitError(exit_code, f"AWS CLI error: {output}") @@ -613,8 +632,9 @@ def configure_accounts_profiles( # A profile identifier looks like `le-security-oaar` account_profiles[f"{short_name}-{account_name}-{PROFILES[_type]['profile_role']}-mfa"] = account_profile - logger.info("Backing up account profiles file.") - shutil.copy(paths.aws_config_file, paths.aws_config_file.with_suffix(".bkp")) + if paths.aws_config_file.exists(): + logger.info("Backing up account profiles file.") + shutil.copy(paths.aws_config_file, paths.aws_config_file.with_suffix(".bkp")) for profile_identifier, profile_values in account_profiles.items(): configure_profile(profile_identifier, profile_values) diff --git a/leverage/modules/tf.py b/leverage/modules/tf.py index 5394a69..4aec08a 100644 --- a/leverage/modules/tf.py +++ b/leverage/modules/tf.py @@ -485,7 +485,7 @@ def _make_layer_backend_key(cwd, account_dir, account_name): @pass_paths def _validate_layout(paths, layer: str): - paths.check_for_layer_location() + paths.check_for_layer_location(Path(layer)) # Check for `environment = ` in account.tfvars account_name = paths.account_conf.get("environment") diff --git a/leverage/path.py b/leverage/path.py index 04c35e0..346a152 100644 --- a/leverage/path.py +++ b/leverage/path.py @@ -157,7 +157,7 @@ def __init__(self, env_conf: dict): self.backend_conf = hcl2.loads(backend_config.read_text()) if backend_config.exists() else {} # Get MFA enabled status - self.mfa_enabled = env_conf.get("MFA_ENABLED", "false") + self.mfa_enabled = str(env_conf.get("MFA_ENABLED", "false")).strip().lower() == "true" # Get project name self.project = self.common_conf.get("project", env_conf.get("PROJECT", False)) @@ -183,11 +183,13 @@ def __init__(self, env_conf: dict): @property def common_tfvars(self): - return f"{self.root_dir}/config/{self.COMMON_TF_VARS}" + # return f"{self.root_dir}/config/{self.COMMON_TF_VARS}" + return self.root_dir / "config" / self.COMMON_TF_VARS @property def account_tfvars(self): - return f"{self.account_dir}/config/{self.ACCOUNT_TF_VARS}" + # return f"{self.account_dir}/config/{self.ACCOUNT_TF_VARS}" + return self.account_dir / "config" / self.ACCOUNT_TF_VARS @property def backend_tfvars(self): @@ -267,3 +269,25 @@ def get_project_root_or_current_dir_path() -> Path: root = Path.cwd() return root + + +def is_project_yaml_only_bootstrap() -> bool: + """Whether only `project.yaml` exists yet: `project init` has run but `project create` + hasn't, so neither `build.env` nor `config/common.tfvars` exist. PathsHandler can't resolve + a project name in this state; callers should skip it and derive the name from project.yaml. + """ + root = get_project_root_or_current_dir_path() + build_env = root / "build.env" + common_tfvars = Path(get_global_config_path()) / "common.tfvars" + return (root / "project.yaml").exists() and not build_env.exists() and not common_tfvars.exists() + + +def build_paths_and_environment(config: dict) -> tuple[PathsHandler, dict]: + """Build a PathsHandler and the AWS cli environment variables derived from it. Shared by any + group callback that needs to resolve project paths from `state.config`.""" + paths = PathsHandler(config) + environment = { + "AWS_SHARED_CREDENTIALS_FILE": str(paths.aws_credentials_file), + "AWS_CONFIG_FILE": str(paths.aws_config_file), + } + return paths, environment diff --git a/tests/test_modules/test_credentials.py b/tests/test_modules/test_credentials.py index 1933315..1f637ee 100644 --- a/tests/test_modules/test_credentials.py +++ b/tests/test_modules/test_credentials.py @@ -7,6 +7,8 @@ import click import pytest +from leverage import conf as conf_module +from leverage import path as lepath from leverage._internals import State from leverage._utils import ExitError from leverage.modules.credentials import ( @@ -15,6 +17,7 @@ _extract_credentials, _get_mfa_serial, _get_organization_accounts, + _profile_is_configured, _replace_hcl_attribute, configure_credentials, _credentials_are_valid, @@ -43,12 +46,22 @@ def cli_context(runner=None, paths=None, config=None, verbose=False): state.config = config with click.Context(command=click.Command("leverage"), obj=state): - yield + yield state -def awscli_returning(exit_code, output): - """AWS cli runner double whose `exec` returns the given exit code and output.""" - return Mock(exec=Mock(return_value=(exit_code, output, ""))) +def awscli_returning(exit_code, output, error=""): + """AWS cli runner double whose `exec` mimics Runner.exec's `raises=True` default: raises + ExitError on a nonzero exit_code unless the caller passed `raises=False`, exactly like the + real Runner.exec/Runner.run does. This lets a call site that forgets to pass `raises=False` + fail its own test the same way it would fail in production. + """ + + def fake_exec(*args, raises=True, **kwargs): + if raises and exit_code: + raise ExitError(exit_code, f"Command execution failed: {error or output}") + return (exit_code, output, error) + + return Mock(exec=Mock(side_effect=fake_exec)) PROJECT_YAML = { @@ -94,6 +107,73 @@ def test_load_configs_for_credentials(): } +@mock.patch.object(credentials_module, "_load_project_yaml", Mock(return_value={"short_name": "abc"})) +@mock.patch.object(credentials_module, "Runner", Mock()) +def test_credentials_group_bootstraps_paths_from_project_yaml_only(monkeypatch, tmp_path): + """ + Test that `credentials configure` works right after `project init`, before `project create`: + only project.yaml exists (no build.env, no common.tfvars yet). Since the top-level `leverage` + group callback skips PathsHandler in that state (state.paths stays None), the `credentials` + group callback itself should write build.env from project.yaml's short_name and build real, + project-scoped paths. + """ + monkeypatch.setattr(credentials_module, "PROJECT_ROOT", tmp_path) + home = tmp_path / "home" + monkeypatch.setattr(Path, "home", lambda: home) + monkeypatch.setattr(lepath, "get_root_path", lambda: str(tmp_path)) + monkeypatch.setattr(lepath, "get_working_path", lambda: str(tmp_path)) + monkeypatch.setattr(conf_module, "get_root_path", lambda: str(tmp_path)) + monkeypatch.setattr(conf_module, "get_working_path", lambda: str(tmp_path)) + + with cli_context(config={}) as state: + credentials_module.credentials.callback() + + build_env = tmp_path / "build.env" + assert build_env.read_text() == "PROJECT=abc\nTF_IMAGE_TAG=1.1.9" + assert state.paths is not None + assert state.paths.project == "abc" + assert state.environment["AWS_SHARED_CREDENTIALS_FILE"] == str(home / ".aws" / "abc" / "credentials") + assert state.environment["AWS_CONFIG_FILE"] == str(home / ".aws" / "abc" / "config") + + +@mock.patch.object(credentials_module, "_load_project_yaml", Mock(return_value={"short_name": "abc"})) +@mock.patch.object(credentials_module, "Runner", Mock()) +def test_credentials_group_does_not_rebuild_or_clobber_when_already_resolved(monkeypatch, tmp_path): + """ + Test that once state.paths has already been resolved upstream (build.env/common.tfvars + already existed), the `credentials` group callback neither rewrites the existing build.env + nor reconstructs state.paths, even though project.yaml is still present on disk. + """ + monkeypatch.setattr(credentials_module, "PROJECT_ROOT", tmp_path) + build_env = tmp_path / "build.env" + build_env.write_text("PROJECT=abc\nMFA_ENABLED=true\nTF_BINARY=/usr/bin/tofu\n") + + existing_paths = Mock() + with cli_context(paths=existing_paths, config={"PROJECT": "abc"}) as state: + credentials_module.credentials.callback() + + assert build_env.read_text() == "PROJECT=abc\nMFA_ENABLED=true\nTF_BINARY=/usr/bin/tofu\n" + assert state.paths is existing_paths + + +def test_profile_is_configured(): + """Test that an already-configured profile is reported as such.""" + awscli = awscli_returning(0, "some output") + with cli_context(runner=awscli): + assert _profile_is_configured("foo") + + +def test_profile_is_not_configured(): + """ + Regression test: a never-configured profile makes `aws configure list` exit 255 + ("The config profile (...) could not be found"), which must be reported as False, + not raised - `_profile_is_configured` is meant to probe, not crash. + """ + awscli = awscli_returning(255, "The config profile (foo) could not be found") + with cli_context(runner=awscli): + assert not _profile_is_configured("foo") + + @mock.patch.object(credentials_module, "_get_mfa_serial", new=Mock(return_value="mfa123")) @mock.patch.object(credentials_module.shutil, "copy") def test_configure_accounts_profiles(mocked_copy): @@ -128,6 +208,28 @@ def test_configure_accounts_profiles(mocked_copy): assert mocked_config.call_args_list[0][0][1] == expected +@mock.patch.object(credentials_module, "_get_mfa_serial", new=Mock(return_value="mfa123")) +@mock.patch.object(credentials_module.shutil, "copy") +def test_configure_accounts_profiles_skips_backup_when_config_file_does_not_exist(mocked_copy): + """ + Regression test: on a profile's first-ever assumable-roles setup, `~/.aws//config` + has never been written yet (only the credentials file has, via `aws configure set`), so + backing it up must be skipped instead of raising FileNotFoundError. + """ + paths = Mock(aws_config_file=Mock(exists=Mock(return_value=False))) + with cli_context(paths=paths): + with mock.patch.object(credentials_module, "configure_profile"): + configure_accounts_profiles( + "test-management", + "us-test-1", + {"acc1": "12345"}, + [{"name": "acc1"}], + fetch_mfa_device=False, + ) + + mocked_copy.assert_not_called() + + @mock.patch.object(credentials_module, "_get_mfa_serial", new=Mock(return_value="mfa123")) @mock.patch.object(credentials_module.shutil, "copy") def test_configure_accounts_profiles_mfa(mocked_copy): @@ -278,8 +380,8 @@ def test_configure_profile(): configure_profile("test-acc1-oaar-mfa", {"region": "us-test-1", "output": "json"}) assert awscli.exec.call_args_list == [ - mock.call("configure", "set", "region", "us-test-1", "--profile", "test-acc1-oaar-mfa"), - mock.call("configure", "set", "output", "json", "--profile", "test-acc1-oaar-mfa"), + mock.call("configure", "set", "region", "us-test-1", "--profile", "test-acc1-oaar-mfa", raises=False), + mock.call("configure", "set", "output", "json", "--profile", "test-acc1-oaar-mfa", raises=False), ] diff --git a/tests/test_modules/test_tf.py b/tests/test_modules/test_tf.py index 363603a..48bb246 100644 --- a/tests/test_modules/test_tf.py +++ b/tests/test_modules/test_tf.py @@ -1,9 +1,15 @@ +from pathlib import Path from unittest.mock import patch +import click import pytest +from leverage import conf from leverage import leverage -from leverage.modules.tf import has_a_plan_file +from leverage import path as lepath +from leverage._internals import State +from leverage.path import PathsHandler +from leverage.modules.tf import _validate_layout, has_a_plan_file @pytest.mark.parametrize( @@ -58,6 +64,34 @@ def test_init_with_args(leverage_project, leverage_runner): assert called_args[-1] == f"-backend-config={leverage_project / 'account' / 'config' / 'backend.tfvars'}" +def test_validate_layout_checks_the_given_layer_not_cwd(leverage_project, monkeypatch): + """ + Regression test: `_validate_layout` must validate the *layer* it was given, not `paths.cwd`. + Otherwise `leverage tf init --layers a,b` run from an account-level "layers-group" directory + (e.g. account/us-east-1) always fails with "This command can only run at layer level.", + since cwd itself - the layers-group - has no .tf files of its own, only its layer + subdirectories do. + """ + layers_group = leverage_project / "account" / "us-east-1" + layer = layers_group / "security-base" + + monkeypatch.setattr(Path, "cwd", lambda: layers_group) + monkeypatch.setattr(lepath, "get_working_path", lambda: layers_group) + monkeypatch.setattr(lepath, "get_root_path", lambda: leverage_project) + monkeypatch.setattr(conf, "get_root_path", lambda: leverage_project) + monkeypatch.setattr(conf, "get_working_path", lambda: layers_group) + + state = State() + state.verbosity = False + state.config = conf.load() + state.paths = PathsHandler(state.config) + + with click.Context(command=click.Command("leverage"), obj=state): + # Must not raise ExitError("This command can only run at layer level."), which it would + # if check_for_layer_location() fell back to checking cwd (the layers-group) instead. + _validate_layout(layer) + + @pytest.mark.parametrize( "args, expected_output", [ diff --git a/tests/test_path.py b/tests/test_path.py index 9eae5cd..d2593a9 100644 --- a/tests/test_path.py +++ b/tests/test_path.py @@ -14,6 +14,7 @@ get_build_script_path, get_account_path, get_project_root_or_current_dir_path, + is_project_yaml_only_bootstrap, NotARepositoryError, ) @@ -109,6 +110,62 @@ def test_get_build_script_path_no_build_script(dir_structure): assert get_build_script_path() is None +def test_is_project_yaml_only_bootstrap_true(monkeypatch, tmp_path): + monkeypatch.setattr(lepath, "get_root_path", lambda: str(tmp_path)) + (tmp_path / "project.yaml").touch() + + assert is_project_yaml_only_bootstrap() is True + + +def test_is_project_yaml_only_bootstrap_false_without_project_yaml(monkeypatch, tmp_path): + monkeypatch.setattr(lepath, "get_root_path", lambda: str(tmp_path)) + + assert is_project_yaml_only_bootstrap() is False + + +def test_is_project_yaml_only_bootstrap_false_with_build_env(monkeypatch, tmp_path): + monkeypatch.setattr(lepath, "get_root_path", lambda: str(tmp_path)) + (tmp_path / "project.yaml").touch() + (tmp_path / "build.env").touch() + + assert is_project_yaml_only_bootstrap() is False + + +def test_is_project_yaml_only_bootstrap_false_with_common_tfvars(monkeypatch, tmp_path): + monkeypatch.setattr(lepath, "get_root_path", lambda: str(tmp_path)) + (tmp_path / "project.yaml").touch() + (tmp_path / "config").mkdir() + (tmp_path / "config" / "common.tfvars").touch() + + assert is_project_yaml_only_bootstrap() is False + + +@pytest.mark.parametrize( + "mfa_enabled_value, expected", + [ + ("true", True), + ("True", True), + ("TRUE", True), + ("false", False), + ("False", False), + (None, False), + ], +) +def test_mfa_enabled_is_cast_to_bool(mfa_enabled_value, expected): + """ + Regression test: PathsHandler.mfa_enabled must be a real bool, not the raw string from + build.env - a non-empty string like "false" is truthy in Python, which would otherwise make + `elif paths.mfa_enabled:` (leverage/modules/auth.py) always fire regardless of its value. + """ + env_conf = {"PROJECT": "test"} + if mfa_enabled_value is not None: + env_conf["MFA_ENABLED"] = mfa_enabled_value + + paths = PathsHandler(env_conf) + + assert paths.mfa_enabled is expected + + def test_check_for_cluster_layer(muted_click_context, propagate_logs): """ Test that if we are not on a cluster layer, we raise an error.