GoogleCloudPlatform / cloud-foundation-fabric

End-to-end modular samples and landing zones toolkit for Terraform on GCP.
Apache License 2.0
1.49k stars 854 forks source link

Added unique suffix to the certname to prevent conflict when updating #2594

Closed SadriG91 closed 4 days ago

SadriG91 commented 1 week ago

I added unique suffix to the certname to prevent conflict when updating domains

Checklist

I applicable, I acknowledge that I have:

wiktorn commented 1 week ago

@SadriG91 did you try following the instructions here, that should help avoid this issue?

SadriG91 commented 1 week ago

@wiktorn the link basically says changes will fail if in use, so how would the change be done then? What if it's during a planned maintenance window for example then shouldn't the module allow for that?

SadriG91 commented 1 week ago

Hi @SadriG91,

Thanks for the contribution! We generally avoid using random suffixes/prefixes. Could you instead add an optional name field to var.ssl_certificates and use coalesce() to determine the name?

This would allow users to specify a custom name while providing a clear default.

The idea is that if you for example add a subdomain and want to add that to the cert it will get recreated and since it can't be detached while in use we need to add the flag create_before_destroy and for that we need a unique id or name for the cert otherwise name conflict because of name exist already

wiktorn commented 1 week ago

@wiktorn the link basically says changes will fail if in use, so how would the change be done then? What if it's during a planned maintenance window for example then shouldn't the module allow for that?

if your configuration is:

  ssl_certificates = {
    managed_configs = {
      default = {
        domains = ["${module.addresses.global_addresses["glb-0"].address}.nip.io"]
      }
  }

You change it to:

  ssl_certificates = {
    managed_configs = {
      default = {
        domains = ["${module.addresses.global_addresses["glb-0"].address}.nip.io"]
      }
      new = {
        domains = [
          "${module.addresses.global_addresses["glb-0"].address}.nip.io",
          "${replace(module.addresses.global_addresses["glb-0"].address, ".", "-")}.nip.io",
        ]
      }
    }
  }

(or whatever new certificate should look like), and once that is provisioned and working, you can remove the default part.

ludoo commented 4 days ago

closing this, feel free to reopen with a different feature set