Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

application gateway resource not honoring lifecycle ignore_changes for ssl_certificates during apply #6330

Closed
darrenk13 opened this issue Apr 1, 2020 · 18 comments · Fixed by #8761

Comments

@darrenk13
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform v0.12.18

  • provider.azurerm v2.3.0
  • provider.helm v0.10.4
  • provider.kubernetes v1.11.1
  • provider.null v2.1.2
  • provider.random v2.2.1

Tested with the following additional versions of azurerm:
v2.2.0 -> affected
v2.1.0 -> not affected

Affected Resource(s)

  • azurerm_application_gateway

Terraform Configuration Files

provider "azurerm" {
  version = "~> 2.0"
  features {}
}

data "azurerm_resource_group" "resource_group" {
  name = var.resource_group_name
}

resource "azurerm_public_ip" "application_gateway_public_ip" {
  count               = var.enabled ? 1 : 0
  name                = "${var.application_gateway_name}-ip"
  resource_group_name = data.azurerm_resource_group.resource_group.name
  location            = data.azurerm_resource_group.resource_group.location
  allocation_method   = "Static"
  sku                 = "Standard"
  tags                = var.tags
}

locals {
  backend_address_pool_name      = "${var.application_gateway_name}-beap"
  frontend_port_name             = "${var.application_gateway_name}-feport"
  frontend_ip_configuration_name = "${var.application_gateway_name}-feip"
  http_setting_name              = "${var.application_gateway_name}-be-htst"
  listener_name                  = "${var.application_gateway_name}-http"
  request_routing_rule_name      = "${var.application_gateway_name}-rqrt"
  probe_name                     = "${var.application_gateway_name}-http-probe"
}

resource "azurerm_application_gateway" "application_gateway" {
  count               = var.enabled ? 1 : 0
  name                = var.application_gateway_name
  resource_group_name = data.azurerm_resource_group.resource_group.name
  location            = data.azurerm_resource_group.resource_group.location
  tags                = merge({ "managed-by-k8s-ingress" = "true" }, var.tags)

  sku {
    name = "WAF_v2"
    tier = "WAF_v2"
  }
  autoscale_configuration {
    min_capacity = 2
    max_capacity = 10
  }
  gateway_ip_configuration {
    name      = "appGatewayIpConfig"
    subnet_id = var.frontend_subnet_id
  }
  frontend_port {
    name = local.frontend_port_name
    port = 80
  }
  frontend_ip_configuration {
    name                 = local.frontend_ip_configuration_name
    public_ip_address_id = azurerm_public_ip.application_gateway_public_ip[count.index].id
  }
  backend_address_pool {
    name = local.backend_address_pool_name
  }
  probe {
    name                                      = local.probe_name
    interval                                  = 30
    path                                      = "/"
    pick_host_name_from_backend_http_settings = true
    protocol                                  = "http"
    timeout                                   = 30
    unhealthy_threshold                       = 2
  }
  backend_http_settings {
    name                                = local.http_setting_name
    cookie_based_affinity               = "Disabled"
    pick_host_name_from_backend_address = true
    port                                = 80
    protocol                            = "Http"
    request_timeout                     = 1
    probe_name                          = local.probe_name
  }
  http_listener {
    name                           = local.listener_name
    frontend_ip_configuration_name = local.frontend_ip_configuration_name
    frontend_port_name             = local.frontend_port_name
    protocol                       = "Http"
  }
  request_routing_rule {
    name                       = local.request_routing_rule_name
    rule_type                  = "Basic"
    http_listener_name         = local.listener_name
    backend_address_pool_name  = local.backend_address_pool_name
    backend_http_settings_name = local.http_setting_name
  }
  waf_configuration {
    enabled                  = true
    firewall_mode            = "Prevention"
    rule_set_type            = "OWASP"
    rule_set_version         = "3.1"
  }
  ssl_policy {
    policy_type = "Predefined"
    policy_name = "AppGwSslPolicy20170401S"
  }

  lifecycle {
    ignore_changes = [
      backend_address_pool,
      backend_http_settings,
      frontend_port,
      http_listener,
      probe,
      redirect_configuration,
      request_routing_rule,
      ssl_certificate,
      tags,
      url_path_map,
    ]
  }
}

Expected Behavior

Terraform should have applied the changes to the application gateway and ignore the ssl certificate configuration.

Actual Behavior

Terraform did not change the settings to the application gateway and outputted the following error:

Error: Error expanding `ssl_certificate`: either `key_vault_secret_id` or `data` must be specified for the `ssl_certificate` block "<cert_block_name>"

Steps to Reproduce

  1. Setup application gateway using Terraform
  2. Add SSL certificate/listener outside of Terraform (like through AGIC)
  3. Perform terraform plan. No pending changes (expected, good)
  4. Modify config for application gateway setting autoscale_configuration max_capacity to 11
  5. Perform terraform plan. Only pending change is autoscale_configuration (expected, good)
  6. Perform terraform apply. Receive an error like above (not expected)

Important Factoids

We are using an application gateway to route traffic to an AKS cluster. As part of this we use the Application Gateway Ingress Controller in the kubernetes cluster. The AGIC will update the Application Gateway independent of Terraform so we have many fields set to ignore changes in the lifecycle block of the application gateway resource. T

References

@georgespatton
Copy link

georgespatton commented Apr 1, 2020

Another way to explain this perhaps is that there are required arguments as of azurerm module 2.2.0+ which are restricting us. In 2.2.0 and 2.3.0 empty passwords for ssl certificates were not accepted in addition to lifecycle ignore_changes not being respected. Seeing this error which seems to be correlated with the issue reported here.

