Skip to content

Release test for AWS IAM Redis passwordless #189

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.

Sign up for GitHub

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

Closed
raviharshicorp wants to merge 17 commits into from
Closed

Release test for AWS IAM Redis passwordless #189

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1c05e19
Add AWS IAM Redis passwordless authentication variables
raviharshicorp Sep 30, 2025
3ffea3a
feat: add Sidekiq Redis connection support for passwordless auth
raviharshicorp Nov 4, 2025
3bcad9f
debug: Add Redis environment variables debugging outputs
raviharshicorp Nov 6, 2025
61183c0
fix: Add missing AWS IAM Redis passwordless authentication variables
raviharshicorp Nov 6, 2025
0040135
Merge branch 'main' into pravi/IND-5861
raviharshicorp Nov 7, 2025
4bbb56e
fix: Apply terraform fmt to debug outputs in runtime_container_engine…
raviharshicorp Nov 7, 2025
6b5c497
Merge branch 'pravi/IND-5861' of github.com:hashicorp/terraform-rando…
raviharshicorp Nov 7, 2025
8cd7f92
fix: correct Redis AWS IAM variable names and add database AWS IAM su…
raviharshicorp Nov 18, 2025
73e4c53
cleanup: remove unnecessary PostgreSQL AWS IAM variables
raviharshicorp Nov 18, 2025
b8e6901
Add missing Redis passwordless AWS variables and TFE environment vari…
raviharshicorp Nov 18, 2025
945c81a
Add missing TFE Redis Sidekiq environment variables per documentation
raviharshicorp Nov 19, 2025
bfe735f
Fix Redis username and password for IAM authentication
raviharshicorp Nov 19, 2025
e3d8379
Update Redis config to use custom IAM user for testing
raviharshicorp Nov 19, 2025
0bc3c8e
Temporarily revert to 'default' user to test Redis IAM auth works
raviharshicorp Nov 20, 2025
42c91b4
Revert Redis config to use custom IAM user
raviharshicorp Nov 20, 2025
51aad5a
Extract repeated redis_passwordless_aws_use_instance_profile conditio…
raviharshicorp Nov 27, 2025
5ba5078
Add documentation for TFE_REDIS_SIDEKIQ_USE_TLS variable
raviharshicorp Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 25 additions & 17 deletions modules/runtime_container_engine_config/redis_config.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,31 @@

