Skip to content

Conversation

dalekurt
Copy link
Contributor

Description

This PR adds comprehensive input validation for the engine_version variable to prevent users from accidentally using invalid version formats that cause OpenSearch domain destruction and recreation failures.

Key Changes:

  • Added regex validation to engine_version variable in variables.tf
  • Added length validation (≤18 characters) to catch software service versions
  • Enhanced .pre-commit-config.yaml with custom engine version validation hook
  • Provided clear, educational error messages with examples of valid formats

Problem Solved:
Users were mistakenly using software service versions (e.g., OpenSearch_2_19_R20250630-P5) instead of engine versions (e.g., OpenSearch_2.19), causing:

  1. Domain destruction during terraform apply
  2. Recreation failure due to invalid version format
  3. Potential downtime and data loss risks

Motivation and Context

This change is required to prevent a critical issue where users confuse software service versions with engine versions.

Real-world incident:

  • User attempted to upgrade to OpenSearch_2_19_R20250630-P5
  • Terraform destroyed the existing domain (17+ minutes)
  • Recreation failed with validation error: Member must satisfy regular expression pattern: ^Elasticsearch_[0-9]{1}\.[0-9]{1,2}$|^OpenSearch_[0-9]{1,2}\.[0-9]{1,2}$
  • Caused service disruption and potential data loss

Root Cause:

  • No input validation on engine_version variable
  • Users receiving software service version strings from AWS but needing engine version format for Terraform
  • Lack of clear guidance on correct version format

Breaking Changes

No breaking changes. This is purely additive validation that:

  • Maintains backward compatibility with all existing valid configurations
  • Only rejects previously invalid configurations that would have failed at AWS API level
  • Provides better error messages earlier in the workflow (at terraform plan vs terraform apply)

Existing valid configurations like engine_version = "OpenSearch_2.11" continue to work unchanged.

How Has This Been Tested?

Validation Testing

  • Regex Pattern Testing: Validated regex patterns against known valid and invalid formats:

    • ✅ Valid: OpenSearch_1.3, OpenSearch_2.11, OpenSearch_2.15, Elasticsearch_7.10
    • ❌ Invalid: OpenSearch_2_19_R20250630-P5, OpenSearch_2.19.1, 2.19
  • Length Validation Testing: Confirmed 18-character limit catches software service versions:

    • ✅ Valid: OpenSearch_2.11 (14 chars)
    • ❌ Invalid: OpenSearch_2_19_R20250630-P5 (29 chars)
  • Error Message Testing: Verified clear, actionable error messages are displayed

Pre-commit Testing

  • Enhanced pre-commit hooks: Added custom validation that catches invalid versions during development
  • Existing hooks: All existing pre-commit checks continue to pass

Compatibility Testing

  • Null values: Confirmed engine_version = null (default) continues to work
  • Existing examples: Verified no impact on existing example configurations
  • Variable defaults: All default values remain unchanged and valid

Manual Testing Scenarios

# ✅ These should work (and do)
engine_version = "OpenSearch_2.11"
engine_version = "OpenSearch_2.15" 
engine_version = null  # Uses AWS default

# ❌ These should fail fast (and do)
engine_version = "OpenSearch_2_19_R20250630-P5"
engine_version = "OpenSearch_2.19.1"

Testing Commands:

# Test validation
terraform init
terraform plan  # Fails immediately with clear error for invalid versions

# Test pre-commit
pre-commit run -a  # Passes with new validation hooks

Testing Checklist

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
    • Note: No example updates needed as this is validation-only change that doesn't affect valid configurations
  • I have tested and validated these changes using one or more of the provided examples/* projects
    • Tested against existing examples/complete - no impact on valid configurations
  • I have executed pre-commit run -a on my pull request
    • All checks pass, including new engine version validation

Additional Testing:

  • Verified no linting errors introduced
  • Confirmed Terraform syntax is valid
  • Tested both positive and negative validation cases
  • Validated error messages are helpful and actionable

- Add comprehensive input validation for engine_version variable
- Prevent software service versions (e.g., OpenSearch_2_19_R20250630-P5)
- Enforce correct AWS engine version format (OpenSearch_X.Y)
- Add length validation (≤18 chars) to catch overly long versions
- Include clear error messages with examples of valid formats
- Add pre-commit hook for engine version validation
- Prevents accidental domain destruction from invalid version changes

Fixes issue where users mistakenly use software service versions instead
of engine versions, causing domain recreation failures.
- '--args=--only=terraform_standard_module_structure'
- '--args=--only=terraform_workspace_remote'
- id: terraform_validate
- id: terraform_tfsec
Copy link
Member

Choose a reason for hiding this comment

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

the variable changes are ok, but lets revert any changes on the pre-commit config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bryantbiggs Thanks for the review. I have revert the changes to the .pre-commit-config.yaml file.

@bryantbiggs bryantbiggs changed the title feat: add engine_version validation to prevent invalid formats feat: Add variable validation rules to engine_version to prevent invalid formats Aug 26, 2025
@dalekurt dalekurt force-pushed the feat/add-engine-version-validation branch from 7577cba to 3a7ab05 Compare August 27, 2025 16:02
@bryantbiggs bryantbiggs merged commit 8fd929b into terraform-aws-modules:master Aug 27, 2025
8 checks passed
antonbabenko pushed a commit that referenced this pull request Aug 27, 2025
## [2.1.0](v2.0.0...v2.1.0) (2025-08-27)

### Features

* Add variable validation rules to `engine_version` to prevent invalid formats ([#42](#42)) ([8fd929b](8fd929b))
@antonbabenko
Copy link
Member

This PR is included in version 2.1.0 🎉

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants