aws-ia/terraform-aws-mwaa

Terraform module for Amazon MWAA(Apache Airflow)

terraform-aws-mwaa's Issues

How do you set log retention?

What's the easiest way to configure the log retention policy on MWAA cloudwatch logs? This module allows users to toggle the logs and configure the level but it doesn't seem to allow us to configure the log retention period, nor does the module output the log group arn.

S3 Bucket Arn is mandatory argument when providing S3 Bucket Name

When we run the mwaa artefact we get below error message :

│ Error: Missing required argument
│
│ with aws_mwaa_environment.mwaa,
│ on main.tf line 23, in resource "aws_mwaa_environment" "mwaa":
│ 23: source_bucket_arn = local.source_bucket_arn
│
│ The argument "source_bucket_arn" is required, but no definition was found.

I have a PR which resolves this dependency. #36

Add example to run sample Glue and EMR Jobs with RAEDME

Add an example to trigger a simple python job

• Add a new folder samples under examples
• Add a readme with steps to execute the jobs

Terratest for module testing

• Identify AWS Account for E2E tests using Terratest
• Build Terratest scripts

AWS version pinning

I noticed that the AWS provider version is pinned to 4.20, is there a reason for that?

For context I'm wondering because this conflicts with the version requirement of https://github.com/terraform-aws-modules/terraform-aws-rds/blob/master/modules/db_parameter_group/versions.tf

https://github.com/aws-ia/terraform-aws-mwaa/blob/main/versions.tf

aws_mwaa_environment gets destroyed and recreated unnecessarily

I'm seeing a behavior in aws_mwaa_environment that I was not seeing previously. With hashicorp/aws v4.64.0, every time I deploy, it destroys and recreates the aws_mwaa_environment with this message shown in the terraform apply:

  ~ network_configuration {
      ~ security_group_ids = [
          - "sg-xxxxxxxxx",
        ] -> (known after apply)
      ~ subnet_ids         = [ # forces replacement
          - "subnet-xxxxxxxxx",
          - "subnet-yyyyyyyyy",
        ] -> (known after apply)
    }
}

Even though the subnet_ids are exactly the same every single time, it indicates that they have changed and therefore that is forcing replacement.

Originally these 2 subnet_ids were coming from an aws_subnet data block, with filter that resulted in those 2 exact same subnet_ids every time. Just as a test, I simply hardcoded the 2 subnet_ids in the aws_mwaa_environment resource block:

subnet_ids = ["subnet-xxxxxxxxx", "subnet-yyyyyyyyy"]

But even hardcoded, I get the same behavior.

What is the intended deployment pattern for changing requirements and plugins?

I see that the lifecycle settings ignore changes to both the requirements and plugins objects

  lifecycle {
    ignore_changes = [
      plugins_s3_object_version,
      requirements_s3_object_version
    ]
  }

Is there some kind of bad/unintended behavior if these changes are not ignored? Or is this an intentional decision to encourage these these updates via the console only?

Deployment Architecture Diagram

Architecture diagram of the most pertinent services and concepts

Please use Powerpoint and use the official AWS icons: https://aws.amazon.com/architecture/icons/

place both a ppt and a png in images/

Add secret_prefix var so that one can grant MWAA access to secrets under a specific prefix

See #23. There is a need for MWAA to have access to certain secrets so it can connect to various API's. Granting MWAA access to all secrets would be a security concern, as it should only have access to the secrets it needs, so we should grant it access to secrets under a specified prefix.

MWAA executor role requires 2 extra permissions for airflow resource

I got the permission error with "airflow:ListEnvironments" and later "airflow:GetEnvironment".
Also those permissions being added to policy with manual MWAA creation.

Fix is lines 49, 50 in data.tf

Update README

Update README for standard template

Fully private MWAA environment with example

• New Example to deploy fully private MWAA environment
• Create VPC an VPC Endpoints with Private Only MWAA environment

Output of VPC endpoint id if it was created

Can you add output of VPC endpoint if it was create during environment provisioning ?

Enable support for setting KMS key for S3 bucket and MWAA

I want to request a feature to enable setting KMS key for MWAA. It is required to use the same key for both S3 and MWAA instance and there will be some additional IAM policies involved in the creation process.

Using a CMK with MWAA
Permissions involved when setting a CMK

Feature Request: Allow passthrough arns for cloudwatch log groups

Currently, the terraform resource for MWAA has the capability to create cloudwatch logs using the logging_configuration block. Our organization would like to be able to output logs to an existing cloudwatch log group by specifying an arn. This would allow us to get access to all of the features available when creating a log group using the aws_cloudwatch_log_group resource (such as log retention).

Let me know if this is the wrong location for such a feature request - I'd be happy to open it in the appropriate location

var.source_bucket_name should be respected as the S3 bucket name and not the prefix

When setting var.source_bucket_name, it turns out that it's only setting the bucket name prefix (see here) and not the actual name of the bucket. I'm not sure why the behavior is different given the variable's name and description. The other name variables, var.name and var.iam_role_name, are respected and passed through as the names for those resources, so I'm not sure why it's inconsistent for the s3 bucket. Is this something that y'all would be open to? I am more than happy to contribute this change.

I understand that this proposal will be a breaking change, so we could have another variable var.source_bucket_name_prefix that preserves the original behavior.

iam_role_additional_policies and external IAM Roles

When bringing external iam role with below config

  create_iam_role       = false
  execution_role_arn    = data.aws_iam_role.mwaa.arn
  iam_role_additional_policies = []

TF throws below error

│ Error: Invalid object key
│ 
│   on .terraform/modules/mwaa/locals.tf line 14, in locals:
│   14:   iam_role_additional_policies = { for k, v in toset(concat([var.iam_role_additional_policies])) : k => v if var.execution_role_arn != null }
│ 

Upon verification, terraform-aws-eks uses a similar pattern, but with different variable types

iam_role_additional_policies in var should be map(string) rather than list(string)

Also, the if conditional should not be checking external role, it should be checking create_iam_role

The concact should enclose var.iam_role_additional_policies with []. Detail see below screenshot

> { for k, v in toset(concat([[]])) : k => v if "asdf" != null }
╷
│ Error: Invalid object key
│ 
│   on <console-input> line 1:
│   (source code not available)
│ 
│ The key expression produced an invalid result: string required.
╵


> { for k, v in toset(concat([[]])) : k => v if null != null }
{}


> { for k, v in toset(concat([])) : k => v if "asdf" != null }
{}

if needed, we can discuss about the detail using aws internal channels.

Add support for startup scripts

The mwaa_environment resource now supports startup_script_s3_path and startup_script_s3_object_version.

This would be a nice addition to the provider :)

(note: this issue was created with the intention of making a PR for it myself, I will edit this comment if I cannot do so for some reason)

Configuring tags don't apply to the MWAA assets created

In the variables.tf, when configuring tags like the following

variable "tags" {
  description = "Default tags"
  default     = {"env": "test", "service": "MWAA Apache AirFlow"}
  type        = map(string)
}

These will get applied to the VPC resources configured, but not the MWAA resources.

Security issue: MWAA has more access than it needs

MWAA has "eks:*" access, but it does not need EKS access by default. It does not need S3 delete permissions by default either. Finally, I don't think it needs batch permissions.

#23 fixes this.