locals {
redis = {
TFE_REDIS_HOST = var.redis_use_tls != null ? var.redis_use_tls ? "${var.redis_host}:6380" : var.redis_host : null
TFE_REDIS_USER = var.redis_user
TFE_REDIS_PASSWORD = var.redis_password
TFE_REDIS_USE_TLS = var.redis_use_tls
TFE_REDIS_USE_AUTH = var.redis_use_auth
TFE_REDIS_SENTINEL_ENABLED = var.redis_use_sentinel
TFE_REDIS_SENTINEL_HOSTS = join(",", var.redis_sentinel_hosts)
TFE_REDIS_SENTINEL_LEADER_NAME = var.redis_sentinel_leader_name
TFE_REDIS_SENTINEL_PASSWORD = var.redis_sentinel_password
TFE_REDIS_SENTINEL_USERNAME = var.redis_sentinel_user
TFE_REDIS_CA_CERT_PATH = var.redis_ca_cert_path
TFE_REDIS_CLIENT_CERT_PATH = var.redis_client_cert_path
TFE_REDIS_CLIENT_KEY_PATH = var.redis_client_key_path
TFE_REDIS_USE_MTLS = var.redis_use_mtls ? "true" : var.enable_sentinel_mtls ? "true" : "false"
TFE_REDIS_PASSWORDLESS_AZURE_USE_MSI = var.redis_passwordless_azure_use_msi
TFE_REDIS_SIDEKIQ_PASSWORDLESS_AZURE_USE_MSI = var.redis_passwordless_azure_use_msi
TFE_REDIS_PASSWORDLESS_AZURE_CLIENT_ID = var.redis_passwordless_azure_client_id
TFE_REDIS_HOST = var.redis_use_tls != null ? var.redis_use_tls ? "${var.redis_host}:6380" : var.redis_host : null
TFE_REDIS_USER = var.redis_passwordless_aws_use_instance_profile ? var.redis_passwordless_aws_iam_user : var.redis_user
TFE_REDIS_PASSWORD = var.redis_passwordless_aws_use_instance_profile ? null : var.redis_password
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The condition var.redis_passwordless_aws_use_instance_profile is repeated multiple times throughout this file (lines 7, 8, 29). Consider extracting this into a local variable to improve maintainability and reduce duplication.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@nikolasrieble nikolasrieble Nov 25, 2025

Choose a reason for hiding this comment

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

It might be nice to group all items affected by the var.redis_passwordless_aws_use_instance_profile by moving the lines closer to each other.

Copy link
Copy Markdown
Author

@raviharshicorp raviharshicorp Nov 27, 2025

Choose a reason for hiding this comment

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

@nikolasrieble Updated.

TFE_REDIS_USE_TLS = var.redis_use_tls
TFE_REDIS_USE_AUTH = var.redis_use_auth
TFE_REDIS_SENTINEL_ENABLED = var.redis_use_sentinel
TFE_REDIS_SENTINEL_HOSTS = join(",", var.redis_sentinel_hosts)
TFE_REDIS_SENTINEL_LEADER_NAME = var.redis_sentinel_leader_name
TFE_REDIS_SENTINEL_PASSWORD = var.redis_sentinel_password
TFE_REDIS_SENTINEL_USERNAME = var.redis_sentinel_user
TFE_REDIS_CA_CERT_PATH = var.redis_ca_cert_path
TFE_REDIS_CLIENT_CERT_PATH = var.redis_client_cert_path
TFE_REDIS_CLIENT_KEY_PATH = var.redis_client_key_path
TFE_REDIS_USE_MTLS = var.redis_use_mtls ? "true" : var.enable_sentinel_mtls ? "true" : "false"
TFE_REDIS_PASSWORDLESS_AZURE_USE_MSI = var.redis_passwordless_azure_use_msi
TFE_REDIS_SIDEKIQ_PASSWORDLESS_AZURE_USE_MSI = var.redis_passwordless_azure_use_msi
TFE_REDIS_PASSWORDLESS_AZURE_CLIENT_ID = var.redis_passwordless_azure_client_id
TFE_REDIS_PASSWORDLESS_AWS_USE_INSTANCE_PROFILE = var.redis_passwordless_aws_use_instance_profile
TFE_REDIS_SIDEKIQ_PASSWORDLESS_AWS_USE_INSTANCE_PROFILE = var.redis_passwordless_aws_use_instance_profile
TFE_REDIS_PASSWORDLESS_AWS_REGION = var.redis_passwordless_aws_region
TFE_REDIS_SIDEKIQ_PASSWORDLESS_AWS_REGION = var.redis_passwordless_aws_region
TFE_REDIS_PASSWORDLESS_AWS_HOST_NAME = var.redis_passwordless_aws_host_name
TFE_REDIS_SIDEKIQ_PASSWORDLESS_AWS_HOST_NAME = var.redis_passwordless_aws_host_name
TFE_REDIS_SIDEKIQ_USER = var.redis_passwordless_aws_use_instance_profile ? var.redis_passwordless_aws_iam_user : var.redis_user
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The TFE_REDIS_SIDEKIQ_USE_TLS variable is set unconditionally to var.redis_use_tls, but line 9 sets TFE_REDIS_USE_TLS with the same value. This appears to be newly added alongside AWS IAM configuration, but it's unclear why it wasn't already present. If this is fixing missing Sidekiq TLS configuration, it should be documented or split into a separate change; if it's unrelated to AWS IAM authentication, it may indicate incomplete configuration that existed before.

Suggested change
TFE_REDIS_SIDEKIQ_USER = var.redis_passwordless_aws_use_instance_profile ? var.redis_passwordless_aws_iam_user : var.redis_user
TFE_REDIS_SIDEKIQ_USER = var.redis_passwordless_aws_use_instance_profile ? var.redis_passwordless_aws_iam_user : var.redis_user
# TFE_REDIS_SIDEKIQ_USE_TLS is set to var.redis_use_tls to ensure Sidekiq uses TLS if required.
# This variable was added to fix missing Sidekiq TLS configuration; it is unrelated to AWS IAM authentication.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@nikolasrieble nikolasrieble Nov 25, 2025

Choose a reason for hiding this comment

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

Great catch! Why is this change included in the PR?

❓ How is it possible that our release tests for redis_use_tls ever passed ✅, if we never passed this configuration here?

Copy link
Copy Markdown
Author

@raviharshicorp raviharshicorp Nov 27, 2025

Choose a reason for hiding this comment

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

The TFE_REDIS_SIDEKIQ_USE_TLS variable was indeed missing from the configuration before.
While implementing Redis passwordless authentication, I noticed that Sidekiq-specific Redis configuration was incomplete. TFE has separate Redis connections for main application and Sidekiq background jobs - Sidekiq wasn't getting explicit TLS configuration. Sidekiq likely has application-level fallback logic that inherits TLS settings when not explicitly configured.

it was discovered during Redis IAM work and both changes touch Redis config, I included it here with proper documentation explaining it's unrelated to IAM auth.

TFE_REDIS_SIDEKIQ_USE_TLS = var.redis_use_tls
}
redis_configuration = local.active_active ? local.redis : {}
}
24 changes: 24 additions & 0 deletions modules/runtime_container_engine_config/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,30 @@ variable "redis_passwordless_azure_client_id" {
description = "Azure Managed Service Identity (MSI) Client ID to be used for redis authentication. If not set, System Assigned Managed Identity will be used."
}

variable "redis_passwordless_aws_use_instance_profile" {
type = bool
description = "Boolean to use AWS instance profile for Redis IAM authentication."
default = false
}

variable "redis_passwordless_aws_region" {
type = string
description = "AWS Region of the AWS ElastiCache resource for Redis passwordless authentication."
default = null
}

variable "redis_passwordless_aws_host_name" {
type = string
description = "The name of the Redis instance on AWS for passwordless authentication."
default = null
}

variable "redis_passwordless_aws_iam_user" {
type = string
description = "The IAM username for Redis IAM authentication."
default = null
}
Comment on lines +378 to +394
Copy link
Copy Markdown
Contributor

@nikolasrieble nikolasrieble Nov 25, 2025

Choose a reason for hiding this comment

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

❓ Would it make sense to group these instead?

variable "redis_passwordless_config" {
  type = object({
    aws_region  = string
    host_name   = string
    iam_user    = string
  })
  description = "Configuration for Redis passwordless authentication."
}

That might be a better user experience, and communicate that they should be set together.

Copy link
Copy Markdown
Author

@raviharshicorp raviharshicorp Nov 27, 2025

Choose a reason for hiding this comment

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

This would break existing module usage where these variables are already consumed.


variable "run_pipeline_image" {
type = string
description = "Container image used to execute Terraform runs. Leave blank to use the default image that comes with Terraform Enterprise. Defaults to \"\" if no value is given."
Expand Down
Loading