-
Notifications
You must be signed in to change notification settings - Fork 104
Release Tests for AWS Redis passwordless authentication #380
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.
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
base: main
Are you sure you want to change the base?
Conversation
- Add redis_enable_iam_auth variable for IAM authentication control - Create ElastiCache IAM user and user group for passwordless access - Configure replication group to use IAM authentication when enabled - Add IAM policy for ElastiCache Connect permissions in service accounts - Pass IAM authentication parameters to runtime container engine config - Enable passwordless Redis authentication using AWS IAM roles This allows TFE to connect to ElastiCache Redis using IAM authentication instead of passwords, improving security and enabling passwordless workflows.
…D-5861 branch for Redis passwordless authentication
- Add redis_passwordless_aws_host_name to runtime configuration - Extract Redis cluster name from local.redis.hostname for IAM auth - Support Redis passwordless authentication with host name for ElastiCache
- Create aws_elasticache_user.default_user explicitly for user group requirement - AWS ElastiCache requires 'default' user to exist in every user group - Update user_group references to use created default_user instead of string 'default' - Add proper dependencies for user creation order - Fix ElastiCache user group creation error for IAM authentication Resolves: DefaultUserRequired: Redis user group needs to contain a user with the user name default
- Set default user access_string to +@ALL for full Redis command access - Ensure default user has same level of access as IAM user - Add clarifying comments about AWS requirement for default user - This ensures the user group functions properly with both users
- Change authentication mode from 'no-password' to 'no-password-required' - AWS provider expects 'no-password-required' as valid type - This resolves the validation error in terraform plan Error was: expected type to be one of ["password" "no-password-required" "iam"], got no-password
- Remove aws_elasticache_user.default_user resource - AWS ElastiCache has a built-in 'default' user that already exists - Reference the built-in default user directly in user_ids: ['default', iam_user_id] - Remove default_user dependencies since it's not created by Terraform - This resolves: UserAlreadyExists: User default already exists The user group now correctly includes both required users: 1. Built-in AWS 'default' user (referenced by string) 2. Our custom IAM user (created by Terraform)
- AWS ElastiCache requires encryption-in-transit for user group based access control - Update transit_encryption_enabled to enable when IAM auth is used - Formula: transit_encryption_enabled = var.redis_encryption_in_transit || local.redis_use_iam_auth - This ensures IAM authentication works while preserving user's encryption choice Resolves: User group based access control requires encryption-in-transit to be enabled on the replication group
- Add Redis module debug outputs for IAM user creation - Add main module debug outputs for Redis configuration chain - Track username propagation from Redis module to TFE container - Debug Redis environment variables in terraform-random-tfe-utility
- Only set CA certificate paths when mTLS is explicitly enabled - For Redis IAM authentication, ElastiCache uses AWS managed certificates - Setting ca_cert_path to null allows TLS without requiring custom CA certs - Resolves startup check failure: open /etc/ssl/private/terraform-enterprise/redis/cacert.pem: no such file or directory
- Remove postgres-passwordless module reference that was causing test failures - Redis passwordless test should ONLY configure Redis IAM auth, not PostgreSQL - Fixed module count logic to not trigger postgres_passwordless module - Removed postgres_enable_iam_auth variables that were incorrectly added - This resolves the 'modules/postgres-passwordless: no such file' error
- Update all terraform-random-tfe-utility module references to pravi/IND-5861 - Ensures Redis passwordless variables are available across all modules
- Add redis_passwordless_aws_use_iam variable to variables.tf - Pass redis_passwordless_aws_use_iam to runtime_container_engine_config module - Enables AWS IAM authentication for Redis passwordless access via ElastiCache
…m in debug outputs - Fix undefined variable reference in debug outputs causing Terraform plan failure - Use correct variable name redis_passwordless_aws_use_iam instead of redis_enable_iam_auth - Resolves 'Reference to undeclared input variable' error in release test
- Fix corrupted file structure in tests/standalone-vault/main.tf where copyright header was replaced with module code - Restore proper copyright header: '# Copyright (c) HashiCorp, Inc.' and '# SPDX-License-Identifier: MPL-2.0' - Update hcp_vault module reference to use pravi/IND-5861 branch instead of main - Apply terraform fmt to fix alignment and formatting issues in main.tf, modules/redis/main.tf, and outputs.tf - Resolves 'Argument or block definition required' terraform fmt error
- Change redis_passwordless_aws_use_iam to redis_passwordless_aws_use_instance_profile - Update all references in main.tf, variables.tf, and outputs.tf - Variable name now matches TFE_REDIS_PASSWORDLESS_AWS_USE_INSTANCE_PROFILE This aligns with the official TFE configuration reference documentation and resolves PR review feedback about incorrect configuration variables.
- Add redis_passwordless_aws_region variable to support TFE_REDIS_PASSWORDLESS_AWS_REGION - Add redis_passwordless_aws_host_name mapping using existing redis hostname - Update main.tf to pass AWS region and hostname to runtime configuration - Fix terraform formatting alignment for CI compliance - Complete Redis passwordless IAM authentication implementation per TFE docs
- Add redis_enable_iam_auth parameter to Redis module call - Fix Redis module username output to return IAM username when IAM auth enabled - This resolves 'redis user must be set when using AWS instance profile' error - Redis IAM user is already created but wasn't being passed to TFE configuration
Critical fix: The redis_enable_iam_auth parameter was not being passed to the service_accounts module, which prevented the elasticache:Connect IAM policy from being created. This policy is required for TFE instances to connect to Redis using IAM authentication with the created IAM user.
The TFE_REDIS_USE_AUTH environment variable must be true when using either password auth OR IAM auth. Previously it was only set to true for password auth, causing 'WRONGPASS invalid username-password pair' errors for IAM authentication. Now redis_use_auth = password_auth OR iam_auth, which correctly enables authentication for both modes.
…auth TFE validates that passwordless authentication and AUTH cannot be enabled simultaneously. When using IAM authentication: - TFE_REDIS_USE_AUTH should be FALSE (not true) - TFE_REDIS_PASSWORDLESS_AWS_USE_INSTANCE_PROFILE should be TRUE - Authentication happens via IAM user and instance profile This is similar to PostgreSQL passwordless auth where we disable traditional authentication when using IAM.
The ElastiCache user group for IAM authentication should only contain IAM users, not the built-in 'default' user which is for password-based authentication. Having both can cause authentication conflicts. This should resolve the WRONGPASS error for IAM authentication.
Fixes duplicate variable definition in Redis module variables.tf
- Update aws_security_group_redis output to use var.tfe_instance_sg - Remove reference to deleted aws_security_group.redis resource - This fixes 'Reference to undeclared resource' error in terraform plan ERROR WAS: aws_security_group.redis[0].id referenced but resource was removed FIX: Use var.tfe_instance_sg (the shared TFE security group) instead
- Add 'default' username to elasticache:Username condition - Allow both AWS-managed default user and custom IAM user - Fixes WRONGPASS error when using custom IAM user authentication
- Add iam_user output from Redis module - Pass redis_passwordless_aws_iam_user parameter to runtime config - Use custom IAM user instead of 'default' for proper AWS IAM testing - Add redis_passwordless_aws_iam_user variable to runtime config This ensures we test the actual custom ElastiCache IAM user per AWS documentation instead of using the 'default' user which bypasses true IAM authentication validation.
Root cause: IAM policy was using Resource='*' with StringEquals condition, but AWS documentation requires explicit resource ARNs for ElastiCache users. Changes: - Update IAM policy to use explicit ARNs for replication group and users - Add required variables for building resource ARNs - Add data sources for region and account ID - Pass Redis module outputs to service_accounts module This follows the AWS documentation pattern: https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/auth-iam.html
Use var.redis_passwordless_aws_use_instance_profile condition instead of local.redis.enabled which doesn't exist
- Remove debug outputs from main outputs.tf - Remove unnecessary EC2 cost reduction comments - Remove unused variables from Redis module (network_id, network_private_subnet_cidrs) - Update Redis module call to remove unused parameters - Remove debug test script files - Run terraform fmt for consistent formatting This cleanup addresses lint warnings and follows Terraform best practices.
|
would also request to run the existing AWS redis release test to make sure nothing is breaking since we are making common file changes @raviharshicorp |
| resource "aws_elasticache_subnet_group" "tfe" { | ||
| count = var.active_active ? 1 : 0 | ||
| name = "${var.friendly_name_prefix}-tfe-redis" | ||
| subnet_ids = var.network_subnets_private |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens here if we use existing security groups
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always creates a new subnet group, even when using existing security groups. This could cause conflicts or resource duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it works with the existing configuration we should continue to do so. wydt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't worked with existing configuration.
When using existing security groups, EC2 instances get the existing security group.
But Redis always creates its own subnet group regardless of security group configuration.
This lead to Redis being deployed in different subnets than the EC2 instances.
This cause connectivity issues between EC2 and Redis, breaking IAM authentication
Ran the tests again: https://github.com/hashicorp/terraform-enterprise/actions/runs/19529721862/job/56043488911 |
modules/redis/main.tf
Outdated
| auth_token = var.redis_encryption_in_transit && local.redis_use_password_auth ? random_id.redis_password[0].hex : null | ||
|
|
||
| # Transit encryption is required when using user groups (IAM authentication) | ||
| transit_encryption_enabled = var.redis_encryption_in_transit || local.redis_use_iam_auth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encryption should not depend on redis_use_iam_auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajmera-naman, agreed. Encryption and IAM authentication are independent concerns and shouldn't be coupled together. Removed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces AWS Redis passwordless authentication using IAM for Terraform Enterprise deployments. The implementation adds support for authenticating to ElastiCache Redis using AWS IAM credentials instead of passwords, which improves security by leveraging instance profiles and eliminates the need to manage Redis passwords.
Key Changes:
- Added IAM authentication support for Redis ElastiCache with new variables (
redis_passwordless_aws_use_instance_profile,redis_passwordless_aws_region) - Simplified Redis security architecture by reusing TFE instance security group instead of creating separate Redis security groups
- Added IAM policy for ElastiCache Connect permissions with proper resource scoping
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| variables.tf | Added variables for Redis IAM authentication and existing security group support |
| tests/standalone-vault/main.tf | Updated module reference to feature branch (needs update to main before merge) |
| modules/vm/variables.tf | Added variable for optional existing security group ID |
| modules/vm/outputs.tf | Changed output to use local security group ID (existing or new) |
| modules/vm/main.tf | Implemented conditional security group creation and rule management |
| modules/service_accounts/variables.tf | Added variables for Redis IAM authentication configuration |
| modules/service_accounts/main.tf | Added IAM policy and role attachment for ElastiCache Connect permissions |
| modules/redis/variables.tf | Removed unused network variables, added redis_enable_iam_auth variable |
| modules/redis/outputs.tf | Added cluster_id and iam_user outputs, updated descriptions |
| modules/redis/main.tf | Implemented ElastiCache User/UserGroup for IAM auth, removed separate security group creation |
| modules/database/outputs.tf | Marked password output as sensitive (security improvement) |
| main.tf | Integrated Redis IAM auth configuration, updated module references to feature branch |
| fixtures/test_proxy/main.tf | Updated module reference to feature branch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| redis_enable_iam_auth = var.redis_passwordless_aws_use_instance_profile | ||
| redis_replication_group_id = var.redis_passwordless_aws_use_instance_profile ? local.redis.cluster_id : null | ||
| redis_iam_user_name = var.redis_passwordless_aws_use_instance_profile ? local.redis.iam_user : null |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Circular dependency detected: module.service_accounts (lines 48-50) references local.redis.cluster_id and local.redis.iam_user, which come from module.redis outputs. However, module.redis (line 88) depends on module.vm.tfe_instance_sg, and module.vm is declared after both modules. This creates a dependency cycle: service_accounts → redis → vm. Consider restructuring to break this cycle, perhaps by making the IAM policy attachment conditional or by passing the security group ID differently.
| variable "redis_passwordless_aws_use_instance_profile" { | ||
| default = false | ||
| type = bool | ||
| description = "Whether or not to use AWS IAM authentication to connect to Redis. Defaults to false if no value is given." | ||
| } |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add validation to ensure redis_passwordless_aws_use_instance_profile and redis_use_password_auth are mutually exclusive, as both cannot be enabled simultaneously. Consider adding:
validation {
condition = !(var.redis_passwordless_aws_use_instance_profile && var.redis_use_password_auth)
error_message = "redis_passwordless_aws_use_instance_profile and redis_use_password_auth cannot both be true. Choose either IAM authentication or password authentication."
}| variable "existing_vm_security_group_id" { | ||
| type = string | ||
| default = null | ||
| description = "(Optional) Existing security group ID to use for TFE instances. If provided, no new security group will be created. Required for Redis IAM authentication when using existing infrastructure." |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The description states this is "Required for Redis IAM authentication when using existing infrastructure" but the variable is marked as "(Optional)". Consider clarifying the description to indicate when this is required vs optional, e.g., "(Optional, but required when using Redis IAM authentication with existing infrastructure)".
| description = "(Optional) Existing security group ID to use for TFE instances. If provided, no new security group will be created. Required for Redis IAM authentication when using existing infrastructure." | |
| description = "(Optional, but required when using Redis IAM authentication with existing infrastructure) Existing security group ID to use for TFE instances. If provided, no new security group will be created." |
| redis_replication_group_id = var.redis_passwordless_aws_use_instance_profile ? local.redis.cluster_id : null | ||
| redis_iam_user_name = var.redis_passwordless_aws_use_instance_profile ? local.redis.iam_user : null |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential runtime error: Lines 49-50 reference local.redis.cluster_id and local.redis.iam_user, but these fields don't exist in local.redis_default (see locals.tf). If redis_passwordless_aws_use_instance_profile is enabled without active-active mode, the module will fail. Add validation to ensure redis_passwordless_aws_use_instance_profile can only be true when operational_mode == "active-active", or add cluster_id and iam_user fields to redis_default in locals.tf.
| # ------------------------------------------------------------------------------------ | ||
| module "runtime_container_engine_config" { | ||
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/runtime_container_engine_config?ref=main" | ||
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/runtime_container_engine_config?ref=pravi/IND-5861" |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module source references a feature branch (pravi/IND-5861) instead of main. According to the PR description, this should be updated to main before merging.
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/runtime_container_engine_config?ref=pravi/IND-5861" | |
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/runtime_container_engine_config?ref=main" |
| } | ||
|
|
||
| variable "redis_authentication_mode" { | ||
| description = "The authentincation mode for redis server instances. Must be one of [USER_AND_PASSWORD, PASSWORD, NONE]." |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in description: "authentincation" should be "authentication".
| redis_encryption_at_rest = var.redis_encryption_at_rest | ||
| redis_use_password_auth = var.redis_use_password_auth | ||
| redis_enable_iam_auth = var.redis_passwordless_aws_use_instance_profile | ||
| redis_port = var.redis_encryption_in_transit ? "6380" : "6379" |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redis_authentication_mode variable is not being passed to the redis module, but it's used in modules/redis/main.tf line 6 to determine password authentication. Add redis_authentication_mode = var.redis_authentication_mode to the module inputs.
| redis_port = var.redis_encryption_in_transit ? "6380" : "6379" | |
| redis_port = var.redis_encryption_in_transit ? "6380" : "6379" | |
| redis_authentication_mode = var.redis_authentication_mode |
| variable "redis_passwordless_aws_region" { | ||
| default = null | ||
| type = string | ||
| description = "AWS Region of the AWS ElastiCache resource for Redis passwordless authentication." | ||
| } |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The redis_passwordless_aws_region variable may be unnecessary. The AWS region can be obtained from the existing data.aws_region.current data source (already used in main.tf line 311). Consider removing this variable or documenting why a separate region configuration is needed for Redis IAM auth.
| variable "redis_passwordless_aws_region" { | |
| default = null | |
| type = string | |
| description = "AWS Region of the AWS ElastiCache resource for Redis passwordless authentication." | |
| } |
|
|
||
| module "hcp_vault" { | ||
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//fixtures/test_hcp_vault?ref=main" | ||
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//fixtures/test_hcp_vault?ref=pravi/IND-5861" |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module source references a feature branch (pravi/IND-5861) instead of main. According to the PR description, this should be updated to main before merging.
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//fixtures/test_hcp_vault?ref=pravi/IND-5861" | |
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//fixtures/test_hcp_vault?ref=main" |
| # -------------------------------------------------------------------------------------------- | ||
| module "settings" { | ||
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/settings?ref=main" | ||
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/settings?ref=pravi/IND-5861" |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module source references a feature branch (pravi/IND-5861) instead of main. According to the PR description, this should be updated to main before merging.
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/settings?ref=pravi/IND-5861" | |
| source = "git::https://github.com/hashicorp/terraform-random-tfe-utility//modules/settings?ref=main" |
| enable_sentinel_mtls = var.enable_sentinel_mtls | ||
| redis_ca_cert_path = var.enable_redis_mtls || var.enable_sentinel_mtls ? "/etc/ssl/private/terraform-enterprise/redis/cacert.pem" : null | ||
| redis_client_cert_path = var.enable_redis_mtls || var.enable_sentinel_mtls ? "/etc/ssl/private/terraform-enterprise/redis/cert.pem" : null | ||
| redis_client_key_path = var.enable_redis_mtls || var.enable_sentinel_mtls ? "/etc/ssl/private/terraform-enterprise/redis/key.pem" : null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems there is another issue here, if this was the case then the existing tests would have failed and same way of configuration is also used for database, I request to pls have a look at this issue
Background
Release Tests for AWS Redis passwordless authentication.
Infra related variables are defined here.
Will updated the reference to main before merging this PR.
Related PR terraform-enterprise # https://github.com/hashicorp/terraform-enterprise/pull/3149
How Has This Been Tested
CI/CD: https://github.com/hashicorp/terraform-enterprise/actions/runs/19529721862/job/55909640280
Screenshots