search

jason-johnson / azure-pipelines-tasks-terraform

Azure Pipelines extension for Terraform
MIT License
127 stars 55 forks source link

Add support for state commands list, move, and rm #245

Closed loispostula closed 2 years ago

loispostula commented 2 years ago

This PR resolve the issue https://github.com/charleszipp/azure-pipelines-tasks-terraform/issues/202

charleszipp commented 2 years ago

Appreciate you helping with the review @SimonAlling!

loispostula commented 2 years ago

Thanks so much for the contribution @loispostula. The only issue I see is the repurposing of workspaceSubCommand to subCommand. I would request you leave workspaceSubCommand as its own input exclusively for workspace. The reason is the renaming of this input will break existing pipelines that have this input defined.

Instead of having workspace and state share the same input for subcommand, lets create a stateSubCommand input. Use stateSubCommand in the state command handlers. Then, revert the changes to workspace command handlers. Don't forget to update the tasks/terraform-cli/task.json to have the new stateSubCommand input. This input follow similar configuration/definition as workspaceSubCommand.

Finally, please be sure to address the codeql comments. There are some unused imports in several files.

@charleszipp I did not think of the existing pipelines, only on simplifying management of Terraform subcommand. It is a very valuable input, I've adapted the PR accordingly

charleszipp commented 2 years ago

@loispostula would you mind updating the overview.md also to have examples of how to use these new commands? I usually just copy/paste what I setup in the test pipeline.

Also if you can share email I can add you to the ADO project that runs the CI pipeline so you can monitor any failures.

charleszipp commented 2 years ago

@loispostula the pipeline is getting a different error now. Given this worked for you locally, but its failing in the pipeline, I think state just needs to be added to the command input pickList. I would just push this myself but, I cant push to your fork.

image image

charleszipp commented 2 years ago

merging. will fix the pipeline with follow up PR.