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

Feat: Use Security Group 4.x Module #94

Merged
merged 80 commits into from
Nov 10, 2021
Merged

Feat: Use Security Group 4.x Module #94

merged 80 commits into from
Nov 10, 2021

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Nov 5, 2021

what

  • Use standardized Cloud Posse Security Group convention
  • Bump cloudposse/terraform-aws-route53-cluster-hostname module
  • Bump example/complete vpc and subnet modules
  • Remove unnecessary provider pins
  • Bump aws provider to 3.x
  • Run make github/init

why

  • update GHA-related files to their latest distribution from build-harness
  • new Security Group standards
  • Unblock new PRs from entering this repo

references

commands

Verified enabled=false
⨠ terraform plan -var-file=fixtures.us-east-2.tfvars -var="enabled=false"

Changes to Outputs:
  + efs_mount_target_dns_names = [
      + "",
    ]
  + efs_mount_target_ids       = [
      + "",
    ]
  + efs_mount_target_ips       = [
      + "",
    ]
  + efs_network_interface_ids  = [
      + "",
    ]
  + private_subnet_cidrs       = []
  + public_subnet_cidrs        = []
Backwards compatibility with 0.30.1 and earlier using security_group_name override

This is to avoid recreation of the efs file system due to the name change

⨠ terraform plan -var-file=fixtures.us-east-2.tfvars -var=security_group_name=snip -var=security_group_change_before_destroy=false

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create
 <= read (data resources)

Terraform will perform the following actions:

  # module.efs.aws_efs_file_system.default[0] will be created
  + resource "aws_efs_file_system" "default" {
      + arn                             = (known after apply)
      + availability_zone_id            = (known after apply)
      + availability_zone_name          = (known after apply)
      + creation_token                  = (known after apply)
      + dns_name                        = (known after apply)
      + encrypted                       = true
      + id                              = (known after apply)
      + kms_key_id                      = (known after apply)
      + number_of_mount_targets         = (known after apply)
      + owner_id                        = (known after apply)
      + performance_mode                = "generalPurpose"
      + provisioned_throughput_in_mibps = 0
      + size_in_bytes                   = (known after apply)
      + tags                            = {
          + "Name"      = "eg-test-efs-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + tags_all                        = {
          + "Name"      = "eg-test-efs-test"
          + "Namespace" = "eg"
          + "Stage"     = "test"
        }
      + throughput_mode                 = "bursting"
    }

  # module.efs.module.security_group.aws_security_group.default[0] will be created
  + resource "aws_security_group" "default" {
      + arn                    = (known after apply)
      + description            = "MSK broker access"
      + egress                 = (known after apply)
      + id                     = (known after apply)
      + ingress                = (known after apply)
      + name                   = "eg-test-efs-test-efs"
      + name_prefix            = (known after apply)
      + owner_id               = (known after apply)
      + revoke_rules_on_delete = false
      + tags                   = {
          + "Attributes" = "efs"
          + "Name"       = "eg-test-efs-test-efs"
          + "Namespace"  = "eg"
          + "Stage"      = "test"
        }
      + tags_all               = {
          + "Attributes" = "efs"
          + "Name"       = "eg-test-efs-test-efs"
          + "Namespace"  = "eg"
          + "Stage"      = "test"
        }
      + vpc_id                 = (known after apply)

      + timeouts {
          + create = "10m"
          + delete = "15m"
        }
    }

...

Plan: 24 to add, 0 to change, 0 to destroy.

Changes to Outputs:
...
  + security_group_name        = "eg-test-efs-test-efs"
Deployed 0.30.1, performed `state mv`s, and then ran a plan to ensure nothing would break
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create
  ~ update in-place
+/- create replacement and then destroy

