feat!: Use defaultBucket (#312) *BREAKING CHANGE*

* feat: Use defaultBucket * use create_bucket flag * remove unused * use var.bucket_name * remove unused var * use {} * temp save * example update * add moved * fix moved * migration note * Update README.md
wandb · Jan 14, 2025 · 6044819 · 6044819
1 parent 633e06c
commit 6044819
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 33 deletions.
diff --git a/README.md b/README.md
@@ -373,6 +373,13 @@ This can be donw by adding the following policy document.
     }
 ```
 
+### 6.x -> 7.x
+
+`v7` changes how the module references storage from using terraform's `count` to always creating a "defaultBucket" which can be overidden latter or but providing some initial bucket.
+
+We are considering this a major change because of the terraform `moved` block which migrates the resource. After moving to a `v7` applying an earlier version of the module may result in terraform deleting your bucket.
+
+removed the `create_bucket` var due to the above.
 ### Upgrading from 2.x -> 3.x
 
 - No changes required by you

diff --git a/data.tf b/data.tf
@@ -1,6 +1,6 @@
 data "aws_s3_bucket" "file_storage" {
   depends_on = [module.file_storage]
-  bucket     = local.bucket_name
+  bucket     = local.main_bucket_name
 }
 
 data "aws_sqs_queue" "file_storage" {
@@ -9,4 +9,4 @@ data "aws_sqs_queue" "file_storage" {
   name       = local.bucket_queue_name
 }
 
-data "aws_region" "current" {}
+data "aws_region" "current" {}
diff --git a/examples/byo-vpc-eks-sql-redis/main.tf b/examples/byo-vpc-eks-sql-redis/main.tf
@@ -13,7 +13,7 @@ provider "aws" {
 }
 data "aws_s3_bucket" "file_storage" {
   depends_on = [module.file_storage]
-  bucket     = local.bucket_name
+  bucket     = var.bucket_name
 }
 
 data "aws_sqs_queue" "file_storage" {
@@ -61,7 +61,6 @@ locals {
 }
 
 module "file_storage" {
-  count  = var.create_bucket ? 1 : 0
   source = "../../modules/file_storage"
 
   create_queue        = !local.use_internal_queue
@@ -72,8 +71,7 @@ module "file_storage" {
 }
 
 locals {
-  bucket_name       = local.use_external_bucket ? var.bucket_name : module.file_storage.0.bucket_name
-  bucket_queue_name = local.use_internal_queue ? null : module.file_storage.0.bucket_queue_name
+  bucket_queue_name = local.use_internal_queue ? null : module.file_storage.bucket_queue_name
 }
 
 locals {
@@ -176,11 +174,17 @@ module "wandb" {
 
         extraEnv = var.other_wandb_env
 
-        bucket = {
+        bucket = var.bucket_name != "" ? {
           provider = "s3"
-          name     = local.bucket_name
+          name     = var.bucket_name
           region   = data.aws_s3_bucket.file_storage.region
-          kmsKey   = local.use_external_bucket ? var.bucket_kms_key_arn : local.kms_key_arn
+          kmsKey   = var.bucket_kms_key_arn
+        } : null
+        defaultBucket = {
+          provider = "s3"
+          name     = module.file_storage.bucket_name
+          region   = module.file_storage.bucket_region
+          kmsKey   = module.kms.key.arn
         }
 
         mysql = {

diff --git a/examples/byo-vpc-sql/main.tf b/examples/byo-vpc-sql/main.tf
@@ -13,7 +13,7 @@ provider "aws" {
 }
 data "aws_s3_bucket" "file_storage" {
   depends_on = [module.file_storage]
-  bucket     = local.bucket_name
+  bucket     = var.bucket_name
 }
 
 data "aws_sqs_queue" "file_storage" {
@@ -102,7 +102,6 @@ locals {
 }
 
 module "file_storage" {
-  count  = var.create_bucket ? 1 : 0
   source = "../../modules/file_storage"
 
   create_queue        = !local.use_internal_queue
@@ -113,7 +112,6 @@ module "file_storage" {
 }
 
 locals {
-  bucket_name       = local.use_external_bucket ? var.bucket_name : module.file_storage.0.bucket_name
   bucket_queue_name = local.use_internal_queue ? null : module.file_storage.0.bucket_queue_name
 }
 
@@ -167,7 +165,7 @@ module "app_eks" {
   map_users      = var.kubernetes_map_users
 
   bucket_kms_key_arns  = local.use_external_bucket ? var.bucket_kms_key_arn : local.kms_key_arn
-  bucket_arn           = data.aws_s3_bucket.file_storage.arn
+  bucket_arn           = var.bucket_name == "" ? module.file_storage.bucket_arn : data.aws_s3_bucket.file_storage.arn
   bucket_sqs_queue_arn = local.use_internal_queue ? null : data.aws_sqs_queue.file_storage.0.arn
 
   network_id              = local.network_id
@@ -191,6 +189,12 @@ module "app_eks" {
   system_reserved_pid                 = var.system_reserved_pid
 
   aws_loadbalancer_controller_tags = var.aws_loadbalancer_controller_tags
+
+  eks_addon_efs_csi_driver_version = var.eks_addon_efs_csi_driver_version
+  eks_addon_ebs_csi_driver_version = var.eks_addon_ebs_csi_driver_version
+  eks_addon_coredns_version        = var.eks_addon_coredns_version
+  eks_addon_kube_proxy_version     = var.eks_addon_kube_proxy_version
+  eks_addon_vpc_cni_version        = var.eks_addon_vpc_cni_version
 }
 
 locals {
@@ -281,11 +285,18 @@ module "wandb" {
 
         extraEnv = var.other_wandb_env
 
-        bucket = {
+        bucket = var.bucket_name != "" ? {
           provider = "s3"
-          name     = local.bucket_name
+          name     = var.bucket_name
           region   = data.aws_s3_bucket.file_storage.region
-          kmsKey   = local.use_external_bucket ? var.bucket_kms_key_arn : local.kms_key_arn
+          kmsKey   = var.bucket_kms_key_arn
+        } : null
+
+        defaultBucket = {
+          provider = "s3"
+          name     = module.file_storage.bucket_name
+          region   = module.file_storage.bucket_region
+          kmsKey   = module.kms.key.arn
         }
 
         mysql = {

diff --git a/examples/byo-vpc-sql/variables.tf b/examples/byo-vpc-sql/variables.tf
@@ -403,6 +403,41 @@ variable "aws_loadbalancer_controller_tags" {
   default     = {}
 }
 
+
+##########################################
+# EKS Cluster Addons                     #
+##########################################
+variable "eks_addon_efs_csi_driver_version" {
+  description = "The version of the EFS CSI driver to install. Check the docs for more information about the compatibility https://docs.aws.amazon.com/eks/latest/userguide/vpc-add-on-update.html."
+  type        = string
+  default     = "v2.0.7-eksbuild.1"
+}
+
+variable "eks_addon_ebs_csi_driver_version" {
+  description = "The version of the EBS CSI driver to install. Check the docs for more information about the compatibility https://docs.aws.amazon.com/eks/latest/userguide/vpc-add-on-update.html."
+  type        = string
+  default     = "v1.35.0-eksbuild.1"
+}
+
+variable "eks_addon_coredns_version" {
+  description = "The version of the CoreDNS addon to install. Check the docs for more information about the compatibility https://docs.aws.amazon.com/eks/latest/userguide/vpc-add-on-update.html."
+  type        = string
+  default     = "v1.11.3-eksbuild.1"
+}
+
+variable "eks_addon_kube_proxy_version" {
+  description = "The version of the kube-proxy addon to install. Check the docs for more information about the compatibility https://docs.aws.amazon.com/eks/latest/userguide/vpc-add-on-update.html."
+  type        = string
+  default     = "v1.29.7-eksbuild.9"
+}
+
+variable "eks_addon_vpc_cni_version" {
+  description = "The version of the VPC CNI addon to install. Check the docs for more information about the compatibility https://docs.aws.amazon.com/eks/latest/userguide/vpc-add-on-update.html.s"
+  type        = string
+  default     = "v1.18.3-eksbuild.3"
+}
+
+
 ##########################################
 # External Bucket                        #
 ##########################################
@@ -419,12 +454,14 @@ variable "bucket_name" {
 }
 
 variable "bucket_kms_key_arn" {
-  type        = string
-  description = "The Amazon Resource Name of the KMS key with which S3 storage bucket objects will be encrypted."
-  default     = ""
+  type    = string
+  default = ""
+  validation {
+    condition     = can(regex("^arn:aws:kms:[a-z0-9-]+:[0-9]+:key/[a-zA-Z0-9-_]+$", var.bucket_kms_key_arn)) || var.bucket_kms_key_arn == ""
+    error_message = "Invalid value for bucket kms ARN"
+  }
 }
 
-
 ##########################################
 # Redis                                  #
 ##########################################

diff --git a/main.tf b/main.tf
@@ -29,7 +29,6 @@ locals {
 }
 
 module "file_storage" {
-  count               = var.create_bucket ? 1 : 0
   source              = "./modules/file_storage"
   namespace           = var.namespace
   create_queue        = !local.use_internal_queue
@@ -39,8 +38,8 @@ module "file_storage" {
 }
 
 locals {
-  bucket_name       = local.use_external_bucket ? var.bucket_name : module.file_storage[0].bucket_name
-  bucket_queue_name = local.use_internal_queue ? null : module.file_storage[0].bucket_queue_name
+  bucket_queue_name = local.use_internal_queue ? null : module.file_storage.bucket_queue_name
+  main_bucket_name  = var.bucket_name != "" ? var.bucket_name : module.file_storage.bucket_name
 }
 
 module "networking" {
@@ -272,12 +271,18 @@ module "wandb" {
         cloudProvider = "aws"
         extraEnv      = var.other_wandb_env
 
-        bucket = {
+        bucket = var.bucket_name != "" ? {
           provider = "s3"
-          name     = local.bucket_name
+          name     = var.bucket_name
           path     = var.bucket_path
           region   = data.aws_s3_bucket.file_storage.region
-          kmsKey   = local.s3_kms_key_arn
+          kmsKey   = var.bucket_kms_key_arn
+        } : {}
+        defaultBucket = {
+          provider = "s3"
+          name     = module.file_storage.bucket_name
+          region   = module.file_storage.bucket_region
+          kmsKey   = module.kms.key.arn
         }
 
         mysql = {
@@ -374,3 +379,8 @@ module "wandb" {
     }
   }
 }
+
+moved {
+  from = module.file_storage[0]
+  to   = module.file_storage
+}
diff --git a/outputs.tf b/outputs.tf
@@ -1,5 +1,5 @@
 output "bucket_name" {
-  value = local.bucket_name
+  value = local.main_bucket_name
 }
 output "bucket_path" {
   value = var.bucket_path

diff --git a/variables.tf b/variables.tf
@@ -398,7 +398,9 @@ variable "aws_loadbalancer_controller_tags" {
   type        = map(string)
   default     = {}
 }
-
+##########################################
+# EKS Cluster Addons                     #
+##########################################
 variable "eks_addon_efs_csi_driver_version" {
   description = "The version of the EFS CSI driver to install. Check the docs for more information about the compatibility https://docs.aws.amazon.com/eks/latest/userguide/vpc-add-on-update.html."
   type        = string
@@ -435,11 +437,6 @@ variable "eks_addon_vpc_cni_version" {
 ##########################################
 # Most users will not need these settings. They are ment for users who want a
 # bucket and sqs that are in a different account.
-variable "create_bucket" {
-  type    = bool
-  default = true
-}
-
 variable "bucket_name" {
   type    = string
   default = ""