From 2c0ec7b1a3cff5d565d0146db44c8a4f82c9b5a4 Mon Sep 17 00:00:00 2001
From: Veronika Gnilitska <veronika.gnilitska@gmail.com>
Date: Thu, 28 Aug 2025 20:14:39 +0300
Subject: [PATCH 1/2] feat: allow passing VPC and Subnet names

---
 data.tf                                 |  86 +++++++++++++++++-
 examples/with-names/main.tf             |  75 ++++++++++++++++
 examples/with-names/outputs.tf          |  14 +++
 examples/with-names/variables.tf        |  29 ++++++
 main.tf                                 | 114 +++++++++---------------
 tests/vpc-subnet-locals-only.tftest.hcl |  88 ++++++++++++++++++
 variables.tf                            |  14 +++
 7 files changed, 345 insertions(+), 75 deletions(-)
 create mode 100644 examples/with-names/main.tf
 create mode 100644 examples/with-names/outputs.tf
 create mode 100644 examples/with-names/variables.tf
 create mode 100644 tests/vpc-subnet-locals-only.tftest.hcl

diff --git a/data.tf b/data.tf
index 74472cb..c418ca2 100644
--- a/data.tf
+++ b/data.tf
@@ -1,6 +1,26 @@
 data "aws_region" "current" {}
 data "aws_caller_identity" "current" {}
 
+# VPC lookup by name (when vpc_name is provided)
+data "aws_vpc" "selected" {
+  count = length(var.vpc_name) > 0 ? 1 : 0
+
+  filter {
+    name   = "tag:Name"
+    values = [var.vpc_name]
+  }
+}
+
+# Individual subnet lookup by name (when subnet_names are provided)
+data "aws_subnet" "selected" {
+  for_each = toset(var.subnet_names)
+
+  filter {
+    name   = "tag:Name"
+    values = [each.value]
+  }
+}
+
 # Most recent Amazon Linux 2023 AMI
 data "aws_ami" "amazon_linux_2023" {
   most_recent = true
@@ -33,7 +53,7 @@ data "aws_ami" "amazon_linux_2023" {
 # This rule was introduced in the following PR:
 # https://github.com/masterpointio/terraform-aws-ssm-agent/pull/43.
 #
-# trunk-ignore(trivy/AVD-AWS-0344)
+# trivy:ignore:AVD-AWS-0344
 data "aws_ami" "instance" {
   count = length(var.ami) > 0 ? 1 : 0
 
@@ -44,3 +64,67 @@ data "aws_ami" "instance" {
     values = [var.ami]
   }
 }
+
+# IAM policy document for EC2 instances to assume the SSM Agent role
+data "aws_iam_policy_document" "default" {
+
+  statement {
+    effect  = "Allow"
+    actions = ["sts:AssumeRole"]
+
+    principals {
+      type        = "Service"
+      identifiers = ["ec2.amazonaws.com"]
+    }
+  }
+}
+
+# S3 bucket for session logging (when using existing bucket)
+data "aws_s3_bucket" "logs_bucket" {
+  count  = var.session_logging_enabled ? 1 : 0
+  bucket = try(coalesce(var.session_logging_bucket_name, module.logs_bucket.bucket_id), "")
+}
+
+# https://docs.aws.amazon.com/systems-manager/latest/userguide/getting-started-create-iam-instance-profile.html#create-iam-instance-profile-ssn-logging
+data "aws_iam_policy_document" "session_logging" {
+  count = var.session_logging_enabled ? 1 : 0
+
+  statement {
+    sid    = "SSMAgentSessionAllowS3Logging"
+    effect = "Allow"
+    actions = [
+      "s3:PutObject"
+    ]
+    resources = ["${join("", data.aws_s3_bucket.logs_bucket.*.arn)}/*"]
+  }
+
+  statement {
+    sid    = "SSMAgentSessionAllowCloudWatchLogging"
+    effect = "Allow"
+    actions = [
+      "logs:CreateLogStream",
+      "logs:PutLogEvents",
+      "logs:DescribeLogGroups",
+      "logs:DescribeLogStreams"
+    ]
+    resources = ["*"]
+  }
+
+  statement {
+    sid    = "SSMAgentSessionAllowGetEncryptionConfig"
+    effect = "Allow"
+    actions = [
+      "s3:GetEncryptionConfiguration"
+    ]
+    resources = ["*"]
+  }
+
+  statement {
+    sid    = "SSMAgentSessionAllowKMSDataKey"
+    effect = "Allow"
+    actions = [
+      "kms:GenerateDataKey"
+    ]
+    resources = ["*"]
+  }
+}
diff --git a/examples/with-names/main.tf b/examples/with-names/main.tf
new file mode 100644
index 0000000..ccddd4f
--- /dev/null
+++ b/examples/with-names/main.tf
@@ -0,0 +1,75 @@
+provider "aws" {
+  region = var.region
+}
+
+# Create VPC and subnets with specific names for demonstration
+module "vpc" {
+  source  = "cloudposse/vpc/aws"
+  version = "2.1.0"
+
+  namespace = var.namespace
+  stage     = var.stage
+  name      = "example-vpc"
+
+  ipv4_primary_cidr_block          = "10.0.0.0/16"
+  assign_generated_ipv6_cidr_block = true
+}
+
+module "subnets" {
+  source    = "cloudposse/dynamic-subnets/aws"
+  version   = "2.3.0"
+  namespace = var.namespace
+  stage     = var.stage
+  name      = "example-subnets"
+
+  availability_zones = var.availability_zones
+  vpc_id             = module.vpc.vpc_id
+  igw_id             = [module.vpc.igw_id]
+  ipv4_cidr_block    = [module.vpc.vpc_cidr_block]
+  ipv6_enabled       = var.ipv6_enabled
+}
+
+# Example 1: Using VPC and subnet IDs (traditional approach)
+module "ssm_agent_with_ids" {
+  source    = "../../"
+  stage     = var.stage
+  namespace = var.namespace
+  name      = "ssm-with-ids"
+
+  vpc_id     = module.vpc.vpc_id
+  subnet_ids = module.subnets.private_subnet_ids
+}
+
+# Example 2: Using VPC and subnet names (new functionality)
+module "ssm_agent_with_names" {
+  source    = "../../"
+  stage     = var.stage
+  namespace = var.namespace
+  name      = "ssm-with-names"
+
+  vpc_name = module.vpc.vpc_id_tag_name # This would be the Name tag value
+  subnet_names = [
+    # These would be the actual Name tag values of the subnets
+    # In a real scenario, you'd know these names or get them from data sources
+    "example-private-subnet-1",
+    "example-private-subnet-2"
+  ]
+
+  depends_on = [module.subnets]
+}
+
+# Example 3: Mixed configuration (VPC by ID, subnets by name)
+module "ssm_agent_mixed" {
+  source    = "../../"
+  stage     = var.stage
+  namespace = var.namespace
+  name      = "ssm-mixed"
+
+  vpc_id = module.vpc.vpc_id
+  subnet_names = [
+    "example-private-subnet-1",
+    "example-private-subnet-2"
+  ]
+
+  depends_on = [module.subnets]
+}
diff --git a/examples/with-names/outputs.tf b/examples/with-names/outputs.tf
new file mode 100644
index 0000000..11f3a3d
--- /dev/null
+++ b/examples/with-names/outputs.tf
@@ -0,0 +1,14 @@
+output "ssm_agent_with_ids_instance_profile_arn" {
+  description = "ARN of the IAM instance profile for SSM agent (using IDs)"
+  value       = module.ssm_agent_with_ids.iam_instance_profile_arn
+}
+
+output "ssm_agent_with_names_instance_profile_arn" {
+  description = "ARN of the IAM instance profile for SSM agent (using names)"
+  value       = module.ssm_agent_with_names.iam_instance_profile_arn
+}
+
+output "ssm_agent_mixed_instance_profile_arn" {
+  description = "ARN of the IAM instance profile for SSM agent (mixed approach)"
+  value       = module.ssm_agent_mixed.iam_instance_profile_arn
+}
diff --git a/examples/with-names/variables.tf b/examples/with-names/variables.tf
new file mode 100644
index 0000000..a1648b4
--- /dev/null
+++ b/examples/with-names/variables.tf
@@ -0,0 +1,29 @@
+variable "region" {
+  type        = string
+  description = "AWS region"
+  default     = "us-east-1"
+}
+
+variable "namespace" {
+  type        = string
+  description = "Namespace for resource naming"
+  default     = "mp"
+}
+
+variable "stage" {
+  type        = string
+  description = "Stage for resource naming"
+  default     = "test"
+}
+
+variable "availability_zones" {
+  type        = list(string)
+  description = "List of availability zones"
+  default     = ["us-east-1a", "us-east-1b"]
+}
+
+variable "ipv6_enabled" {
+  type        = bool
+  description = "Enable IPv6"
+  default     = false
+}
diff --git a/main.tf b/main.tf
index 8cf7cbb..26bb09d 100644
--- a/main.tf
+++ b/main.tf
@@ -14,6 +14,10 @@ locals {
     length(var.ami) > 0 ? element(data.aws_ami.instance, 0).root_device_name : "/dev/xvda"
   )
 
+  # VPC and Subnet ID resolution - names take precedence over IDs
+  vpc_id     = length(var.vpc_name) > 0 ? one(data.aws_vpc.selected[*].id) : var.vpc_id
+  subnet_ids = length(var.subnet_names) > 0 ? values(data.aws_subnet.selected)[*].id : var.subnet_ids
+
 }
 
 resource "null_resource" "validate_instance_type" {
@@ -27,6 +31,34 @@ resource "null_resource" "validate_instance_type" {
   }
 }
 
+# Validation using terraform_data to halt execution if requirements aren't met
+resource "terraform_data" "vpc_subnet_validation" {
+  lifecycle {
+    precondition {
+      condition     = length(var.vpc_name) > 0 || length(var.vpc_id) > 0
+      error_message = "Either vpc_name or vpc_id must be provided."
+    }
+
+    precondition {
+      condition     = length(var.subnet_names) > 0 || length(var.subnet_ids) > 0
+      error_message = "Either subnet_names or subnet_ids must be provided."
+    }
+  }
+}
+
+# Warning checks for VPC and subnet configuration (non-blocking)
+check "vpc_subnet_warnings" {
+  assert {
+    condition     = !(length(var.vpc_name) > 0 && length(var.vpc_id) > 0)
+    error_message = "Both vpc_name and vpc_id are provided. When vpc_name is specified, vpc_id will be ignored."
+  }
+
+  assert {
+    condition     = !(length(var.subnet_names) > 0 && length(var.subnet_ids) > 0)
+    error_message = "Both subnet_names and subnet_ids are provided. When subnet_names are specified, subnet_ids will be ignored."
+  }
+}
+
 module "role_label" {
   source  = "cloudposse/label/null"
   version = "0.25.0"
@@ -57,68 +89,6 @@ locals {
 ## SSM AGENT ROLE ##
 ###################
 
-data "aws_iam_policy_document" "default" {
-
-  statement {
-    effect  = "Allow"
-    actions = ["sts:AssumeRole"]
-
-    principals {
-      type        = "Service"
-      identifiers = ["ec2.amazonaws.com"]
-    }
-  }
-}
-
-data "aws_s3_bucket" "logs_bucket" {
-  count  = var.session_logging_enabled ? 1 : 0
-  bucket = try(coalesce(var.session_logging_bucket_name, module.logs_bucket.bucket_id), "")
-}
-
-# https://docs.aws.amazon.com/systems-manager/latest/userguide/getting-started-create-iam-instance-profile.html#create-iam-instance-profile-ssn-logging
-data "aws_iam_policy_document" "session_logging" {
-  count = var.session_logging_enabled ? 1 : 0
-
-  statement {
-    sid    = "SSMAgentSessionAllowS3Logging"
-    effect = "Allow"
-    actions = [
-      "s3:PutObject"
-    ]
-    resources = ["${join("", data.aws_s3_bucket.logs_bucket.*.arn)}/*"]
-  }
-
-  statement {
-    sid    = "SSMAgentSessionAllowCloudWatchLogging"
-    effect = "Allow"
-    actions = [
-      "logs:CreateLogStream",
-      "logs:PutLogEvents",
-      "logs:DescribeLogGroups",
-      "logs:DescribeLogStreams"
-    ]
-    resources = ["*"]
-  }
-
-  statement {
-    sid    = "SSMAgentSessionAllowGetEncryptionConfig"
-    effect = "Allow"
-    actions = [
-      "s3:GetEncryptionConfiguration"
-    ]
-    resources = ["*"]
-  }
-
-  statement {
-    sid    = "SSMAgentSessionAllowKMSDataKey"
-    effect = "Allow"
-    actions = [
-      "kms:GenerateDataKey"
-    ]
-    resources = ["*"]
-  }
-}
-
 resource "aws_iam_role" "default" {
   name                 = module.role_label.id
   assume_role_policy   = data.aws_iam_policy_document.default.json
@@ -157,12 +127,13 @@ resource "aws_iam_instance_profile" "default" {
 ###################
 
 resource "aws_security_group" "default" {
-  vpc_id      = var.vpc_id
+  vpc_id      = local.vpc_id
   name        = module.this.id
   description = "Allow ALL egress from SSM Agent."
   tags        = module.this.tags
 }
 
+# trivy:ignore:aws-vpc-no-public-egress-sgr SSM Agent requires broad egress to communicate with AWS services
 resource "aws_security_group_rule" "allow_all_egress" {
   type              = "egress"
   from_port         = 0
@@ -170,6 +141,7 @@ resource "aws_security_group_rule" "allow_all_egress" {
   protocol          = "-1"
   cidr_blocks       = ["0.0.0.0/0"]
   ipv6_cidr_blocks  = ["::/0"]
+  description       = "Allow all outbound traffic for SSM Agent communication with AWS services"
   security_group_id = aws_security_group.default.id
 }
 
@@ -246,21 +218,15 @@ DOC
 
 module "logs_bucket" {
   source  = "cloudposse/s3-bucket/aws"
-  version = "3.1.2"
+  version = "4.10.0"
 
   enabled = local.logs_bucket_enabled
   context = module.logs_label.context
 
   # Encryption / Security
-  acl                          = "private"
-  sse_algorithm                = "aws:kms"
-  kms_master_key_arn           = local.session_logging_kms_key_arn
-  allow_encrypted_uploads_only = false
-  force_destroy                = true
-
-  # Feature enablement
-  user_enabled       = false
-  versioning_enabled = true
+  sse_algorithm      = "aws:kms"
+  kms_master_key_arn = local.session_logging_kms_key_arn
+  force_destroy      = true
 
   lifecycle_configuration_rules = [{
     enabled                                = true
@@ -385,7 +351,7 @@ resource "aws_autoscaling_group" "default" {
   # By default, we don't care to protect from scale in as we want to roll instances frequently
   protect_from_scale_in = var.protect_from_scale_in
 
-  vpc_zone_identifier = var.subnet_ids
+  vpc_zone_identifier = local.subnet_ids
 
   default_cooldown          = 180
   health_check_grace_period = 180
diff --git a/tests/vpc-subnet-locals-only.tftest.hcl b/tests/vpc-subnet-locals-only.tftest.hcl
new file mode 100644
index 0000000..de85cb0
--- /dev/null
+++ b/tests/vpc-subnet-locals-only.tftest.hcl
@@ -0,0 +1,88 @@
+# Minimal tests for VPC and subnet locals logic only
+
+mock_provider "aws" {
+  mock_data "aws_vpc" {
+    defaults = {
+      id = "vpc-from-name"
+    }
+  }
+
+  mock_data "aws_subnet" {
+    defaults = {
+      id = "subnet-from-name"
+    }
+  }
+
+  mock_data "aws_region" {
+    defaults = {
+      name = "us-east-1"
+    }
+  }
+
+  mock_data "aws_caller_identity" {
+    defaults = {
+      account_id = "123456789012"
+    }
+  }
+
+  mock_data "aws_ami" {
+    defaults = {
+      id = "ami-mock"
+      root_device_name = "/dev/xvda"
+    }
+  }
+
+  mock_data "aws_iam_policy_document" {
+    defaults = {
+      json = "{\"Version\":\"2012-10-17\",\"Statement\":[]}"
+    }
+  }
+
+  mock_data "aws_s3_bucket" {
+    defaults = {
+      id = "mock-bucket"
+      arn = "arn:aws:s3:::mock-bucket"
+    }
+  }
+
+  # Mock AWS resources that get created in the module
+  mock_resource "aws_launch_template" {
+    defaults = {
+      id = "lt-mock123456"
+      latest_version = "1"
+    }
+  }
+}
+
+# Test precedence - names over IDs
+run "test_precedence_local" {
+  command = plan
+
+  variables {
+    vpc_id       = "vpc-should-be-ignored"
+    vpc_name     = "my-vpc"
+    subnet_ids   = ["subnet-should-be-ignored"]
+    subnet_names = ["my-subnet"]
+    session_logging_enabled = false
+
+    # Add required context variables for proper naming
+    namespace = "test"
+    stage     = "unit"
+    name      = "ssm-agent"
+  }
+
+  # Expect warnings from check blocks since we're providing both IDs and names
+  expect_failures = [
+    check.vpc_subnet_warnings
+  ]
+
+  assert {
+    condition     = local.vpc_id == "vpc-from-name"
+    error_message = "VPC name should take precedence"
+  }
+
+  assert {
+    condition     = contains(local.subnet_ids, "subnet-from-name")
+    error_message = "Subnet names should take precedence"
+  }
+}
diff --git a/variables.tf b/variables.tf
index fd95fa6..1ab359b 100644
--- a/variables.tf
+++ b/variables.tf
@@ -1,11 +1,25 @@
 variable "vpc_id" {
   type        = string
   description = "The ID of the VPC which the EC2 Instance will run in."
+  default     = ""
 }
 
 variable "subnet_ids" {
   type        = list(string)
   description = "The Subnet IDs which the SSM Agent will run in. These *should* be private subnets."
+  default     = []
+}
+
+variable "vpc_name" {
+  type        = string
+  description = "The name of the VPC which the EC2 Instance will run in. If provided, vpc_id will be ignored."
+  default     = ""
+}
+
+variable "subnet_names" {
+  type        = list(string)
+  description = "The Subnet names which the SSM Agent will run in. If provided, subnet_ids will be ignored. These *should* be private subnets."
+  default     = []
 }
 
 variable "permissions_boundary" {

From 21f62857ae80c6a4809410f2f5bf6a92929cf0da Mon Sep 17 00:00:00 2001
From: Veronika Gnilitska <veronika.gnilitska@gmail.com>
Date: Fri, 29 Aug 2025 12:39:52 +0300
Subject: [PATCH 2/2] chore: security best-practices + minor improvements

---
 data.tf                                       | 30 +++++++++-------
 examples/with-names/outputs.tf                | 14 --------
 examples/with-names/variables.tf              |  6 ----
 main.tf                                       | 35 +++----------------
 outputs.tf                                    |  4 +--
 ...ftest.hcl => vpc-subnet-locals.tftest.hcl} | 16 ++++-----
 validations.tf                                | 27 ++++++++++++++
 variables.tf                                  | 13 +++++--
 8 files changed, 71 insertions(+), 74 deletions(-)
 delete mode 100644 examples/with-names/outputs.tf
 rename tests/{vpc-subnet-locals-only.tftest.hcl => vpc-subnet-locals.tftest.hcl} (81%)
 create mode 100644 validations.tf

diff --git a/data.tf b/data.tf
index c418ca2..9a178ac 100644
--- a/data.tf
+++ b/data.tf
@@ -3,7 +3,7 @@ data "aws_caller_identity" "current" {}
 
 # VPC lookup by name (when vpc_name is provided)
 data "aws_vpc" "selected" {
-  count = length(var.vpc_name) > 0 ? 1 : 0
+  count = var.vpc_name != null ? 1 : 0
 
   filter {
     name   = "tag:Name"
@@ -19,6 +19,10 @@ data "aws_subnet" "selected" {
     name   = "tag:Name"
     values = [each.value]
   }
+  filter {
+    name   = "vpc-id"
+    values = [local.vpc_id]
+  }
 }
 
 # Most recent Amazon Linux 2023 AMI
@@ -79,12 +83,6 @@ data "aws_iam_policy_document" "default" {
   }
 }
 
-# S3 bucket for session logging (when using existing bucket)
-data "aws_s3_bucket" "logs_bucket" {
-  count  = var.session_logging_enabled ? 1 : 0
-  bucket = try(coalesce(var.session_logging_bucket_name, module.logs_bucket.bucket_id), "")
-}
-
 # https://docs.aws.amazon.com/systems-manager/latest/userguide/getting-started-create-iam-instance-profile.html#create-iam-instance-profile-ssn-logging
 data "aws_iam_policy_document" "session_logging" {
   count = var.session_logging_enabled ? 1 : 0
@@ -95,7 +93,7 @@ data "aws_iam_policy_document" "session_logging" {
     actions = [
       "s3:PutObject"
     ]
-    resources = ["${join("", data.aws_s3_bucket.logs_bucket.*.arn)}/*"]
+    resources = ["${local.session_logging_bucket_arn}/*"]
   }
 
   statement {
@@ -103,11 +101,19 @@ data "aws_iam_policy_document" "session_logging" {
     effect = "Allow"
     actions = [
       "logs:CreateLogStream",
-      "logs:PutLogEvents",
+      "logs:PutLogEvents"
+    ]
+    resources = ["${local.session_logging_log_group_arn}:*"]
+  }
+
+  statement {
+    sid    = "SSMAgentSessionAllowCloudWatchDescribe"
+    effect = "Allow"
+    actions = [
       "logs:DescribeLogGroups",
       "logs:DescribeLogStreams"
     ]
-    resources = ["*"]
+    resources = [local.session_logging_log_group_arn]
   }
 
   statement {
@@ -116,7 +122,7 @@ data "aws_iam_policy_document" "session_logging" {
     actions = [
       "s3:GetEncryptionConfiguration"
     ]
-    resources = ["*"]
+    resources = [local.session_logging_bucket_arn]
   }
 
   statement {
@@ -125,6 +131,6 @@ data "aws_iam_policy_document" "session_logging" {
     actions = [
       "kms:GenerateDataKey"
     ]
-    resources = ["*"]
+    resources = [local.session_logging_kms_key_arn]
   }
 }
diff --git a/examples/with-names/outputs.tf b/examples/with-names/outputs.tf
deleted file mode 100644
index 11f3a3d..0000000
--- a/examples/with-names/outputs.tf
+++ /dev/null
@@ -1,14 +0,0 @@
-output "ssm_agent_with_ids_instance_profile_arn" {
-  description = "ARN of the IAM instance profile for SSM agent (using IDs)"
-  value       = module.ssm_agent_with_ids.iam_instance_profile_arn
-}
-
-output "ssm_agent_with_names_instance_profile_arn" {
-  description = "ARN of the IAM instance profile for SSM agent (using names)"
-  value       = module.ssm_agent_with_names.iam_instance_profile_arn
-}
-
-output "ssm_agent_mixed_instance_profile_arn" {
-  description = "ARN of the IAM instance profile for SSM agent (mixed approach)"
-  value       = module.ssm_agent_mixed.iam_instance_profile_arn
-}
diff --git a/examples/with-names/variables.tf b/examples/with-names/variables.tf
index a1648b4..4867ca8 100644
--- a/examples/with-names/variables.tf
+++ b/examples/with-names/variables.tf
@@ -21,9 +21,3 @@ variable "availability_zones" {
   description = "List of availability zones"
   default     = ["us-east-1a", "us-east-1b"]
 }
-
-variable "ipv6_enabled" {
-  type        = bool
-  description = "Enable IPv6"
-  default     = false
-}
diff --git a/main.tf b/main.tf
index 26bb09d..6a80e67 100644
--- a/main.tf
+++ b/main.tf
@@ -15,7 +15,7 @@ locals {
   )
 
   # VPC and Subnet ID resolution - names take precedence over IDs
-  vpc_id     = length(var.vpc_name) > 0 ? one(data.aws_vpc.selected[*].id) : var.vpc_id
+  vpc_id     = var.vpc_name != null ? one(data.aws_vpc.selected[*].id) : var.vpc_id
   subnet_ids = length(var.subnet_names) > 0 ? values(data.aws_subnet.selected)[*].id : var.subnet_ids
 
 }
@@ -31,33 +31,6 @@ resource "null_resource" "validate_instance_type" {
   }
 }
 
-# Validation using terraform_data to halt execution if requirements aren't met
-resource "terraform_data" "vpc_subnet_validation" {
-  lifecycle {
-    precondition {
-      condition     = length(var.vpc_name) > 0 || length(var.vpc_id) > 0
-      error_message = "Either vpc_name or vpc_id must be provided."
-    }
-
-    precondition {
-      condition     = length(var.subnet_names) > 0 || length(var.subnet_ids) > 0
-      error_message = "Either subnet_names or subnet_ids must be provided."
-    }
-  }
-}
-
-# Warning checks for VPC and subnet configuration (non-blocking)
-check "vpc_subnet_warnings" {
-  assert {
-    condition     = !(length(var.vpc_name) > 0 && length(var.vpc_id) > 0)
-    error_message = "Both vpc_name and vpc_id are provided. When vpc_name is specified, vpc_id will be ignored."
-  }
-
-  assert {
-    condition     = !(length(var.subnet_names) > 0 && length(var.subnet_ids) > 0)
-    error_message = "Both subnet_names and subnet_ids are provided. When subnet_names are specified, subnet_ids will be ignored."
-  }
-}
 
 module "role_label" {
   source  = "cloudposse/label/null"
@@ -79,8 +52,10 @@ locals {
   region     = coalesce(var.region, data.aws_region.current.region)
   account_id = data.aws_caller_identity.current.account_id
 
-  session_logging_bucket_name = try(coalesce(var.session_logging_bucket_name, module.logs_label.id), "")
-  session_logging_kms_key_arn = try(coalesce(var.session_logging_kms_key_arn, module.kms_key.key_arn), "")
+  session_logging_bucket_name   = try(coalesce(var.session_logging_bucket_name, module.logs_label.id), "")
+  session_logging_kms_key_arn   = try(coalesce(var.session_logging_kms_key_arn, module.kms_key.key_arn), "")
+  session_logging_bucket_arn    = var.session_logging_enabled ? "arn:aws:s3:::${local.session_logging_bucket_name}" : ""
+  session_logging_log_group_arn = var.session_logging_enabled ? "arn:aws:logs:${local.region}:${local.account_id}:log-group:${module.logs_label.id}" : ""
 
   logs_bucket_enabled = var.session_logging_enabled && length(var.session_logging_bucket_name) == 0
 }
diff --git a/outputs.tf b/outputs.tf
index 2c672d5..c70d871 100644
--- a/outputs.tf
+++ b/outputs.tf
@@ -24,11 +24,11 @@ output "role_id" {
 }
 
 output "session_logging_bucket_id" {
-  value       = local.logs_bucket_enabled ? join("", data.aws_s3_bucket.logs_bucket.*.id) : ""
+  value       = var.session_logging_enabled ? local.session_logging_bucket_name : ""
   description = "The ID of the SSM Agent Session Logging S3 Bucket."
 }
 
 output "session_logging_bucket_arn" {
-  value       = local.logs_bucket_enabled ? join("", data.aws_s3_bucket.logs_bucket.*.arn) : ""
+  value       = local.session_logging_bucket_arn
   description = "The ARN of the SSM Agent Session Logging S3 Bucket."
 }
diff --git a/tests/vpc-subnet-locals-only.tftest.hcl b/tests/vpc-subnet-locals.tftest.hcl
similarity index 81%
rename from tests/vpc-subnet-locals-only.tftest.hcl
rename to tests/vpc-subnet-locals.tftest.hcl
index de85cb0..c26d6e7 100644
--- a/tests/vpc-subnet-locals-only.tftest.hcl
+++ b/tests/vpc-subnet-locals.tftest.hcl
@@ -27,7 +27,7 @@ mock_provider "aws" {
 
   mock_data "aws_ami" {
     defaults = {
-      id = "ami-mock"
+      id               = "ami-mock"
       root_device_name = "/dev/xvda"
     }
   }
@@ -40,7 +40,7 @@ mock_provider "aws" {
 
   mock_data "aws_s3_bucket" {
     defaults = {
-      id = "mock-bucket"
+      id  = "mock-bucket"
       arn = "arn:aws:s3:::mock-bucket"
     }
   }
@@ -48,21 +48,21 @@ mock_provider "aws" {
   # Mock AWS resources that get created in the module
   mock_resource "aws_launch_template" {
     defaults = {
-      id = "lt-mock123456"
+      id             = "lt-mock123456"
       latest_version = "1"
     }
   }
 }
 
 # Test precedence - names over IDs
-run "test_precedence_local" {
+run "verify_vpc_subnet_name_precedence" {
   command = plan
 
   variables {
-    vpc_id       = "vpc-should-be-ignored"
-    vpc_name     = "my-vpc"
-    subnet_ids   = ["subnet-should-be-ignored"]
-    subnet_names = ["my-subnet"]
+    vpc_id                  = "vpc-should-be-ignored"
+    vpc_name                = "my-vpc"
+    subnet_ids              = ["subnet-should-be-ignored"]
+    subnet_names            = ["my-subnet"]
     session_logging_enabled = false
 
     # Add required context variables for proper naming
diff --git a/validations.tf b/validations.tf
new file mode 100644
index 0000000..8652dc2
--- /dev/null
+++ b/validations.tf
@@ -0,0 +1,27 @@
+# Validation using terraform_data to halt execution if requirements aren't met
+resource "terraform_data" "vpc_subnet_validation" {
+  lifecycle {
+    precondition {
+      condition     = var.vpc_name != null || var.vpc_id != null
+      error_message = "Either vpc_name or vpc_id must be provided."
+    }
+
+    precondition {
+      condition     = length(var.subnet_names) > 0 || length(var.subnet_ids) > 0
+      error_message = "Either subnet_names or subnet_ids must be provided."
+    }
+  }
+}
+
+# Warning checks for VPC and subnet configuration (non-blocking)
+check "vpc_subnet_warnings" {
+  assert {
+    condition     = !(var.vpc_name != null && var.vpc_id != null)
+    error_message = "Both vpc_name and vpc_id are provided. When vpc_name is specified, vpc_id will be ignored."
+  }
+
+  assert {
+    condition     = !(length(var.subnet_names) > 0 && length(var.subnet_ids) > 0)
+    error_message = "Both subnet_names and subnet_ids are provided. When subnet_names are specified, subnet_ids will be ignored."
+  }
+}
diff --git a/variables.tf b/variables.tf
index 1ab359b..d513d34 100644
--- a/variables.tf
+++ b/variables.tf
@@ -1,7 +1,7 @@
 variable "vpc_id" {
   type        = string
   description = "The ID of the VPC which the EC2 Instance will run in."
-  default     = ""
+  default     = null
 }
 
 variable "subnet_ids" {
@@ -13,7 +13,7 @@ variable "subnet_ids" {
 variable "vpc_name" {
   type        = string
   description = "The name of the VPC which the EC2 Instance will run in. If provided, vpc_id will be ignored."
-  default     = ""
+  default     = null
 }
 
 variable "subnet_names" {
@@ -172,6 +172,15 @@ variable "session_logging_bucket_name" {
   default     = ""
   type        = string
   description = "The name of the S3 Bucket to ship session logs to. This will remove creation of an independent session logging bucket. This is only relevant if the session_logging_enabled variable is `true`."
+
+  validation {
+    condition = var.session_logging_bucket_name == "" || (
+      can(regex("^[a-z0-9][a-z0-9.-]*[a-z0-9]$", var.session_logging_bucket_name)) &&
+      length(var.session_logging_bucket_name) >= 3 &&
+      length(var.session_logging_bucket_name) <= 63
+    )
+    error_message = "S3 bucket name must follow AWS naming conventions: 3-63 characters, lowercase letters, numbers, dots, and hyphens only, must start and end with letter or number."
+  }
 }
 
 variable "region" {