F5Networks / terraform-aws-bigip-module

Terraform module for Deploying BIG-IP in AWS
Apache License 2.0
10 stars 21 forks source link

external_source_dest_check logic and default is flipped #28

Closed JeffGiroux closed 2 years ago

JeffGiroux commented 2 years ago

The BIG-IP should be deployed with external interfaces set to DISABLE source destination checks. However the current behavior based on the variables value is incorrect.

The default value on the aws_network_interface resource is to set 'source_dest_check' to true. However, to enable inline routing you need to set to false to DISABLE checks.

Current behavior

BIG-IP deployed with external NICs set to check (true) src-dst.

Expected behavior

BIG-IP should have external NICs set to false 'source_dest_check'

Code example for reference...this applies to other NICs too

resource "aws_network_interface" "public" {
  count                   = length(compact(local.external_public_private_ip_primary)) > 0 ? length(local.external_public_subnet_id) : 0
  subnet_id               = local.external_public_subnet_id[count.index]
  security_groups         = [local.external_public_security_id[count.index]]
  private_ip_list         = [local.external_public_private_ip_primary[count.index], local.external_public_private_ip_secondary[count.index]]
  private_ip_list_enabled = true
  source_dest_check       = var.external_source_dest_check
  tags = merge(local.tags, {
    Name = format("%s-%d", "BIGIP-External-Public-Interface", count.index)
    }
  )
}

Then variables.tf has this default for the variable...

variable "external_source_dest_check" {
  description = "Disable source/dest check on External interface to allow inline routing for backends"
  default     = true
}

Fix

The fix should be to update the variables.tf description to properly reflect the default based on AWS resource. Sure, you are disabling it by setting to true. However, the input of true passed to aws_network_interface in fact ENABLES src/dst check. So the output and value should be different and default value should be false for external network interfaces.

variable "external_source_dest_check" {
  description = "Enable source destination checking for the External interfaces. To allow inline routing for backends, this must be set to false."
  default     = false
}
RavinderReddyF5 commented 2 years ago

@JeffGiroux fixed in v1.1.7, please verify