Skip to content
This repository has been archived by the owner on Feb 15, 2022. It is now read-only.

Bedrock should allow for BYO resource group #474

Closed
jmspring opened this issue Jul 10, 2019 · 8 comments · Fixed by #549
Closed

Bedrock should allow for BYO resource group #474

jmspring opened this issue Jul 10, 2019 · 8 comments · Fixed by #549
Assignees
Labels

Comments

@jmspring
Copy link
Contributor

As a: SRE

I want: to be able to bring my own resource group

So that: Terraform can use my resource group and not delete it upon a Terraform delete.

Does this require updates to documentation?: Yes, there will be a new boolean in the terraform variables file for each environment such that one can toggle on/off resource group management

@jmspring jmspring added the enhancement New feature or request label Jul 10, 2019
@jmspring jmspring self-assigned this Jul 10, 2019
@mtarng
Copy link
Contributor

mtarng commented Jul 10, 2019

Locking resource groups seems to work. https://docs.microsoft.com/en-us/azure/azure-resource-manager/resource-group-lock-resources

@evillgenius75 verified that deploying through Terraform to a resource group that is:

  1. locked
  2. has existing resources
    when running terraform destroy, the existing resource groups and its original resources will remain, but the resources created by terraform were deleted.

@evillgenius75
Copy link

Another method is to easily add a lifecycle hook to the resource group stanza in Terraform to add:

resource "azurerm_resource_group" "cluster_rg" {
  name     = "${var.resource_group_name}"
  location = "${var.resource_group_location}"
  lifecycle {
    prevent_destroy = true
  }
}

Then on destroy append the terraform destroy with the -target=<tf_resource_name> to destroy all resources but the RG

This will effectively lock the state file on destroy for the RG
This should be easier than forcing infra owners to use resource group locks in azure.

@jmspring
Copy link
Contributor Author

@evillgenius75 while true, take a look at this open issue -- hashicorp/terraform#17599

There still is a risk of RG deletion.

@jmspring
Copy link
Contributor Author

I'll have a checked in first pass solution in about an hour. Just running through all the environments. I will need to add docs too.

@evillgenius75
Copy link

evillgenius75 commented Jul 15, 2019

@jmspring with that in mind we should then conform to the Azure 2.0 provider process which will require strict import of any resources. Since this will be the new provider soon it would be better we not make code changes to allow for existing RG's through the template process as this is a bad process taking advantage of a known bad issue with the current provider. We should only allow existing RGs through an explicit import process :
https://www.terraform.io/docs/providers/azurerm/guides/2.0-upgrade-guide.html#changes-when-importing-existing-resources
We can just pin to a version higher than 1.22 and document that the environment variable AZURE_PROVIDER_STRICT is set to true on the TF workspace being used.

@jmspring
Copy link
Contributor Author

@evillgenius75 - So with the 2.0 provider, if one declares a resource "azurerm_resource_group" snippet and the specified resource group exists, TF will throw an error?

@evillgenius75
Copy link

@evillgenius75 - So with the 2.0 provider, if one declares a resource "azurerm_resource_group" snippet and the specified resource group exists, TF will throw an error?

@jmspring that is correct, that will be the new functionality in the Azure 2.0 provider as I understand it. I would double check with Tom Harvey to be certain, but that is the indication moving forward.

@jmspring
Copy link
Contributor Author

@evillgenius75 - re: the environment variable. I still that approach is quite error prone. Specifically because it requires the user the ensure that the variable is set every time they open a console window and are interacting with resources via TF. I know, personally, I'm bound to open multiple terminal windows and don't always have ENV vars set consistently (activating a service principal settings, etc).

Additionally, re: the 2.0 behavior of importing, there will still be required education and understanding to what that means. There could be a case where an RG is imported when the desire is not to do so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants