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

Handle multiple security groups and security group rules across regions #20

Closed
ronaldtse opened this issue Feb 7, 2018 · 14 comments
Closed
Assignees

Comments

@ronaldtse
Copy link
Contributor

Here's an enhancement we need for the module.

For each security group, we want to handle rules (and ports) separately. These security groups may belong to different regions (different providers)

  • use providers as argument, don't pass aws_account_id and aws_region into module anymore
  • for each security group, we allow defining multiple rules and conditions inside

This is how the new structure should be used:

provider "aws" {
  allowed_account_ids = [ "${var.eu_aws_account_id}" ]
  region              = "eu-west-1"
  alias               = "eu"
}

provider "aws" {
  allowed_account_ids = [ "${var.us_aws_account_id}" ]
  region              = "us-east-1"
  alias               = "us"
}

module "dynamic-secgroup" {
  source = "riboseinc/authenticating-secgroup/aws"

  # prefix string used in named resources
  name_prefix             = "getting-started-"

  # Description of this secgroup
  description             = "${var.description}"

  security_group {
    id = "${aws_security_group.instance_group_1_ssh.id}"

    providers = {
      "aws" = "aws.us"
    }

    security_group_rule {
      secgroup_rule_type      = "ingress"
      secgroup_rule_from_port = 22
      secgroup_rule_to_port   = 22
      secgroup_rule_protocol  = "tcp"
    }

    security_group_rule {
      secgroup_rule_type      = "ingress"
      secgroup_rule_from_port = 443
      secgroup_rule_to_port   = 443
      secgroup_rule_protocol  = "tcp"
    }

    time_to_expire = 600

  }

  security_group {
    id = "${aws_security_group.instance_group_2_http.id}"

    providers = {
      "aws" = "aws.eu"
    }

    security_group_rule {
      secgroup_rule_type      = "ingress"
      secgroup_rule_from_port = 80
      secgroup_rule_to_port   = 80
      secgroup_rule_protocol  = "tcp"
    }

    time_to_expire = 300
  }
}

Let me know what you think!

@phuonghuynh
Copy link
Contributor

I think its OK. We can enhance the module for this. We can use type list for "security_group_rule", like:

security_group_rules =[ 
    {
      secgroup_rule_type      = "ingress"
      secgroup_rule_from_port = 22
      secgroup_rule_to_port   = 22
      secgroup_rule_protocol  = "tcp"
    }, 
    {
      secgroup_rule_type      = "ingress"
      secgroup_rule_from_port = 443
      secgroup_rule_to_port   = 443
      secgroup_rule_protocol  = "tcp"
    }
]

@ronaldtse
Copy link
Contributor Author

Is type list better, or would a command be cleaner?

@phuonghuynh
Copy link
Contributor

The "command" is better, but i dont know it. Do you mean terraform has a way to define data driven variable?

@ronaldtse
Copy link
Contributor Author

Now I think about it — it’s not possible to do it as a module. That would have to be a custom resource to accept such syntax.

Your approach is good enough indeed; let’s do it. Thanks!

@ronaldtse
Copy link
Contributor Author

@phuonghuynh any updates so far? Thanks!

@phuonghuynh
Copy link
Contributor

I still working on it, the syntax might be different. Terraform not support dynamic create module using "count" as its resources. Also, i dont know how to get provider attribute to generate arn for lambda permission, so i keep "aws_account_id" & "aws_region" for now.

@ronaldtse
Copy link
Contributor Author

Got it.

Provider attribute account arn can be retrieved via https://www.terraform.io/docs/providers/aws/d/caller_identity.html which already includes account ID and region code.

@ronaldtse
Copy link
Contributor Author

Hi @phuonghuynh , any updates on the syntax? We are kind of stuck here without a method to specify multiple ports. Thanks!

@phuonghuynh
Copy link
Contributor

@ronaldtse the terraform v11 not support syntax to allow configure provider by variable, hashicorp/terraform#11578

This is the syntax that implementing,

// used to deploy
provider "aws" {
  access_key = "${var.aws_access_key}"
  secret_key = "${var.aws_secret_key}"
  region     = "${var.aws_region}"
}

provider "aws" {
  allowed_account_ids = [
    "${var.eu_aws_account_id}"
  ]

  region              = "eu-west-1"
  alias               = "eu"
}

provider "aws" {
  allowed_account_ids = [
    "${var.us_aws_account_id}"
  ]

  region              = "us-east-1"
  alias               = "us"
}

module "dynamic-secgroup" {
  source          = "../.."
  name_prefix     = "getting-started-"

  # Description of this secgroup
  description     = "${var.description}"


  security_groups = [
    {
      group_ids      = [
        "sg-df7a88a3",
        "sg-c9c72eb5"
      ]

      provider_alias = "aws.eu"

      rules          = [
        {
          secgroup_rule_type      = "ingress"
          secgroup_rule_from_port = 22
          secgroup_rule_to_port   = 22
          secgroup_rule_protocol  = "tcp"
        },
        {
          secgroup_rule_type      = "ingress"
          secgroup_rule_from_port = 443
          secgroup_rule_to_port   = 443
          secgroup_rule_protocol  = "tcp"
        }
      ]
    },
  ]

  //  time_to_expire = 600
}

Sample resource using provider alias

resource "aws_api_gateway_rest_api" "this" {
  //not supported in Terraform v11 now, https://github.com/hashicorp/terraform/issues/11578
  provider = "aws.${lookup(element(var.security_groups, count.index), "provider_alias") }"
  count = "${local.count}"
  name        = "${var.name_prefix}terraform-aws-authenticating-secgroup-${count.index}"
  description = "${var.description}"
}

Or we might try with this, hashicorp/terraform#11578 (comment) . However, i not sure how could we use "assume_role" here.

@phuonghuynh
Copy link
Contributor

phuonghuynh commented Feb 27, 2018

We are kind of stuck here without a method to specify multiple ports

@ronaldtse Its easier to do support multiple ports with the old config (not support multiple account_id)

@phuonghuynh phuonghuynh self-assigned this Feb 27, 2018
@ronaldtse
Copy link
Contributor Author

@phuonghuynh but this kind of provider support is good enough, right?

module "dynamic-secgroup" {
  source          = "../.."
  name_prefix     = "getting-started-"

  # Description of this secgroup
  description     = "${var.description}"

  security_groups = [
    {
      group_ids      = [
        "sg-df7a88a3",
        "sg-c9c72eb5"
      ]

# SEE THIS >>>
      providers {
        "aws" = "aws.eu"
      }
# <<<

      rules          = [
        {
          secgroup_rule_type      = "ingress"
          secgroup_rule_from_port = 22
          secgroup_rule_to_port   = 22
          secgroup_rule_protocol  = "tcp"
        },
        {
          secgroup_rule_type      = "ingress"
          secgroup_rule_from_port = 443
          secgroup_rule_to_port   = 443
          secgroup_rule_protocol  = "tcp"
        }
      ]
    },
  ]
  //  time_to_expire = 600
}

@ronaldtse
Copy link
Contributor Author

@phuonghuynh we don't need to use multiple account-ids, but if possible we want to use one dynamic secgroup module to handle multiple regions. But if it can't, we can separate per-region, too.

@phuonghuynh
Copy link
Contributor

Rest API Region is an enhancement now hashicorp/terraform-provider-aws#2167

So i will make it work with multi-ports first.

@erikbor
Copy link
Collaborator

erikbor commented Apr 24, 2018

Done and closed.

@erikbor erikbor closed this as completed Apr 24, 2018
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