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

Idempotency broken when creating volterra_http_loadbalancer with simple_route and origin_pool due to : kind = (known after apply) #277

Open
slavekjurkowski2 opened this issue Sep 9, 2024 · 4 comments

Comments

@slavekjurkowski2
Copy link

I believe idempotency is broken in the case where volterra_http_loadbalancer with simple_route and origin_pool is provisioned with terraform due to : kind = (known after apply)

Terraform v1.9.5
on linux_arm64

  • provider registry.terraform.io/volterraedge/volterra v0.11.35

########Module Code:

locals {
use_service_policies_from_namespace = length(var.active_service_policies) == 0 ? true : false
}
resource "volterra_http_loadbalancer" "lb-https-tf" {
depends_on = [var.depends_on_origin_pool]

name = "${var.name_prefix}-lb-https-${var.name_suffix}"
namespace = var.namespace
domains = var.https_config["https"].domains

default_sensitive_data_policy = var.default_sensitive_data_policy
disable_api_definition = var.disable_api_definition
disable_api_discovery = var.disable_api_discovery
disable_malicious_user_detection = var.disable_malicious_user_detection
disable_threat_mesh = var.disable_threat_mesh
disable_trust_client_ip_headers = var.disable_trust_client_ip_headers
l7_ddos_action_default = var.l7_ddos_action_default
advertise_on_public_default_vip = var.advertise_on_public_default_vip

service_policies_from_namespace = local.use_service_policies_from_namespace

active_service_policies {
dynamic "policies" {
for_each = var.active_service_policies
content {
name = policies.value.name
namespace = policies.value.namespace
tenant = policies.value.tenant
}
}
}

no_challenge = var.no_challenge
disable_rate_limit = var.disable_rate_limit
user_id_client_ip = var.user_id_client_ip
source_ip_stickiness = var.source_ip_stickiness
add_location = var.add_location

https {
port = var.https_config["https"].port
add_hsts = var.https_config["https"].add_hsts
http_redirect = var.https_config["https"].http_redirect
enable_path_normalize = var.https_config["https"].enable_path_normalize
connection_idle_timeout = var.https_config["https"].connection_idle_timeout
default_header = var.https_config["https"].default_header
default_loadbalancer = var.https_config["https"].default_loadbalancer
disable_path_normalize = var.https_config["https"].disable_path_normalize

non_default_loadbalancer = var.https_config["https"].non_default_loadbalancer
pass_through             = var.https_config["https"].pass_through
tls_cert_params {
  no_mtls = var.https_config["https"].tls_cert_params.no_mtls
  tls_config {
    default_security = var.https_config["https"].tls_cert_params.tls_config.default_security
  }
  dynamic "certificates" {
    for_each = var.https_config["https"].tls_cert_params.certificates
    content {
      namespace = certificates.value.namespace
      name      = certificates.value.name
      tenant    = certificates.value.tenant
    }
  }
}

}

dynamic "default_route_pools" {
for_each = var.default_route_pools
content {
pool {
name = default_route_pools.value.name
namespace = default_route_pools.value.namespace
tenant = default_route_pools.value.tenant
}
weight = default_route_pools.value.weight
}
}

dynamic "routes" {
for_each = var.simple_routes
content {
simple_route {
auto_host_rewrite = routes.value.auto_host_rewrite
disable_host_rewrite = routes.value.disable_host_rewrite
http_method = routes.value.http_method

    advanced_options {
      common_buffering          = routes.value.advanced_options.common_buffering
      common_hash_policy        = routes.value.advanced_options.common_hash_policy
      default_retry_policy      = routes.value.advanced_options.default_retry_policy
      disable_location_add      = routes.value.advanced_options.disable_location_add
      disable_mirroring         = routes.value.advanced_options.disable_mirroring
      disable_prefix_rewrite    = routes.value.advanced_options.disable_prefix_rewrite
      disable_spdy              = routes.value.advanced_options.disable_spdy
      disable_waf               = routes.value.advanced_options.disable_waf
      disable_web_socket_config = routes.value.advanced_options.disable_web_socket_config
      do_not_retract_cluster    = routes.value.advanced_options.do_not_retract_cluster
      enable_spdy               = routes.value.advanced_options.enable_spdy

      inherited_bot_defense_javascript_injection = routes.value.advanced_options.inherited_bot_defense_javascript_injection
      inherited_waf                              = routes.value.advanced_options.inherited_waf
      no_retry_policy                            = routes.value.advanced_options.no_retry_policy
      priority                                   = routes.value.advanced_options.priority
      timeout                                    = routes.value.advanced_options.timeout

      response_headers_to_add {
        append = routes.value.advanced_options.response_headers_to_add.append
        name   = routes.value.advanced_options.response_headers_to_add.name
        value  = routes.value.advanced_options.response_headers_to_add.value
      }
    }


    origin_pools {
      priority = routes.value.origin_pools.priority
      weight   = routes.value.origin_pools.weight

      pool {
        name      = routes.value.origin_pools.pool.name
        namespace = var.namespace
        tenant    = var.tenant
      }
    }

    path {
      prefix = routes.value.path.prefix
    }

    incoming_port {
      no_port_match = routes.value.incoming_port.no_port_match
      port          = routes.value.incoming_port.port
    }

  }

}

}

app_firewall {
name = var.app_firewall.name
namespace = var.app_firewall.namespace
tenant = var.app_firewall.tenant
}

dynamic "more_option" {
for_each = var.more_option
content {
response_headers_to_add {
append = more_option.value.append
name = more_option.value.name
value = more_option.value.value
}
}
}
}

#############tfvars:

simple_routes = [
{
auto_host_rewrite = true
disable_host_rewrite = false
http_method = "ANY"
advanced_options = {
common_buffering = true
common_hash_policy = true
default_retry_policy = true
disable_location_add = false
disable_mirroring = true
disable_prefix_rewrite = true
disable_spdy = true
disable_waf = false
disable_web_socket_config = true
do_not_retract_cluster = false
enable_spdy = false
inherited_bot_defense_javascript_injection = true
inherited_waf = true
no_retry_policy = false
priority = "DEFAULT"
request_headers_to_remove = []
response_headers_to_remove = []
retract_cluster = true
timeout = 30000
response_headers_to_add = {
append = true
name = "response_via_simple_route"
value = "true"
}
}
incoming_port = {
no_port_match = true
port = 0
}
origin_pools = {
priority = 1
weight = 1
pool = {
name = "vslx-prod-maint-maint-test-tf"
}
}
path = {
prefix = "/"
}
}
]

#######module caller

module "loadbalancer" {
source = "../../modules/loadbalancer_simple"
namespace = var.namespace
environment = var.environment
tenant = var.tenant

#pass origin pool object to load balancer module
depends_on_origin_pool = [
volterra_origin_pool.op-maint-test,
volterra_origin_pool.op-maint-test2 ]

name_prefix = local.name_prefix
name_suffix = local.name_suffix

https_config = {
https = {
domains = var.domains_lb01
port = 443
add_hsts = true
http_redirect = true
enable_path_normalize = true
connection_idle_timeout = 0
default_header = false
default_loadbalancer = false
disable_path_normalize = false
non_default_loadbalancer = false
pass_through = false
tls_cert_params = {
no_mtls = true
tls_config = {
default_security = true
}
certificates = [
{
namespace = var.namespace
name = volterra_certificate.cert-maint-test.name
tenant = var.tenant
},
{
namespace = var.namespace
name = volterra_certificate.cert-maint-test2.name
tenant = var.tenant
}
]
}
}
}
default_route_pools = [
{
name = volterra_origin_pool.op-maint-test.name
namespace = var.namespace
tenant = var.tenant
weight = 1
}
]

custom_routes = [
# {
# name = volterra_route.route-maint-test2-com.name
# namespace = var.namespace
# tenant = var.tenant
# },
# {
# name = volterra_route.route-maint-test-com.name
# namespace = var.namespace
# tenant = var.tenant
# }
]

simple_routes = [
{
auto_host_rewrite = true
disable_host_rewrite = false
http_method = "ANY"
# (1 unchanged attribute hidden)

  advanced_options = {
    common_buffering          = true
    common_hash_policy        = true
    default_retry_policy      = true
    disable_location_add      = false
    disable_mirroring         = true
    disable_prefix_rewrite    = true
    disable_spdy              = true
    disable_waf               = false
    disable_web_socket_config = true
    do_not_retract_cluster    = false
    enable_spdy               = false

    inherited_bot_defense_javascript_injection = true
    inherited_waf                              = true
    no_retry_policy                            = false
    priority                                   = "DEFAULT"
    request_headers_to_remove                  = []
    response_headers_to_remove                 = []
    retract_cluster                            = true
    timeout                                    = 30000


    response_headers_to_add = {
      append = true
      name   = "response_via_simple_route"
      value  = "true"
    }
  }

  incoming_port = {
    no_port_match = true
    port          = 0
  }

  origin_pools = {

    priority = 1
    weight   = 1

    pool = {
      name      = "vslx-prod-maint-maint-test-tf"
      namespace = var.namespace
      tenant    = var.tenant
    }
  }

  path = {
    prefix = "/"
  }
}

]

active_service_policies = [
{
name = "vslx-${var.environment}-f5xc-srvpoly-explicit-allow-tf"
namespace = var.namespace
tenant = var.tenant
},
{
name = "vslx-${var.environment}-f5xc-srvpoly-default-allow-all-tf"
namespace = var.namespace
tenant = var.tenant
}
]

app_firewall = {
name = volterra_app_firewall.firewall-maint-waap.name
namespace = var.namespace
tenant = var.tenant
}

more_option = [
{
append = true
name = "response_from_lb"
value = "true"
}
]

tags = {
Terraform = "true"
Environment = "${var.environment}"
}
}

