Adding support for Terraform Destroy. Adding ConfirmApply customization

[jleopold28]

Support for TF destroy (#88)

Also adding support to customize the ConfirmApply messages. (#24)

[vincentclee]

This is some great work @jleopold28 ! Thoughts on adding -refresh=false to the default options for destroy?

Common Scenario
hashicorp/terraform#15386 (comment)

@kmanning

[jleopold28]

@vincentclee thanks for testing! I don't think -refresh=false should be a default option, but I am in favor of allowing it to be configured.

[kmanning]

@vincentclee @jleopold28 ?

In regards to the -refresh=false, does anything need to be done from a code perspective? Would TerraformApply.withArgument('-refresh=false') work?

[kmanning]

Oh! I also finally published the new Contributor instructions. Let's do away with long-lived dev branches, and queue work in master instead. It's simpler that way. (Ie: you can make your PR against master, and I'll delete v5.9-dev - there's nothing in there at the moment). https://github.com/manheim/terraform-pipeline#how-to-contribute

[jleopold28]

@kmanning I updated the base to master. I also removed the section in the docs talking about "Destroy after Deploy". We can discuss that as a future piece of work.

[jleopold28]

@vincentclee @jleopold28 ?

In regards to the -refresh=false, does anything need to be done from a code perspective? Would TerraformApply.withArgument('-refresh=false') work?

That would work. However, I don't think that should be the default behavior...

[kmanning]

You don't think "-refresh=false" should be the default behavior, or you don't think users should need to use .withArgument in order to get this to work?

[jleopold28]

You don't think "-refresh=false" should be the default behavior, or you don't think users should need to use .withArgument in order to get this to work?

@kmanning I think users should have to use withArgument or something similar to add that option.

When doing a destroy, I think we should always refresh the state before executing a destroy. If users are hitting a bug that requires them to add refresh=false, they should specify that with arguments during the plugin init.

[kmanning]

This new state of TerraformEnvironmentStage.DESTROY. How will this behavior with the existing plugins that are part of the library (or worse, plugins that are in completely external libraries?)

eg:

• terraform-pipeline/src/ParameterStoreExecPlugin.groovy

  Lines 14 to 20 in 74de97b

  	public void apply(TerraformEnvironmentStage stage) {
  	def environment = stage.getEnvironment()
  	def parameterStorePath = pathForEnvironment(environment)
  	stage.decorate(PLAN, addEnvVariables(parameterStorePath))
  	stage.decorate(APPLY, addEnvVariables(parameterStorePath))
  	}

• terraform-pipeline/src/AnsiColorPlugin.groovy

  Lines 11 to 14 in 74de97b

  	public void apply(TerraformEnvironmentStage stage) {
  	stage.decorate(PLAN, addColor())
  	stage.decorate(APPLY, addColor())
  	}

• terraform-pipeline/src/ParameterStoreBuildWrapperPlugin.groovy

  Lines 17 to 29 in 817be51

  	@Override
  	public void apply(TerraformEnvironmentStage stage) {
  	def environment = stage.getEnvironment()
  	def parameterStorePath = pathForEnvironment(environment)
  	def options = [
  	path: parameterStorePath,
  	credentialsId: "${environment.toUpperCase()}_PARAMETER_STORE_ACCESS"
  	]
  	stage.decorate(PLAN, addParameterStoreBuildWrapper(options))
  	stage.decorate(APPLY, addParameterStoreBuildWrapper(options))
  	}

• terraform-pipeline/src/CrqPlugin.groovy

  Lines 11 to 16 in 00e3eed

  	@Override
  	public void apply(TerraformEnvironmentStage stage) {
  	def environment = stage.getEnvironment()
  	stage.decorate(TerraformEnvironmentStage.APPLY, addCrq(environment))
  	}

[jleopold28]

@kmanning I am making some changes now. I will have to add stage.decorate(DESTROY, someMethod(options)) to the above plugins. This will add functionality if DESTROY is enabled. Otherwise this will not change the existing functionality.

[kmanning]

Nice touch on the screenshot - thank you for that.

[kmanning]

Thanks for accounting for the other plugins. I'm still concerned about the maintainability of the existing PLAN/APPLY hooks, and I'm concerned about introducing more problems with the new DESTROY hook. But I think that can be tackled in a separate PR/refactor.

I'd like to track and tackle that refactor before the Destroy functionality gets released. #255

[kmanning]