search

shakacode / control-plane-flow

The power of Kubernetes with the ease of Heroku! Our playbook for migrating from Heroku to Control Plane, controlplane.com, and CPL CLI source
MIT License
43 stars 2 forks source link

Introduce terraform subcommand #221

Closed zzaakiirr closed 1 month ago

zzaakiirr commented 1 month ago

What does this PR do?

This PR adds possibility to add subcommands to cpflow cli tool and adds cpflow terraform generate command which does nothing (for now)

Note

Screenshots

Screenshot 2024-07-30 at 13 29 22 Screenshot 2024-07-30 at 13 30 06 Screenshot 2024-07-30 at 13 30 38

Summary by CodeRabbit

coderabbitai[bot] commented 1 month ago

[!IMPORTANT]

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The updates introduce significant enhancements to the cpflow CLI by adding a new generate command for Terraform configuration, improving command discovery and handling of subcommands, and structuring subcommands more effectively. These changes streamline the user experience, making it easier to manage infrastructure as code and facilitating better organization within the command-line application. Testing enhancements ensure that the new functionalities work as intended.

Changes

File Change Summary
docs/commands.md Added generate command documentation for generating Terraform configuration files from controlplane.yml.
lib/command/base.rb Enhanced all_commands to support recursive file discovery and proper namespace handling for commands.
lib/command/base_sub_command.rb Introduced BaseSubCommand class for structured definition of subcommands, including methods for command banners and subcommand prefixes.
lib/command/terraform/generate.rb Created Generate class within Command::Terraform for generating Terraform configurations; includes command metadata and placeholders for future implementation.
lib/cpflow.rb Added subcommand_names and klass_for methods to dynamically handle subcommands and improve command processing.
spec/cpflow_spec.rb Added tests to verify correct handling of subcommands and their help output in the CLI.
spec/support/command_helpers.rb Enhanced run_cpflow_command to manage the global $PROGRAM_NAME; introduced package_name method for better code organization.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Command
    participant Terraform

    User->>CLI: cpflow terraform generate
    CLI->>Command: Initialize Generate Command
    Command->>Terraform: Generate configuration files
    Terraform-->>Command: Return generated files
    Command-->>CLI: Display success message
    CLI-->>User: Show confirmation of files generated

🐰 In the warren where changes bloom,
New commands bring joy, dispelling gloom.
With subcommands now in their place,
Infrastructure flows with grace.
A hop, a skip, we generate with glee,
Oh, what a wondrous CLI, come see! 🎉

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` 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 as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
zzaakiirr commented 1 month ago

@coderabbitai review

coderabbitai[bot] commented 1 month ago
Actions performed Review triggered. > Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
zzaakiirr commented 1 month ago

Just a partial review so far, but I almost wonder if it wouldn't be easier to just use : like we do for ps:start, ps:stop, etc. Like terraform:generate. Are subcommands preferrable?

I missed that 🥲

I still prefer subcommands because user can get all terraform commands by invoking cpflow terraform --help. The problem with : approach is that user needs to invoke cpflow help for each command in this namespace

Another issue with this approach is class names which must include subcommand namespace in class names (class Ps<CommandName> for example), declaring separate module is better (IMHO)

I won't push on subcommand approach @rafaelgomesxyz @borela WDYT?

borela commented 1 month ago

@zzaakiirr I am in favor of your design as it is more common in UNIX for CLIs that support subcommands.

@justin808 @rafaelgomesxyz @dzirtusss @zzaakiirr I would prefer to change the loading logic to just reopen the CLI class for each command and declare them like Thor docs says, e.g.:

# command_1.rb
class CLI < Thor
  desc "usage", "desc"
  long_desc "long desc"
  def command_1
  end
end

# command_2.rb
class CLI < Thor
  desc "usage", "desc"
  long_desc "long desc"
  def command_2
  end
end

...

By reopening the class this way, you can still share state, methods etc... and still follow Thor's design.

rafaelgomesxyz commented 1 month ago

@borela We have a bunch of custom logic that benefits from the current loading logic, as you can see in https://github.com/shakacode/control-plane-flow/blob/main/lib/cpflow.rb#L168.

What are the benefits of switching to your proposed logic?

Thor doesn't really have a design for defining commands in separate files, so I'm not sure what you mean by that.

I think we should keep it the way it is.

borela commented 1 month ago

@rafaelgomesxyz, it looks like you are re-implementing Thor's features; Which features that custom logic implement that Thor already doesn't support? Validations? That could be extracted to a method that accepts a value and pattern to validate against.

rafaelgomesxyz commented 1 month ago

@borela We're not though, we just have a layer on top of Thor to make things easier. It's not just validations, but also handling of deprecated commands and special args / options cases, as well as setting some fallback values here and there.

Regardless, if we were to reopen the class in every command file, we'd have to repeat all of this logic for each file. Sure, we could move some logic to common helpers, but we'd still have repetition, which is why this way seems better to me. I don't see any benefits in switching to the way you proposed.

borela commented 1 month ago

@rafaelgomesxyz The code for each command would be linear and less magical. A little bit of repetition is fine if the end result is simpler to read.

In an example scenario, command x is having a validation issue. I will go to the command x file and see how validation is being performed (either directly or by calling an accessory function) inside the command's method.

That does not happen with the current design, it is not immediately obvious where simple stuff like validations are happening or that those options are being validated. In contrast, other projects using thor just do simple ||= for fallback values and validate the option on the command itself, which again can be extracted to a method and called by the command.

rafaelgomesxyz commented 1 month ago

Let's see what Sergey and Justin think, but I still think we should keep it as is.

I think once you understand the constants defined at the top of the command classes and how options are defined in Base, it becomes pretty straightforward to figure out how something is being validated.

Another thing I like about the current approach is that we have a somewhat centralized place for all Thor stuff, which allows us to have a better separation of concerns, and detaches our app from Thor. It would be a lot easier to adapt the code if we wanted to replace Thor with something else - I know Justin was considering moving to an npm package so we have both the CPLN CLI and our CLI in the same place, and although that wouldn't help too much in this case, because we'd still need to convert from Ruby to JavaScript, I think it would definitely make things easier.

We also have some configs set on the class level based on the commands, which benefits from the current approach:

https://github.com/shakacode/control-plane-flow/blob/82ff517297fa79e066192d9d298f32c8820b77d2/lib/cpflow.rb#L246-L248

These cannot be set individually, so we'd still need a way to go through all command classes looking for matches.

borela commented 1 month ago

@rafaelgomesxyz Looking at the source, just build image has ACCEPTS_EXTRA_OPTIONS set to true, and it looks like it only does this because it forwards them to docker.

Why not declare the docker options supported? The error message will be better and when somebody does cpflow build-image --help they will see them too.

borela commented 1 month ago

@rafaelgomesxyz if zakir pr is following the current pattern and working we can merge this pr and I can move my suggestions to an issue.