BL-255 | Add tofu command to cli
#305
Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughThe changes introduce support for both Terraform and OpenTofu by refactoring the container logic and command-line interface. The main container class is renamed and updated to select the appropriate binary based on a new parameter. Command groups for both Terraform and OpenTofu are established, sharing the same subcommands and logic. Imports and references across modules and tests are updated to use the new container class. Version validation logic is adjusted to handle OpenTofu-specific version tags. New and updated tests ensure correct behavior for both Terraform and OpenTofu workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant TFContainer
participant Terraform
participant OpenTofu
CLI->>TFContainer: Initialize (terraform=True/False)
alt terraform=True
TFContainer->>Terraform: Set entrypoint to /bin/terraform
CLI->>Terraform: Execute command
else terraform=False
TFContainer->>OpenTofu: Set entrypoint to /bin/tofu
CLI->>OpenTofu: Execute command
end
sequenceDiagram
participant User
participant CLI
participant tofu_cmd_group
participant terraform_cmd_group
User->>CLI: Run 'tofu <subcommand>' or 'terraform <subcommand>'
CLI->>tofu_cmd_group: If 'tofu', dispatch subcommand
CLI->>terraform_cmd_group: If 'terraform', dispatch subcommand
tofu_cmd_group->>TFContainer: Initialize (terraform=False)
terraform_cmd_group->>TFContainer: Initialize (terraform=True)
TFContainer->>AppropriateBinary: Run subcommand
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_containers/test_terraform.py (1)
50-55: Hard-coded path couples test to container user
/home/leverage/…may change ifCONTAINER_USERis altered. Derive the expected value from the class to keep the test resilient:-assert tf_container.auth_method() == "/home/leverage/scripts/aws-mfa/aws-mfa-entrypoint.sh -- " +expected = f"/home/{TFContainer.CONTAINER_USER}/scripts/aws-mfa/aws-mfa-entrypoint.sh -- " +assert tf_container.auth_method() == expected
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
leverage/container.py(3 hunks)leverage/containers/kubectl.py(3 hunks)leverage/leverage.py(2 hunks)leverage/modules/__init__.py(1 hunks)leverage/modules/project.py(2 hunks)leverage/modules/shell.py(2 hunks)leverage/modules/terraform.py(12 hunks)tests/test_containers/test_terraform.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
leverage/modules/__init__.py (1)
leverage/modules/terraform.py (2)
terraform(44-55)tofu(27-38)
leverage/leverage.py (1)
leverage/modules/terraform.py (2)
tofu(27-38)terraform(44-55)
leverage/containers/kubectl.py (1)
leverage/container.py (4)
TFContainer(436-687)_start_with_output(267-279)entrypoint(115-116)entrypoint(119-120)
leverage/container.py (1)
leverage/modules/terraform.py (1)
terraform(44-55)
tests/test_containers/test_terraform.py (4)
leverage/container.py (5)
TFContainer(436-687)start_shell(626-632)enable_sso(547-549)refresh_credentials(597-600)auth_method(506-528)tests/test_containers/__init__.py (1)
container_fixture_factory(12-22)tests/conftest.py (1)
muted_click_context(48-50)leverage/modules/terraform.py (1)
refresh_credentials(207-211)
🪛 Ruff (0.8.2)
leverage/modules/__init__.py
3-3: .terraform.tofu imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
3-3: .terraform.terraform imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🪛 GitHub Actions: Tests | Lint w/ Black
leverage/container.py
[error] 1-1: Prettier formatting check failed. File would be reformatted.
leverage/modules/terraform.py
[error] 1-1: Prettier formatting check failed. File would be reformatted.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration_tests_cli_refarch (3.9.15, 1.5.0-0.2.0)
🔇 Additional comments (13)
leverage/container.py (5)
436-439: Good rename for improved clarityRenaming from
TerraformContainertoTFContaineraccurately reflects the container's new dual functionality supporting both Terraform and OpenTofu.
440-442: Well-defined constants for binary pathsThe addition of
TERRAFORM_BINARYandTOFU_BINARYconstants provides clear references to the respective binaries and follows good programming practices by avoiding hardcoded string literals.
446-446: Good default choice for OpenTofuThe
terraformparameter defaulting toFalsealigns well with the PR objective of transitioning from Terraform to OpenTofu, making OpenTofu the default choice.
479-479: Clean ternary implementation for binary selectionThe conditional entrypoint setting is concise and effective, properly implementing the binary selection logic.
690-690: Consistent class inheritance updateThe
TFautomvContainerclass now correctly inherits from the renamedTFContainerclass, maintaining proper inheritance hierarchy.leverage/modules/project.py (2)
24-24: Updated import to use new container class nameThe import statement has been properly updated to reference the renamed
TFContainerclass.
329-329: Updated container instantiationThe code now correctly instantiates the renamed
TFContainerclass.leverage/modules/__init__.py (1)
3-3:✅ Verification successful
Added import for new
tofucommandThe import statement now includes both
tofuandterraform, supporting the new CLI command structure. However, static analysis indicates these imports may be unused.Let's verify that these imports are actually being used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash # Check if these imports are used in the main CLI registration code rg -A 5 -B 5 "from .modules import .*tofu" --type pyLength of output: 54
🏁 Script executed:
#!/bin/bash # Search for any references to "tofu" across all Python files rg -n "tofu" --type pyLength of output: 847
🏁 Script executed:
#!/bin/bash # Search specifically for calls or registrations (e.g., tofu()) in the CLI rg -n "tofu\(" --type pyLength of output: 92
Imports Confirmed in CLI Registration
The
tofuandterraformimports inleverage/modules/__init__.pyare actively used for CLI command setup:• leverage/leverage.py:14 – imports tofu, terraform
• leverage/leverage.py:53–54 – registerstofuas a CLI command (with alias “tf”)No changes required.
🧰 Tools
🪛 Ruff (0.8.2)
3-3:
.terraform.tofuimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
3-3:
.terraform.terraformimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
leverage/modules/shell.py (2)
4-4: Updated import to use new container class nameThe import statement has been properly updated to reference the renamed
TFContainerclass.
31-33: Updated comment and container instantiationBoth the TODO comment and container instantiation have been properly updated to reference the renamed
TFContainerclass.leverage/leverage.py (1)
53-55: Alias change may confuse existing users of thetfshorthand
tfused to point to the Terraform command but now maps to OpenTofu. Anyone relying onleverage tf …semantics will silently start using OpenTofu without noticing.Consider at least one of the following:
- Emit a deprecation warning the first time the alias is invoked, explaining the switch.
- Keep the old alias for Terraform (
tf-terraformor similar) during a grace period.- Update all user-facing docs and completion scripts as part of this PR.
This will minimise surprises in automation and CI pipelines.
leverage/containers/kubectl.py (1)
27-32: Inheritance change looks goodSwitching from
TerraformContainertoTFContainerkeeps the class aligned with the new unified container abstraction and removes redundant code.
No issues spotted here.leverage/modules/terraform.py (1)
23-39: Good reuse patternCreating a separate
tofugroup that re-uses all existing sub-commands keeps the code DRY and makes future maintenance easier. Nice refactor!
angelofenoglio
force-pushed
the
feat/add-opentofu-command
branch
from
7152554 to
8a74269
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
leverage/modules/__init__.py (1)
3-3: Export newtofucommand group alongsideterraform.Importing both
tofuandterraformis necessary for the CLI’s dynamic command registration. To prevent F401 lint errors for unused imports in this module, explicitly declare them as public exports or suppress the warning:Option A: Define
__all__to list public members:from .run import run from .project import project -from .terraform import tofu, terraform +from .terraform import tofu, terraform + +__all__ = [ + "run", + "project", + "tofu", + "terraform", + "tfautomv", + "kubectl", + "shell", +]Option B: Add a
noqacomment to silence the unused-import warning:-from .terraform import tofu, terraform +# noqa: F401 +from .terraform import tofu, terraform # noqa: F401🧰 Tools
🪛 Ruff (0.8.2)
3-3:
.terraform.tofuimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
3-3:
.terraform.terraformimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
leverage/container.py(3 hunks)leverage/containers/kubectl.py(3 hunks)leverage/leverage.py(2 hunks)leverage/modules/__init__.py(1 hunks)leverage/modules/project.py(2 hunks)leverage/modules/shell.py(2 hunks)leverage/modules/terraform.py(12 hunks)tests/test_containers/test_terraform.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- leverage/modules/project.py
- leverage/modules/shell.py
- leverage/leverage.py
- leverage/containers/kubectl.py
- tests/test_containers/test_terraform.py
- leverage/container.py
- leverage/modules/terraform.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
leverage/modules/__init__.py (1)
leverage/modules/terraform.py (2)
terraform(44-55)tofu(27-38)
🪛 Ruff (0.8.2)
leverage/modules/__init__.py
3-3: .terraform.tofu imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
3-3: .terraform.terraform imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration_tests_cli_refarch (3.9.15, 1.5.0-0.2.0)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_conf.py (1)
66-68: Update test docstring to reflect OpenTofu support.The docstring should be updated to mention the dual support for Terraform and OpenTofu, as the test now validates both scenarios.
""" - Test that we get a warning if we are working with a version lower than the required by the project. + Test that we get a warning if we are working with a version lower than the required by the project. + For OpenTofu configurations (with 'tofu' in the image tag), the Terraform version check is skipped. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
leverage/leverage.py(3 hunks)tests/test_conf.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- leverage/leverage.py
🧰 Additional context used
🪛 GitHub Actions: Tests | Lint w/ Black
tests/test_conf.py
[error] 1-1: Prettier formatting check failed. Run 'black --write' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration_tests_cli_refarch (3.9.15, 1.5.0-0.2.0)
🔇 Additional comments (4)
tests/test_conf.py (4)
59-64: Parameterized test now appropriately handles both Terraform and OpenTofu scenarios.The test has been updated to run with two different configurations: one for OpenTofu and one for Terraform. This approach ensures proper validation of version strings in both contexts.
71-71: Simplification of mock configuration.The mock configuration has been updated to use the parameterized values, which makes the test more maintainable.
76-79: Conditional version warning assertion based on OpenTofu presence.The test now conditionally checks for Terraform version warnings only when not using OpenTofu, which aligns with the expected behavior changes in the main code.
59-83:❓ Verification inconclusive
Fix code formatting issues.
The pipeline failure indicates that the file has formatting issues. Please run Black formatter to resolve these issues.
🏁 Script executed:
#!/bin/bash # Run black to check formatting issues black --check tests/test_conf.pyLength of output: 130
Formatting check inconclusive – please verify Black is installed and formatting
It looks like the
blackcommand isn’t available in the sandbox, so we can’t confirm formatting automatically. To ensure pipeline success:
- Install Black (e.g.
pip install black) or add it to your CI environment.- Run
black --check tests/test_conf.pylocally to confirm there are no formatting issues.- Commit any changes Black suggests to keep code style consistent.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
leverage/modules/tf.py (1)
347-351:⚠️ Potential issueList-comprehension bug – letter indexing instead of list indexing
arg[index - 1]indexes a character of the current argument string; you meant the previous argument in the list (args[index-1]).
This causes wrong filtering when stripping-backend-configpairs and will break for one-character arguments (IndexError).- args = [ - arg - for index, arg in enumerate(args) - if not arg.startswith("-backend-config") or not arg[index - 1] == "-backend-config" - ] + args = [ + arg + for index, arg in enumerate(args) + # Skip the value that immediately follows a -backend-config flag + if not arg.startswith("-backend-config") or (index == 0 or args[index - 1] != "-backend-config") + ]🧰 Tools
🪛 Ruff (0.8.2)
350-350: Use
arg[index - 1] != "-backend-config"instead ofnot arg[index - 1] == "-backend-config"Replace with
!=operator(SIM201)
🧹 Nitpick comments (2)
leverage/modules/__init__.py (1)
3-3: Unused imports reported by Ruff – expose them via__all__or remove
tofuandterraformare imported for their side-effects but never referenced in this module, triggering F401.
If the intent is to make them part of the public API, explicitly list them in__all__; otherwise drop the import to silence the warning.from .tf import tofu, terraform + +# Public re-exports +__all__ = [ + "run", + "project", + "tofu", + "terraform", + "tfautomv", + "kubectl", + "shell", +]🧰 Tools
🪛 Ruff (0.8.2)
3-3:
.tf.tofuimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
3-3:
.tf.terraformimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
leverage/modules/tf.py (1)
221-236: Keep registration list and command set in syncManually maintaining the tuple of subcommands is error-prone (e.g.,
versionalready slipped through).
Consider collecting commands dynamically:for _cmd in globals().values(): if isinstance(_cmd, click.core.Command) and _cmd.name not in ("tofu", "terraform"): tofu.add_command(_cmd) terraform.add_command(_cmd)This guarantees new commands are automatically exposed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
leverage/modules/__init__.py(1 hunks)leverage/modules/tf.py(13 hunks)tests/test_containers/test_tf.py(1 hunks)tests/test_modules/test_tf.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/test_modules/test_tf.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/test_containers/test_tf.py (3)
leverage/container.py (3)
TFContainer(436-686)enable_sso(546-548)auth_method(505-527)tests/test_containers/__init__.py (1)
container_fixture_factory(12-22)tests/conftest.py (1)
muted_click_context(48-50)
leverage/modules/__init__.py (1)
leverage/modules/tf.py (2)
tofu(28-39)terraform(46-57)
🪛 Ruff (0.8.2)
leverage/modules/__init__.py
3-3: .tf.tofu imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
3-3: .tf.terraform imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration_tests_cli_refarch (3.9.15, 1.5.0-0.2.0)
Pull Request Test Coverage Report for Build 15832423341Details
💛 - Coveralls |
exequielrafaela
added
major
feature
enhancement
What?
tofucommandtfbe the shorthand fortofuinstead ofterraformWhy?
References
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
TF_IMAGEandTF_IMAGE_TAG.Tests
Chores