v0.4.0 New Standards, Breaking Changes

This release makes no attempt at backward compatibility with earlier versions.
It puts forth some new Cloud Posse standards. See details below.

As a major overhaul, it likely has bugs. It may have breaking changes in the near future as we discover design issues. However, the intention is to get this module stabilized and provide a consistent interface moving forward.

This module requires Terraform version 0.14 or later due to numerous issues in Terraform 0.13.

🚀 Enhancements

Overhaul Module to New Standards @Nuru (#17) (click to see details)

what

• Input use_name_prefix replaced with create_before_destroy. Previously, create_before_destroy was always set to true but of course that fails if you are not using a name prefix, because the names must be unique. Now the name is automatically a prefix if create_before_destroy is true and not if it is not.
• Input security_group_enabled renamed to create_security_group. Whether the security group is created or not, it will be enabled, and setting security_group_enabled to false does not disable the entire module, even though the module is named "security-group", which makes the old name terribly confusing. The new name is more descriptive.
• Input id renamed to target_security_group_id. Again id by itself is too vague. Converted to list to conform to new standard pattern that optional inputs which are used in conditionals are passed as list elements. See Hashicorp recommendation
• Added a security_group_name input, which, if set, will set the security group name. If not set, name will be derived from null-label. Because the security group name must be unique within an account, we should provide some way for people to set/override it other than forcing them to create a customized null-label.
• As a convenience, added rule_matrix . Many of our modules allow users to simply give a list of security groups to allow access to the new resource, typically called allowed_security_groups. This variable allows for easy migration by closely paralleling the existing resource creation code. It allows any number of rules to be applied to any combined list of security groups and CIDRs. See example.
• As a convenience, added allow_all_egress. AWS by default allows full egress to newly created security groups. Terraform removes this when taking over a security group, but our modules frequently want to restore it. Historically, though, the modules have implemented this slightly differently, and few or none have allowed IPv6 egress. Adding this boolean gives us a way to enable it simply and consistently (as opposed to every module writing its own egress rule).
• Abandoned the attempt to create stable keys for rules to use in for_each. Existing generated keys were not guaranteed unique, and keys that were generated and guaranteed to be unique would not be known at plan time and thus could not be used. Instead, provide option for user to provide stable keys and, if not provided, generate keys knowing they might not be stable.

why

• This module is the foundation for how we'll handle security groups across all of our modules and we need to ensure greater consistency from the onset

naming conventions

We want to migrate to a consistent set of name across modules. However, it is also quite painful to be forced to rename, so where possible I would like to maintain existing names but mark them deprecated and feed them into the new names in main.tf locals{}. We have also already seen issues with the most recent set of name changes. Therefore I propose these names with these meanings:

associated_security_group_ids

associated_security_group_ids is a list of IDs of Security Group that are "associated" (AWS' term) with the resource being created. In other words, the new resource is placed in or becomes a member of the Security Groups identified by the ID.

Most often our modules got this information as existing_security_groups and a boolean use_existing_security_groups, and the recent change was to call this input simply security_groups. However, there is no consistency in naming in the AWS provider (redshift_cluster calls it cluster_security_groups, elasticache_replication_group calls it security_group_ids, ec2_instance calls it just security_groups but accepts legacy security group names as well as IDs) and in our modules we typically have several lists of security groups (see below), so just calling this security_groups is very confusing.

Using "associated" makes it clear the purpose, and suffixing with "ids" makes it clear the type. Since AWS in inconsistent and variously uses ARNs, IDs, and Names to identify resources, I think including the type is very helpful.

allowed_security_group_ids

allowed_security_group_ids are security groups that are allowed ingress to the resource being created. Typically rules allowing this are added to the single created security group, as it should be unnecessary for an existing security group, but where desired, these rules can be added to the first in the list of "associated" security groups

allow_all_egress

AWS by default creates security groups that allow no ingress but allow all egress. When Terraform starts managing rules for the security group, it removes this default egress rule. Modules should include an allow_all_egress boolean to restore that rule when true.

security_group_description

Our modules have evolved over time (at community request) to provide more useful descriptions of Security Groups. Unfortunately, Terraform cannot change the description of an existing security group; instead it must replace the SG with a new one with the new description. For this reason, changes to the description field, while beneficial for new users, can be too disruptive on existing infrastructure to be worth it. In order to provide users with control over the description and thus mitigate the impact of changes, all modules that create security groups should include a security_group_description input which, if set, overrides any other kind of generated or default description.

Instance Metadata Services

Although not actually part of the Security Group module, since we are covering consistent naming of inputs, we document here that we are using the following inputs and defaults to configure the AWS Instance Metadata Service. Note that our inputs do not exactly match the Terraform resource inputs because we have chosen to use boolean inputs rather than string inputs to toggle features. Our standard metadata_options block looks like this

  metadata_options {
    http_endpoint               = (var.metadata_http_endpoint_enabled) ? "enabled" : "disabled"
    http_put_response_hop_limit = var.metadata_http_put_response_hop_limit
    http_tokens                 = (var.metadata_http_tokens_required) ? "required" : "optional"
  }

and defaults are

metadata_http_endpoint_enabled = true
metadata_http_put_response_hop_limit = 2
metadata_http_tokens_required = true

We picked these defaults so that we default to best security practices with a concession (hop limit 2 instead of 1) to running containerized services. However, metadata_http_tokens_required = true may break some existing applications and is a breaking change, so when implemented, it should be noted in the release notes, along with how to preserve the previous settings.

Optional Inputs

This module is among the first to implement the new Cloud Posse standard for optional inputs in Terraform. Because of issues like this (just one of many, many examples) we are going to follow Hashicorp's advice and prohibit the conditional creation of resources based on values of inputs. If you want to condition the creation of a resource (e.g. count = xxx ? 1 : 0) based on whether the input is supplied or not, the way we are going to do it is to make the optional input a list. A supplied value will be in a list with 1 element. An omitted value will use the default list of 0 elements. It will remain standard practice to depend on the value of enabled, but otherwise we should avoid conditional creation of resources based on input values.

Unfortunately, this also means we cannot use for_each when the values might be generated during apply. This appears to be a consequence of the fact that for_each requires a set and the cardinality of the set depends on the values generated (adding 2 of the same value to a set only increases the size of the set by 1). So we can only use for_each when we can guarantee the user is hard coding the values so they are all known an plan time. Otherwise use count.

references

Issues with Terraform management of Security Group Rules: drift detection, cyclical dependencies, and competing for control: a post from a Hashicorp engineer

Bug in Terraform AWS provider requires multiple apply cycles to update aws_elasticache_replication_group security groups:

• hashicorp/terraform-provider-aws#20139
• cloudposse/terraform-aws-elasticache-redis#121

Some problems with the previous version:

• cloudposse/terraform-aws-elasticache-redis#122
• cloudposse/terraform-aws-elasticache-redis#123
• cloudposse/terraform-aws-elasticache-redis#119 (comment)...

v0.3.3

🤖 Automatic Updates

Update context.tf @cloudpossebot (#21)

what

This is an auto-generated PR that updates the context.tf file to the latest version from cloudposse/terraform-null-label

why

To support all the features of the context interface.

v0.3.2

🚀 Enhancements

add missing required input (vpc_id) in the example @Zaargh (#20)

what

• add missing required input vpc_id in the example

v0.3.1 Major breaking changes coming

Existing users of terraform-aws-security-group can continue to use it, but the current version has known issues and major breaking changes are coming soon. We recommend waiting for those changes to be released and marked ready-for-production before adopting new use of this module.

🐛 Bug Fixes

Restore Terraform 0.13 support, fix #12 @Nuru (#15)

what

• Restore Terraform 0.13 support
• Fix #12
• Add additional tests

why

• Lots of people still using Terraform 0.13
• Useful, requested functionality
• Ensure against reversion of #12 or #13, protect against future issues with null values

v0.3.0

refactor: rework value mapping for for_each and update dependencies @SweetOps (#11)

what

• rework value mapping for for_each
• bump minimal required version of TF to 0.14.0
• bump terratest to v0.32.8

why

• terraform has weird limitation which blocks using of dynamic values inside the for_each loop

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

v0.2.0

self conflicts with cidr_blocks error fix @maximmi (#14)

what

• default value for self field of security group rule changed to null

why

• With the provider hashicorp/aws v3.38.0 new restrictions have been added to aws_security_group_rule resource

references

• closes #13
• hashicorp/terraform-provider-aws#15606
• hashicorp/terraform-provider-aws#18467

v0.1.4

🚀 Enhancements

Add some missing TF 0.14 updates @Nuru (#10)

what

• Some updates included in the general TF 0.14 updates were missed. This brings the module up-to-date.

why

• Closes #9

v0.1.3

🚀 Enhancements

create md5 from sec group rule description in order to allow multiple… @1david5 (#6)

… rules with the same port and different descriptions.

what

• This PR generates the md5 hash from the security group rule description when creating the rule map keys.

why

• This change prevents key map duplication when generating the security group rule map and there are rules with the same port and different descriptions. Example below:

main.tf

module "security_groups" 
  source = "github.com/cloudposse/terraform-aws-security-group"

  name        =  "sg_name"
  vpc_id      = "vpc-0000111122223333"
  rules       = [
        {
          type        = "ingress"
          from_port   = 22
          to_port     = 22
          protocol    = "tcp"
          cidr_blocks = "10.10.0.0/16"
          description = "Allow ssh from main office"
        },
        {
          type        = "ingress"
          from_port   = 22
          to_port     = 22
          protocol    = "tcp"
          cidr_blocks = "192.168.0.0/24"
          description = "Allow ssh from management VPC"
        },
        {
          type        = "egress"
          from_port   = 0
          to_port     = 65535
          protocol    = "all"
          cidr_blocks = ["0.0.0.0/0"]
          description = "Allow all outbound"
        }
      ]
  description = "sg_descrition"
}

ERROR

Error: Duplicate object key

  on .terraform/modules/security_groups/main.tf line 11, in locals:
   9:   rules = module.this.enabled && var.rules != null ? {
  10:     for rule in flatten(distinct(var.rules)) :
  11:     format("%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
  12:       rule.type,
  13:       rule.protocol,
  14:       rule.from_port,
  15:       rule.to_port,
  16:       lookup(rule, "cidr_blocks", null) == null ? "no_ipv4" : "ipv4",
  17:       lookup(rule, "ipv6_cidr_blocks", null) == null ? "no_ipv6" : "ipv6",
  18:       lookup(rule, "security_group_id", null) == null ? "no_ssg" : "ssg",
  19:       lookup(rule, "prefix_list_ids", null) == null ? "no_pli" : "pli",
  20:       lookup(rule, "self", null) == null ? "no_self" : "self",
  21:       lookup(rule, "description", null) == null ? "no_desc" : "desc"
  22:     ) => rule
  23:   } : {}
    |----------------
    | rule.from_port is 22
    | rule.protocol is "tcp"
    | rule.to_port is 22
    | rule.type is "ingress"

Two different items produced the key
"ingress-tcp-22-22-ipv4-no_ipv6-no_ssg-no_pli-no_self-desc" in this 'for'
expression. If duplicates are expected, use the ellipsis (...) after the value
expression to enable grouping by key.

references

v0.1.2

🤖 Automatic Updates

Update context.tf @cloudpossebot (#3)

what

This is an auto-generated PR that updates the context.tf file to the latest version from cloudposse/terraform-null-label

why

To support all the features of the context interface.

v0.1.1

🤖 Automatic Updates

Update README.md and docs @cloudpossebot (#4)

what

This is an auto-generated PR that updates the README.md and docs

why

To have most recent changes of README.md and doc from origin templates