@jcspencer
Copy link

jcspencer commented Oct 12, 2024

Second this @slavekjurkowski2!

We see Terraform attempting to unset the kind fields in a few places; across both volterra_http_loadbalancer and volterra_origin_pool on each plan.

In all cases, we see + kind = (known after apply), which makes it practically impossible to actually use our Terraform plan output as a reference when inspecting a change.

We have (for now) resorted to extracting changes from a terraform show -json <FILE> output so we can see what's actually changing - very annoying!


Either way, looking into the diff, we're seeing that our state file records "kind": "", rather than what the API returned upon resource creation, which means that Terraform thinks it needs to unset the value on each plan/apply cycle to get it back to the "computed" value.


Diving into the state file, and comparing against the Volterra API, we see the following (though I'm sure there'd be a few more in there too):

Resource Type Path State File Value Actual Value (API) Proposed TF Action
volterra_http_loadbalancer routes[0].simple_route[0]
.origin_pools[0].pool[0]
kind: "" kind: "origin_pool" delete (e.g. unset "kind")
volterra_http_loadbalancer https[0].tls_cert_params[0]
.certificates[0]
kind: "" kind: "certificate" delete (e.g. unset "kind")
volterra_origin_pool origin_servers[0].private_name[0]
.site_locator[0].virtual_site[0]
kind: "" kind: "virtual_site" delete (e.g. unset "kind")

It's very odd, because the fields in all three cases are set as Computed: true in the resource definitions in this repository. What's even weirder, is that while the API returns the correct values (e.g. "origin_pool"), Terraform seems to store in it's state file a value that indicates the kind field is empty (e.g. "").


What it seems is happening is that in the schema.Resource "Read" functions for the following types:

Are not updating the kind field in the state file to match the computed value.

Because the state file has kind="", and both DriftDetectionSpec and DriftDetectionSpec_OriginPool flatten object references back to (name, namespace, tenant), the drift detection routines perpetually think that Terraform needs to unset kind:

func FlattenObjectRefTypeSet(x *ves_io_schema_views.ObjectRefType) []interface{} {
	res := make([]interface{}, 0)
	if x != nil {
		val := map[string]interface{}{
			"name":      x.GetName(),
			"namespace": x.GetNamespace(),
			"tenant":    x.GetTenant(),
		}
		res = append(res, val)
	}
	return res
}

While all the resource schema definitions include the kind field in object references, the ObjectRefType type doesn't include it.


TL;DR: since drift detection came in, it seems that the provider has never set the kind fields in the state file. Looking around, it doesn't really seem like any computed fields are really persisted back to the state file, except for a few distinct areas.

The provider should either be updated to set the kind fields to the API-returned values upon read and use them in object reference diff detection; or just migrate the state file to remove empty "kind" fields entirely.

(Note: in our case, this is in the process of preparing a migration from 0.11.32 to 0.11.36 to enable drift detection; not sure if this matters?)

@jcspencer
Copy link

The real problem here, to simplify things:

In the case of origin pools, for example

When drift detection runs, the ObjectRefType version is laid back on top of the resource schema, which means that kind is never set to a value, and the loop continues...

@jcspencer
Copy link

Not that I have any way to build this module myself, but adding Kind to the "views" version of ObjectRefType like is set here is probably the fix for this?

@SanjeetKr7
Copy link
Collaborator

@jcspencer Thank you for your detailed analysis. We will address this in the upcoming release.
@pranavskumar can you please look into this issue?

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

No branches or pull requests

3 participants