Merge pull request #10 from truefoundry/hl/update-efs

feat(efs): add EFS file system policy and update module source

Author: shubhamrai1993

README.md

@@ -19,16 +19,17 @@ Truefoundry AWS EFS Module
 
 | Name | Source | Version |
 |------|--------|---------|
-| <a name="module_efs"></a> [efs](#module\_efs) | terraform-aws-modules/efs/aws | 1.6.3 |
+| <a name="module_efs"></a> [efs](#module\_efs) | cloudposse/efs/aws | 1.1.0 |
 | <a name="module_iam_assumable_role_admin"></a> [iam\_assumable\_role\_admin](#module\_iam\_assumable\_role\_admin) | terraform-aws-modules/iam/aws//modules/iam-assumable-role-with-oidc | 5.27.0 |
 
 ## Resources
 
 | Name | Type |
 |------|------|
+| [aws_efs_file_system_policy.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/efs_file_system_policy) | resource |
 | [aws_iam_policy.efs](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource |
 | [aws_iam_policy_document.efs](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
-| [aws_subnet.selected](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/subnet) | data source |
+| [aws_iam_policy_document.efs_file_system_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 
 ## Inputs
 
@@ -42,7 +43,8 @@ Truefoundry AWS EFS Module
 | <a name="input_k8s_service_account_namespace"></a> [k8s\_service\_account\_namespace](#input\_k8s\_service\_account\_namespace) | The k8s efs namespace | `string` | n/a | yes |
 | <a name="input_performance_mode"></a> [performance\_mode](#input\_performance\_mode) | the performance mode for EFS | `string` | n/a | yes |
 | <a name="input_private_subnets_cidrs"></a> [private\_subnets\_cidrs](#input\_private\_subnets\_cidrs) | List of CIDR of private subnets | `list(string)` | n/a | yes |
-| <a name="input_private_subnets_id"></a> [private\_subnets\_id](#input\_private\_subnets\_id) | List of IDs of private subnets | `list(string)` | n/a | yes |
+| <a name="input_private_subnets_id"></a> [private\_subnets\_id](#input\_private\_subnets\_id) | List of private subnet IDs | `list(string)` | n/a | yes |
+| <a name="input_region"></a> [region](#input\_region) | The region where the EFS will be provisioned | `string` | n/a | yes |
 | <a name="input_tags"></a> [tags](#input\_tags) | AWS Tags common to all the resources created | `map(string)` | `{}` | no |
 | <a name="input_throughput_mode"></a> [throughput\_mode](#input\_throughput\_mode) | the throughput mode for EFS | `string` | n/a | yes |
 | <a name="input_vpc_id"></a> [vpc\_id](#input\_vpc\_id) | ID of the VPC where the cluster and its nodes will be provisioned | `string` | `null` | no |

UPGRADE.md

@@ -0,0 +1,51 @@
+# Upgrade Guide
+
+This guide helps you upgrade the EFS module from version v0.3.5 and below to newer versions.
+
+## From v0.3.5 to v0.4.0
+
+### Breaking Changes
+
+1. The EFS filesystem resource name has been changed from [aws_efs_module](https://registry.terraform.io/modules/terraform-aws-modules/efs/aws/latest) to [cloudposse_efs](https://registry.terraform.io/modules/cloudposse/efs/aws/latest), due to count dependency on data block inputs. This helps us generate a one-shot plan for the whole Truefoundry installation.
+
+### Required Actions
+
+1. First, create a backup of your terraform state:
+
+```bash
+# For local state
+terraform state pull > terraform.tfstate.backup.$(date +%Y%m%d_%H%M%S)
+```
+
+2. Before running `terraform plan`, you need to migrate the state of the EFS resources to prevent destruction and recreation. Execute the following commands in order:
+
+```bash
+# Move EFS file system
+terraform state mv 'module.efs.module.efs.aws_efs_file_system.default[0]' 'module.efs.module.efs.aws_efs_file_system.this[0]'
+
+# Move mount targets
+terraform state mv 'module.efs.module.efs.aws_efs_mount_target.default[0]' 'module.efs.module.efs.aws_efs_mount_target.this[0]'
+terraform state mv 'module.efs.module.efs.aws_efs_mount_target.default[1]' 'module.efs.module.efs.aws_efs_mount_target.this[1]'
+terraform state mv 'module.efs.module.efs.aws_efs_mount_target.default[2]' 'module.efs.module.efs.aws_efs_mount_target.this[2]'
+
+# Move file system policy
+terraform state mv 'module.efs.aws_efs_file_system_policy.default' 'module.efs.aws_efs_file_system_policy.this'
+
+# Move backup policy
+terraform state mv 'module.efs.module.efs.aws_efs_backup_policy.default[0]' 'module.efs.module.efs.aws_efs_backup_policy.policy[0]'
+
+### Verification Steps
+
+1. Run all the state move commands mentioned above
+2. Execute `terraform plan`
+3. Verify that the plan does not show destruction of any EFS resources
+4. The plan should only show changes related to the resource name changes
+5. If the plan shows EFS resource destruction, DO NOT APPLY and review the state migration steps
+
+### Notes
+
+- Always backup your Terraform state before performing any state migrations
+- Test these changes in a non-production environment first
+- If you encounter any issues during the upgrade, please refer to the module documentation or open an issue in the repository
+- Make sure to execute the state move commands in the order specified above
+- If you have a different number of mount targets, adjust the mount target migration commands accordingly

efs.tf

@@ -4,6 +4,14 @@ resource "aws_iam_policy" "efs" {
   policy      = data.aws_iam_policy_document.efs.json
   tags        = local.tags
 }
+
+resource "aws_efs_file_system_policy" "this" {
+  file_system_id                     = module.efs.id
+  bypass_policy_lockout_safety_check = false
+  policy                             = data.aws_iam_policy_document.efs_file_system_policy.json
+}
+
+
 # https://github.com/kubernetes-sigs/aws-efs-csi-driver/blob/master/docs/iam-policy-example.json
 data "aws_iam_policy_document" "efs" {
   statement {
@@ -14,9 +22,7 @@ data "aws_iam_policy_document" "efs" {
       "elasticfilesystem:DescribeMountTargets",
       "ec2:DescribeAvailabilityZones"
     ]
-    resources = [
-      "*"
-    ]
+    resources = ["*"]
   }
   statement {
     effect = "Allow"
@@ -62,44 +68,42 @@ data "aws_iam_policy_document" "efs" {
   }
 }
 
+# EFS file system policy
+data "aws_iam_policy_document" "efs_file_system_policy" {
+  statement {
+    sid     = "EFS-CSI-Driver-Access"
+    effect  = "Allow"
+    actions = ["elasticfilesystem:ClientMount", "elasticfilesystem:ClientWrite", "elasticfilesystem:ClientRootAccess"]
+    principals {
+      type        = "AWS"
+      identifiers = [var.efs_node_iam_role_arn]
+    }
+    condition {
+      test     = "Bool"
+      values   = ["true"]
+      variable = "elasticfilesystem:AccessedViaMountTarget"
+    }
+  }
+}
+
 module "efs" {
-  source  = "terraform-aws-modules/efs/aws"
-  version = "1.6.3"
+  source  = "cloudposse/efs/aws"
+  version = "1.1.0"
+
+  region           = var.region
+  vpc_id           = var.vpc_id
+  subnets          = var.private_subnets_id
+  allow_all_egress = false
 
-  name = "${var.cluster_name}-efs"
+  allowed_cidr_blocks   = var.private_subnets_cidrs
+  create_security_group = true
+  name                  = "${var.cluster_name}-efs"
 
-  mount_targets                      = local.mount_targets
   security_group_description         = "${var.cluster_name} EFS"
-  security_group_vpc_id              = var.vpc_id
-  attach_policy                      = true
   bypass_policy_lockout_safety_check = false
-  policy_statements = [
-    {
-      sid     = "EFS-CSI-Driver-Access"
-      actions = ["elasticfilesystem:ClientMount", "elasticfilesystem:ClientWrite", "elasticfilesystem:ClientRootAccess"]
-      principals = [
-        {
-          type        = "AWS"
-          identifiers = [var.efs_node_iam_role_arn]
-        }
-      ]
-      conditions = [{
-        test     = "Bool"
-        values   = ["true"]
-        variable = "elasticfilesystem:AccessedViaMountTarget"
-      }]
-    }
-  ]
-  throughput_mode      = var.throughput_mode
-  performance_mode     = var.performance_mode
-  enable_backup_policy = var.enable_backup_policy
-  security_group_rules = {
-    vpc = {
-      # relying on the defaults provdied for EFS/NFS (2049/TCP + ingress)
-      description = "NFS ingress from VPC private subnets"
-      cidr_blocks = var.private_subnets_cidrs
-    }
-  }
+  throughput_mode                    = var.throughput_mode
+  performance_mode                   = var.performance_mode
+  efs_backup_policy_enabled          = var.enable_backup_policy
 
   tags = merge(
     local.tags

iam-sa.tf

@@ -14,4 +14,4 @@ module "iam_assumable_role_admin" {
     aws_iam_policy.efs.arn
   ]
   tags = local.tags
-}
+}

locals.tf

@@ -1,8 +1,3 @@
-data "aws_subnet" "selected" {
-  count = length(var.private_subnets_id)
-  id    = var.private_subnets_id[count.index]
-}
-
 locals {
   tags = merge(
     {
@@ -12,8 +7,4 @@ locals {
     },
     var.tags
   )
-  subnets = merge({ for _, v in data.aws_subnet.selected : v.availability_zone => v.id... })
-  mount_targets = merge({ for k, v in local.subnets : k => {
-    subnet_id = v[0]
-  } })
-}
+}

variables.tf

@@ -10,7 +10,7 @@ variable "vpc_id" {
 }
 
 variable "private_subnets_id" {
-  description = "List of IDs of private subnets"
+  description = "List of private subnet IDs"
   type        = list(string)
 }
 
@@ -69,4 +69,9 @@ variable "tags" {
   type        = map(string)
   default     = {}
   description = "AWS Tags common to all the resources created"
-}
+}
+
+variable "region" {
+  description = "The region where the EFS will be provisioned"
+  type        = string
+}