Error: Error expanding ssl_certificate: 'password' is required if data is specified for the ssl_certificate block "mycert"

myagw.tf line 20, in resource "azurerm_application_gateway" "myagw":
20: resource "azurerm_application_gateway" "myagw" {

...

ssl_certificate {
name = "mycert"
data = filebase64("mycert.pfx")
password = ""
}

Perhaps related to this fix in 2.2.0: #3935

@sarmadsaleem
Copy link

@darrenk13 were you able to find a work around the error somehow?

@darrenk13
Copy link
Author

No, I have not found a work around for this issue. There looks to be a couple of other issues around ssl certs and application gateways:

I wonder this and those two issues are related.

@alecor191
Copy link

FWIW I can still repro this issue with 2.10.0. The two referenced issues (#6368, #6295) should be addressed in that version. However, this one still persists.

Any chance to get an ETA for addressing this issue? It's currently blocking us from upgrading our environment using TF; any info about potential release dates would help us with planning.

@alecor191
Copy link

It would be great if an update could be shared on this topic (is anyone working on it, do we have an ETA,...). Unfortunately this is really blocking us from updating our Azure infra using TF.

@finkj
Copy link

finkj commented Jun 29, 2020

@alecor191 @darrenk13 did you find any workaround for that issue? or is there another solution?

@alecor191
Copy link

@finkj unfortunately not. We're still stuck.

@finkj
Copy link

finkj commented Jun 30, 2020

@alecor191 i destroyed my existing infra and created a new one. solved the problem.

@gitflo1
Copy link

gitflo1 commented Aug 20, 2020

@darrenk13 unfortunately we run into the same problem with a really similar setup. AKS, Azure App Gateway and AGIC within the Kubernetes Cluster... We are using following versions:

  • terraformVersion = "0.12.29"
    
  • azureRmProviderVersion = "= 2.20.0"
    
  • azureAdProviderVersion = "= 0.11.0"
    
  • kubernetesProviderVersion = "= 1.12.0"
    

Have you found a valid workaround or are there any updates?

@ztbrown
Copy link

ztbrown commented Aug 25, 2020

@darrenk13 @alecor191 @gitflo1 I have a workaround for this. I altered the code in expandApplicationGatewaySslCertificates to check for a pre-existing public cert and not error out during expansion if the cert exists but the password / data don't exist. Then I compiled the provider and baked it into the docker image that we do our builds in. Here's the updated code:

func expandApplicationGatewaySslCertificates(d *schema.ResourceData) (*[]network.ApplicationGatewaySslCertificate, error) {
  vs := d.Get("ssl_certificate").([]interface{})
  results := make([]network.ApplicationGatewaySslCertificate, 0)

  for _, raw := range vs { 
    v := raw.(map[string]interface{})

    name := v["name"].(string)
    data := v["data"].(string)
    password := v["password"].(string)
    kvsid := v["key_vault_secret_id"].(string)
    cert := v["public_cert_data"].(string)

    output := network.ApplicationGatewaySslCertificate{
      Name: utils.String(name),
      ApplicationGatewaySslCertificatePropertiesFormat: &network.ApplicationGatewaySslCertificatePropertiesFormat{},
    }    

    // nolint gocritic
    if data != "" && kvsid != "" { 
      return nil, fmt.Errorf("only one of `key_vault_secret_id` or `data` must be specified for the `ssl_certificate` block %q", name)
    } else if data != "" { 
      // data must be base64 encoded
      output.ApplicationGatewaySslCertificatePropertiesFormat.Data = utils.String(utils.Base64EncodeIfNot(data))

      output.ApplicationGatewaySslCertificatePropertiesFormat.Password = utils.String(password)
    } else if kvsid != "" { 
      if password != "" { 
        return nil, fmt.Errorf("only one of `key_vault_secret_id` or `password` must be specified for the `ssl_certificate` block %q", name)
      }    

      output.ApplicationGatewaySslCertificatePropertiesFormat.KeyVaultSecretID = utils.String(kvsid)
    } else if cert != "" { 
      output.ApplicationGatewaySslCertificatePropertiesFormat.PublicCertData = utils.String(cert)
    } else {
      return nil, fmt.Errorf("either `key_vault_secret_id` or `data` must be specified for the `ssl_certificate` block %q", name)
    }    

    results = append(results, output)
  }

  return &results, nil
}

Hope this helps. I might PR into the main repo soon.

@rrey
Copy link

rrey commented Sep 10, 2020

Stuck as well with same error an usage. Any update please ?

@trotman23
Copy link

@rrey I forked and applied the patch provided by @ztbrown here and it's working great.

@Adel-E
Copy link

Adel-E commented Sep 22, 2020

Hello @ztbrown

Do you think you can do the PR ?
Do you need help for it, it will be really good to have this bug resolved properly.

Thx in advance

@kvendingoldo
Copy link
Contributor

@Adel-E @trotman23 @rrey @ztbrown @gitflo1 @darrenk13 @mbfrahry @katbyte I create MR with @ztbrown fix. Can you take a look, please? It is #8761

@pradeepbohra
Copy link

pradeepbohra commented Oct 8, 2020

Looking forward to the provided fix #8761 to be merged.

@jackofallops jackofallops linked a pull request Oct 19, 2020 that will close this issue
@jackofallops jackofallops added this to the v2.33.0 milestone Oct 19, 2020
@jackofallops
Copy link
Member

Closing based on merge of linked PR.

@ghost
Copy link

ghost commented Oct 22, 2020

This has been released in version 2.33.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.33.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Nov 21, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Nov 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.