PaloAltoNetworks / terraform-aws-swfw-modules

Terraform Reusable Modules for Software Firewalls on AWS
https://registry.terraform.io/modules/PaloAltoNetworks/swfw-modules/aws
MIT License
14 stars 11 forks source link

[Bug Report] asg submodule only applies a single security group per eni #85

Closed sgreathouse-rgare closed 1 month ago

sgreathouse-rgare commented 1 month ago

Describe the bug

https://registry.terraform.io/modules/PaloAltoNetworks/swfw-modules/aws/latest/submodules/asg

I'm passing a list of security groups for each of the 3 interfaces for a NGFW. The Lambda function only attaches the first SG to the interfaces.

I tried passing the subnet_ids & security_group_ids arguments to the lambda in the main module call in addition to passing them in the interfaces block. That resulted in the instance only having one interface. subnet_ids & security_group_ids is not really documented, so some understanding of how they function would be helpful as well.

I also noticed that the interfaces_config environment variable passed to Lambda only receives information for the mgmnt index1 & public index 2 interfaces. Maybe a clue, maybe as-designed.

Thanks for your help.

full module call.

module "ngfw_asg" {
    source  = "../ngfw_asg"
    # global_tags = var.default_tags
    global_tags = {} # fix
    name_prefix = "${var.resource_name}-"
    region = var.aws_region
    ssh_key_name = var.ssh_key_name
    # asg_name = ""
    # delete_timeout = ""
    # delicense_enabled = true
    # delicense_ssm_param_name = ""
    # desired_capacity = ""
    # ebs_kms_id = ""
    fw_license_type = "byol" # default can omit
    # health_check =  { "grace_period": 300, "type": "EC2" } 
    include_deprecated_ami = false # default can omit
    # instance_refresh = see docs
    instance_type = "m5.xlarge"
    # ip_target_groups = see docs, for lambda
    # lambda_execute_pip_install_once = see docs
    launch_template_update_default_version = true
    launch_template_version = "$Latest"
    lifecycle_hook_timeout = 300 # default can omit
    max_size = 1
    min_size = 1
    desired_capacity = 1
    # reserved_concurrent_executions = see docs lambda
    # scaling_cloudwatch_namespace = see docs
    scaling_estimated_instance_warmup = 900 # default can omit
    scaling_metric_name = false # default can omit
    scaling_plan_enabled = false # default can omit
    scaling_statistic = "Average" # default can omit   
    scaling_tags = {}
    scaling_target_value = 70 # default can omit
    # security_group_ids = [aws_security_group.ngfw_appliance.id, aws_security_group.ngfw_egress.id] # see docs lambda
    # subnet_ids = toset(concat(local.egress_subnet_list, local.appliance_subnet_list, local.gwlb_subnet_list)) # see docs lambda
    suspended_processes = [] # see docs
    tag_specifications_targets = [ "instance", "volume", "network-interface" ] # default can omit
    target_group_arn = aws_lb_target_group.appliance.arn
    vmseries_ami_id = null # default can omit
    vmseries_iam_instance_profile = aws_iam_instance_profile.this.name
    vmseries_product_code = "6njl1pau431dv1qxipg63mvah" # default BYOL can omit
    vmseries_version = "10.2.11-h1"
    bootstrap_options = "mgmt-interface-swap=enable\ndns-primary = 169.254.169.253\ndns-secondary = 8.8.8.8\nvmseries-bootstrap-aws-s3bucket = insp-vpc-pan-ngfw-bootstrap-962207273628/\n"
    interfaces = {
        mgmt = {
            device_index       = 1
            name               = "mgmt"
            subnet_id          = local.egress_subnet_map
            create_public_ip   = true
            source_dest_check  = false
            security_group_ids = [aws_security_group.ngfw_appliance.id, aws_security_group.ngfw_egress.id]
        },
        public = {
            device_index     = 2
            name             = "public"
            subnet_id    = local.appliance_subnet_map
            create_public_ip = false
            source_dest_check  = false
            security_group_ids = [aws_security_group.ngfw_appliance.id, aws_security_group.ngfw_egress.id]
        },
        private = {
            device_index = 0
            name         = "private"
            source_dest_check  = false
            subnet_id    = local.gwlb_subnet_map
            create_public_ip   = false
            security_group_ids = [aws_security_group.ngfw_appliance.id, aws_security_group.ngfw_egress.id]
        }
    }
}

Module Version

3.0.0-rc.1

Terraform version

Terraform v1.9.7 on linux_arm64 + provider registry.terraform.io/hashicorp/archive v2.6.0 + provider registry.terraform.io/hashicorp/aws v5.70.0 + provider registry.terraform.io/hashicorp/local v2.5.2 + provider registry.terraform.io/hashicorp/null v3.2.1

Expected behavior

multiple security groups per interface

Current behavior

one security groups per interface

Anything else to add?

No response

sgreathouse-rgare commented 1 month ago

Any progress / ETA?

sgreathouse-rgare commented 1 month ago

BTW, it appears that the 2.0.15 release has the same issue

acelebanski commented 1 month ago

Hello @sgreathouse-rgare, thanks for raising the issue. There was no link between your issue and 2.0.15 release. I analysed it this week and I'm working on it. Once tested, I will create a PR, possibly in a couple of days.

sgreathouse-rgare commented 1 month ago

@acelebanski I just re-tested & 2.0.15 will not attach multiple security groups to an ENI either.

acelebanski commented 1 month ago

Hello @sgreathouse-rgare, I meant that there was no bugfix for your issue in 2.0.15 release. I raised a PR #88 addressing your issue now. I will try to get that approved and released early next week. However, feel free to take the module code from the PR to use it locally and you can test it yourself.