Skip to content

Commit

Permalink
[BB-248] Improving test coverage (#249)
Browse files Browse the repository at this point in the history
* tfautomv invoke test

* configure_accounts_profiles tests

* _extract_credentials happy path test

* trigger also on push

* separate unit from integration tests

* get mfa serial + get organization accounts tests

* backup_file + profile_is_configured tests

* configure_credentials tests

* _get_management_account_id + _credentials_are_valid tests

* configure_profile + _update_account_ids tests

* splitting test_configure_accounts_profiles test + comments

* more comments + remove non useful tests

* comments + clean up
  • Loading branch information
Franr authored Apr 10, 2024
1 parent d0303b6 commit 4018cab
Show file tree
Hide file tree
Showing 5 changed files with 276 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -1,27 +1,8 @@
name: Tests | Unit and Integration
name: Tests | Integration

on: [pull_request, workflow_dispatch]

jobs:
unit_tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: install_requirements
run: |
echo "[INFO] Installing requirements..."
pip install -r dev-requirements.txt
- name: run_unit_tests
run: |
echo "[INFO] Running unit tests and generate coverage report"
pytest --verbose
shell: bash

- name: Report Coveralls
uses: coverallsapp/github-action@v2

integration_tests:
runs-on: ubuntu-latest
steps:
Expand Down
23 changes: 23 additions & 0 deletions .github/workflows/tests-unit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Tests | Unit

on: [push, pull_request]

jobs:
unit_tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: install_requirements
run: |
echo "[INFO] Installing requirements..."
pip install -r dev-requirements.txt
- name: run_unit_tests
run: |
echo "[INFO] Running unit tests and generate coverage report"
pytest --verbose
shell: bash

- name: Report Coveralls
uses: coverallsapp/github-action@v2
24 changes: 10 additions & 14 deletions leverage/modules/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from leverage import __toolbox_version__
from leverage import logger
from leverage._utils import ExitError
from leverage.path import get_root_path
from leverage.path import get_global_config_path
from leverage.path import NotARepositoryError
Expand Down Expand Up @@ -446,8 +447,7 @@ def configure_credentials(profile, file=None, make_backup=False):
for key, value in values.items():
exit_code, output = AWSCLI.exec(f"configure set {key} {value}", profile)
if exit_code:
logger.error(f"AWS CLI error: {output}")
raise Exit(exit_code)
raise ExitError(exit_code, f"AWS CLI error: {output}")


def _credentials_are_valid(profile):
Expand Down Expand Up @@ -481,8 +481,7 @@ def _get_management_account_id(profile):
"""
exit_code, caller_identity = AWSCLI.exec("--output json sts get-caller-identity", profile)
if exit_code:
logger.error(f"AWS CLI error: {caller_identity}")
raise Exit(exit_code)
raise ExitError(exit_code, f"AWS CLI error: {caller_identity}")

caller_identity = json.loads(caller_identity)
return caller_identity["Account"]
Expand Down Expand Up @@ -526,8 +525,7 @@ def _get_mfa_serial(profile):
"""
exit_code, mfa_devices = AWSCLI.exec("--output json iam list-mfa-devices", profile)
if exit_code:
logger.error(f"AWS CLI error: {mfa_devices}")
raise Exit(exit_code)
raise ExitError(exit_code, f"AWS CLI error: {mfa_devices}")
mfa_devices = json.loads(mfa_devices)

# Either zero or one MFA device should be configured for either `management` or `security` accounts users.
Expand All @@ -554,8 +552,7 @@ def configure_profile(profile, values):
for key, value in values.items():
exit_code, output = AWSCLI.exec(f"configure set {key} {value}", profile)
if exit_code:
logger.error(f"AWS CLI error: {output}")
raise Exit(exit_code)
raise ExitError(exit_code, f"AWS CLI error: {output}")


def configure_accounts_profiles(profile, region, organization_accounts, project_accounts, fetch_mfa_device):
Expand All @@ -565,18 +562,17 @@ def configure_accounts_profiles(profile, region, organization_accounts, project_
profile(str): Name of the profile to configure.
region (str): Region.
organization_accounts (dict): Name and id of all accounts in the organization.
project_accounts (dict): Name and email of all accounts in project configuration file.
project_accounts (list): Name and email of all accounts in project configuration file.
fetch_mfa_device (bool): Whether to fetch MFA device for profiles.
"""
short_name, type = profile.split("-")
short_name, _type = profile.split("-")

mfa_serial = ""
if fetch_mfa_device:
logger.info("Fetching MFA device serial.")
mfa_serial = _get_mfa_serial(profile)
if not mfa_serial:
logger.error("No MFA device found for user.")
raise Exit(1)
raise ExitError(1, "No MFA device found for user.")

account_profiles = {}
for account in project_accounts:
Expand All @@ -593,13 +589,13 @@ def configure_accounts_profiles(profile, region, organization_accounts, project_
account_profile = {
"output": "json",
"region": region,
"role_arn": f"arn:aws:iam::{account_id}:role/{PROFILES[type]['role']}",
"role_arn": f"arn:aws:iam::{account_id}:role/{PROFILES[_type]['role']}",
"source_profile": profile,
}
if mfa_serial:
account_profile["mfa_serial"] = mfa_serial
# A profile identifier looks like `le-security-oaar`
account_profiles[f"{short_name}-{account_name}-{PROFILES[type]['profile_role']}"] = account_profile
account_profiles[f"{short_name}-{account_name}-{PROFILES[_type]['profile_role']}"] = account_profile

logger.info("Backing up account profiles file.")
_backup_file("config")
Expand Down
4 changes: 0 additions & 4 deletions leverage/modules/tfautomv.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import re

import hcl2
import click
from click.exceptions import Exit

from leverage import logger
from leverage._internals import pass_state
from leverage._internals import pass_container
from leverage.container import get_docker_client
Expand Down
Loading

0 comments on commit 4018cab

Please sign in to comment.