Terraform will perform the following actions:

  # module.efs.aws_efs_backup_policy.policy[0] will be created
  + resource "aws_efs_backup_policy" "policy" {
      + file_system_id = "fs-snip"
      + id             = (known after apply)

      + backup_policy {
          + status = "DISABLED"
        }
    }

  # module.efs.aws_efs_file_system.default[0] will be updated in-place
  ~ resource "aws_efs_file_system" "default" {
        id                              = "fs-snip"
        tags                            = {
            "Environment" = "snip"
            "Name"        = "snip-snip-snip-snip-efs"
            "Namespace"   = "snip"
            "Stage"       = "snip"
            "Tenant"      = "snip"
        }
        # (12 unchanged attributes hidden)

      + lifecycle_policy {}
    }

  # module.efs.aws_efs_mount_target.default[0] will be updated in-place
  ~ resource "aws_efs_mount_target" "default" {
        id                     = "fsmt-snip"
      ~ security_groups        = [
          + "sg-snip",
            # (1 unchanged element hidden)
        ]
        # (10 unchanged attributes hidden)
    }

  # module.efs.aws_efs_mount_target.default[1] will be updated in-place
  ~ resource "aws_efs_mount_target" "default" {
        id                     = "fsmt-snip"
      ~ security_groups        = [
          + "sg-snip",
            # (1 unchanged element hidden)
        ]
        # (10 unchanged attributes hidden)
    }

  # module.efs.aws_efs_mount_target.default[2] will be updated in-place
  ~ resource "aws_efs_mount_target" "default" {
        id                     = "fsmt-snip"
      ~ security_groups        = [
          + "sg-snip",
            # (1 unchanged element hidden)
        ]
        # (10 unchanged attributes hidden)
    }

  # module.efs.module.security_group.aws_security_group.default[0] will be updated in-place
  ~ resource "aws_security_group" "default" {
        id                     = "sg-snip"
        name                   = "snip-snip-snip-snip-efs-efs"
      ~ tags                   = {
          ~ "Name"        = "snip-snip-snip-snip-efs" -> "snip-snip-snip-snip-efs-efs"
            # (4 unchanged elements hidden)
        }
      ~ tags_all               = {
          ~ "Name"        = "snip-snip-snip-snip-efs" -> "snip-snip-snip-snip-efs-efs"
            # (4 unchanged elements hidden)
        }
        # (7 unchanged attributes hidden)

      + timeouts {
          + create = "10m"
          + delete = "15m"
        }
    }

  # module.efs.module.security_group.aws_security_group_rule.keyed["_allow_all_egress_"] must be replaced
+/- resource "aws_security_group_rule" "keyed" {
      + description              = "Allow all egress"
      ~ id                       = "sgrule-2361699180" -> (known after apply)
      ~ ipv6_cidr_blocks         = [ # forces replacement
          + "::/0",
        ]
      + source_security_group_id = (known after apply)
        # (8 unchanged attributes hidden)
    }

  # module.efs.module.security_group.aws_security_group_rule.keyed["_m[0]#[0]#sg#0"] will be updated in-place
  ~ resource "aws_security_group_rule" "keyed" {
      ~ description              = "Allow inbound traffic from existing security groups" -> "Allow ingress EFS traffic"
        id                       = "sgrule-snip"
        # (10 unchanged attributes hidden)
    }

Plan: 2 to add, 6 to change, 1 to destroy.

@nitrocode nitrocode marked this pull request as ready for review November 5, 2021 17:10
@nitrocode nitrocode requested review from a team as code owners November 5, 2021 17:10
@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode requested a review from a team as a code owner November 5, 2021 17:14
@nitrocode
Copy link
Member Author

/test all

main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few comments

@nitrocode
Copy link
Member Author

/test all

nitrocode and others added 3 commits November 8, 2021 13:51
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@nitrocode
Copy link
Member Author

/test all

Nuru
Nuru previously requested changes Nov 10, 2021
Copy link
Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted changes to availability_zone_name and kms_key_id because they were not worth breaking backward compatibility for, because there is no Terraform logic that is affected by the value.

I restored associated_security_group_ids because it is useful.

README.yaml Outdated Show resolved Hide resolved
docs/migration-0.30.1-0.32.x+.md Outdated Show resolved Hide resolved
docs/migration-0.30.1-0.32.x+.md Outdated Show resolved Hide resolved
nitrocode and others added 2 commits November 9, 2021 18:42
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@mergify mergify bot dismissed Nuru’s stale review November 10, 2021 00:42

This Pull Request has been updated, so we're dismissing all reviews.

@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode requested review from Nuru and korenyoni November 10, 2021 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add transition_out_ia lifecycle to EFS
7 participants