quintilesims / layer0

Build, Manage, and Deploy Your Applications
Apache License 2.0
44 stars 20 forks source link

Set the LoadBalancer's port's CertificateARN #534

Closed sesh-kebab closed 6 years ago

sesh-kebab commented 6 years ago

What does this pull request do? It sets a CertificateARN property on a port as well as the CertificateName. The TF Provider will use the ARN to populate its certificate property in the schema.

How should this be tested? Use the sample terraform config below

resource "layer0_environment" "env_a" {
  name = "a"
}

resource "layer0_load_balancer" "haproxy" {
  name        = "test-lb"
  environment = "${layer0_environment.env_a.id}"
  private     = false

  port {
    host_port      = 443
    container_port = 80
    protocol       = "https"
    certificate    = "<redacted>"
  }
}

Apply the changes using terraform apply

Confirm that a consecutive apply won't detect any changes and try to recreate any resources.

Checklist

closes #531

diemonster commented 6 years ago

@sesh-kebab is the intention here to release another layer0 hotfix? Does this also need to be applied to the api-refactor branch?

sesh-kebab commented 6 years ago

@diemonster yes, I think this should be released as another hotfix. Also yes, it does need to be ported to the api-refactor branch also.

jparsons04 commented 6 years ago

I addressed the api-refactor port in my PR https://github.com/quintilesims/layer0/pull/526. It appears the code in API refactor already does this when it needs to read from the load balancer. See here: https://github.com/quintilesims/layer0/blob/api-refactor/api/provider/aws/load_balancer_read.go#L32

sesh-kebab commented 6 years ago

@jparsons04 this isn't an issue in your PR. The port model only has Certificate in api-refactor. In Develop right now, we have both CertificateName & CertificateARN and the read in the provider was only setting the CertificateName which is what gave rise to the issue.