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

Add support for argument availability_zone_name #89

Closed
wants to merge 4 commits into from

Conversation

auebergang
Copy link

what

  • Adds support for argument availability_zone_name

why

  • availability_zone_name creates the filesystem as a One Zone storage class

references

The AWS Availability Zone in which to create the file system. Used to
create a file system that uses One Zone storage classes.
@auebergang auebergang requested a review from a team as a code owner September 27, 2021 01:46
Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

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

Bridgecrew has found 1 infrastructure configuration error in this PR ⬇️

@@ -6,6 +6,7 @@ locals {
resource "aws_efs_file_system" "default" {
count = module.this.enabled ? 1 : 0
tags = module.this.tags
availability_zone_name = var.availability_zone_name
Copy link

@bridgecrew bridgecrew bot Sep 27, 2021

Choose a reason for hiding this comment

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

LOW   Amazon EFS does not have an AWS Backup backup plan
    Resource: aws_efs_file_system.default | ID: BC_AWS_GENERAL_48

How to Fix

resource "aws_backup_plan" "example" {
  name = "tf_example_backup_plan"

  rule {
    rule_name         = "tf_example_backup_rule"
    target_vault_name = aws_backup_vault.test.name
    schedule          = "cron(0 12 * * ? *)"
  }

  advanced_backup_setting {
    backup_options = {
      WindowsVSS = "enabled"
    }
    resource_type = "EC2"
  }
}

resource "aws_backup_selection" "ok_backup" {
  iam_role_arn = aws_iam_role.example.arn
  name         = "tf_example_backup_selection"
  plan_id      = aws_backup_plan.example.id

  resources = [
    aws_db_instance.example.arn,
    aws_ebs_volume.example.arn,
    aws_efs_file_system.ok_efs.arn,
  ]
}

resource "aws_efs_file_system" "ok_efs" {
  creation_token = "my-product"

  tags = {
    Name = "MyProduct"
  }
}



Description

Ensure that Elastic File System (Amazon EFS) file systems are included in your backup plans for the AWS Backup.

Dependent Resources



Path Resource Connecting Attribute
/main.tf aws_efs_mount_target.default file_system_id
/main.tf aws_efs_access_point.default file_system_id

Copy link
Member

Choose a reason for hiding this comment

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

@auebergang can you do us a favor and add the following above line 7?

#bridgecrew:skip=BC_AWS_GENERAL_48 - Skipping as backup plan can be added via https://github.com/cloudposse/terraform-aws-backup

See here for an example: https://github.com/cloudposse/terraform-aws-alb/blob/master/main.tf#L140

@Gowiem
Copy link
Member

Gowiem commented Sep 27, 2021

/test all

@Gowiem Gowiem self-assigned this Sep 27, 2021
@Gowiem
Copy link
Member

Gowiem commented Sep 27, 2021

@auebergang This looks good and I'm happy to get it merged, but we are hitting two issues:

  1. The security issue with BridgeCrew. See the above comment inline on BC's comment.
  2. Auto format is failing -- Try running the following locally:
make init
make readme
git add . 
git commit -m "chore: readme updates"
git push

Alex Uebergang added 3 commits September 27, 2021 13:44
Add note about providing a single subnet in the same AZ when setting
this argument
@auebergang auebergang requested a review from a team as a code owner September 27, 2021 04:03
@auebergang
Copy link
Author

Updated. I've added an update to the var description as well to note providing a single subnet in the same AZ.

@nitrocode
Copy link
Member

You'll need to add an empty commit or some commit that will update this PR so validate owners will work.

@nitrocode
Copy link
Member

/test all

@nitrocode nitrocode changed the title Add support for argument availability_zone_name BLOCKED BY SG: Add support for argument availability_zone_name Oct 8, 2021
@nitrocode
Copy link
Member

nitrocode commented Oct 8, 2021

This repo is still in a pre-release until we upgrade the security group module to use 0.4.0 so this change is blocked until then unfortunately.

@nitrocode nitrocode changed the title BLOCKED BY SG: Add support for argument availability_zone_name Add support for argument availability_zone_name Nov 10, 2021
@nitrocode
Copy link
Member

Thanks for your contribution! This has been released as version 0.32.0.

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

Successfully merging this pull request may close these issues.

3 participants