diff --git a/.claude/plans/coding-standard-improvements.md b/.claude/plans/coding-standard-improvements.md new file mode 100644 index 0000000..b3ed1dc --- /dev/null +++ b/.claude/plans/coding-standard-improvements.md @@ -0,0 +1,270 @@ +# CODING_STANDARD.md Improvement Plan + +**Purpose**: Enhance `modules/plain-repo/files/CODING_STANDARD.md` to be more comprehensive, better organized, and more actionable. + +**Target File**: `modules/plain-repo/files/CODING_STANDARD.md` + +**Impact**: All Terraform module repositories managed by github-control will receive the updated standard. + +--- + +## Phase 1: Reorganization and Structure + +### 1.1 Add Clear Section Headers +**Current State**: ~~Flat bullet list mixing languages and concerns~~ ✅ COMPLETED +**Goal**: Organize into logical sections with hierarchy + +**Tasks**: +- [x] Create section structure: + - [x] General Formatting (All Files) - added for universal standards + - [x] Python Standards (General, Dependencies) + - [x] Puppet Standards (placeholder for future) + - [x] Terraform General Standards + - [x] Terraform Validation Blocks (existing critical section) + - [x] Terraform IAM Policies + - [x] Terraform Tagging Standards + - [x] Terraform Testing Standards + - [ ] Terraform Variables (to be added in Phase 3) + - [ ] Terraform Outputs (to be added in Phase 3) + - [ ] Terraform Documentation (to be added in Phase 3) +- [x] Move existing bullets into appropriate sections +- [x] Ensure consistent formatting (heading levels, bullet styles) + +**Completed**: Sections 1.1 reorganization is done. Some subsections like Variables, Outputs, and Documentation +will be added when content is available (Phase 3). + +--- + +## Phase 2: Add Context and Rationale + +### 2.1 Document "Why" for Each Guideline +**Current State**: ~~Rules without rationale~~ ✅ COMPLETED +**Goal**: Help developers understand and internalize the standards + +**Tasks**: +- [x] Add rationale for tagging conventions + - [x] Why lowercase except Name? (Case-sensitivity prevents confusion; AWS console convention for Name) + - [x] Why is created_by_module required? (Resource provenance tracking - know what created each resource) + - [x] Why require environment tags from users? (Prevents deployment mistakes, no sensible default) +- [x] Add rationale for version pinning strategies + - [x] Why exact versions for modules? (One change at a time; Renovate manages updates) + - [x] Why ~= for Python deps? (Trust semver; libraries need ranges; auto security fixes) +- [x] Add rationale for provider requirements rule (Separation of concerns) +- [x] Add rationale for IAM data source policy preference (Data sources handle versioning correctly) + +**Format**: Brief 1-sentence explanation in sub-bullets + +**Completed**: All existing guidelines now have clear rationale explaining the "why" behind each rule. + +--- + +## Phase 3: Expand Terraform Coverage + +### 3.1 Add Terraform Naming Conventions +**Current State**: ~~Not documented~~ ✅ COMPLETED +**Goal**: Consistent naming across all modules + +**Tasks**: +- [x] Define variable naming patterns: + - [x] Boolean variables: `enable_*`/`create_*` for verbs, `is_*` for nouns + - [x] Plural vs singular for lists (use plural) + - [x] Underscore conventions (snake_case everywhere) +- [x] Define resource naming patterns (`this`/`main` for single, descriptive names for multiple) +- [x] Define locals naming patterns (descriptive snake_case) +- [x] Define output naming patterns (group with prefixes) +- [x] Provide examples for each + +### 3.2 Add Variable Standards +**Current State**: ~~Only HEREDOC for descriptions mentioned~~ ✅ COMPLETED +**Goal**: Complete variable guidelines + +**Tasks**: +- [x] Guidelines for default values (when required vs optional) +- [x] Validation best practices: + - [x] When to add validation (catch definitely wrong values) + - [x] How to write actionable error messages (explain what's wrong, include fix instructions) + - [x] Common validation patterns (format checks, ranges, no arbitrary constraints) +- [x] Type constraints best practices (always specify explicit types) +- [x] Sensitive variable handling (mark sensitive, use infrahouse/secret/aws module) + +### 3.3 Add Output Standards +**Current State**: ~~Not documented~~ ✅ COMPLETED +**Goal**: Consistent, useful outputs + +**Tasks**: +- [x] When to create outputs (reasonable judgment, under-do vs over-do) +- [x] How to group related outputs (covered in naming conventions) +- [x] When outputs need descriptions (always) +- [x] Sensitive output handling (mark as sensitive = true) + +### 3.4 Add Documentation Standards +**Current State**: ~~Not documented~~ ✅ COMPLETED +**Goal**: Clear guidelines for code documentation + +**Tasks**: +- [x] When inline comments are needed (explain "why" not "what") +- [x] Module-level documentation requirements (README.md with badges, description, examples) +- [x] Examples directory requirements (desired but optional) +- [x] README.md requirements (terraform-docs markers, badges, usage examples) +- [x] Integration with terraform-docs (.terraform-docs.yml managed by github-control) + +### 3.5 Add Resource Organization Patterns +**Current State**: ~~Not documented~~ ✅ COMPLETED +**Goal**: Consistent code organization + +**Tasks**: +- [x] When to use `count` vs `for_each` (count for simple enable/disable, for_each for collections) +- [x] When to extract logic to `locals` (readability, DRY principle) +- [x] How to organize files in a module (main.tf, variables.tf, outputs.tf minimum; split by function) +- [x] Dependencies and ordering best practices (prefer implicit, use depends_on as bugfix) + +**Completed**: Phase 3 is fully complete with all Terraform coverage areas documented. + +### 3.6 Expand Security Standards +**Current State**: ~~Only IAM policy format mentioned~~ ✅ COMPLETED +**Goal**: Comprehensive security guidelines + +**Tasks**: +- [x] Secrets handling (use infrahouse/secret/aws module, never hardcode) +- [x] Least privilege principles (general requirement, ISO/SOC compliance) +- [x] Security group best practices (avoid 0.0.0.0/0, prefer SG refs, ICMP rules) +- [x] Encryption requirements (at rest: enable by default, CMK for CloudTrail; in transit: HTTPS/TLS with policy + hierarchy) +- [x] Logging and auditing standards (365 day retention, CloudTrail, VPC Flow Logs, CloudWatch, AWS Config) + +**Completed**: Comprehensive security section added with ISO27001/SOC2 compliance requirements. + +--- + +## Phase 4: Add Examples and Clarity + +### 4.1 Add Missing Examples +**Current State**: ~~Some guidelines lack concrete examples~~ ✅ COMPLETED +**Goal**: Every guideline has a clear example + +**Tasks**: +- [x] Add HEREDOC example for variable descriptions (lines 36-45) +- [x] Add module pinning example with source and version (lines 55-69) +- [x] Add IAM data source vs JSON example (lines 231-263) +- [x] Add tagging example showing all required tags (lines 282-297) +- [x] Add terraform.tfvars example for testing (lines 301-310) + +**Completed**: All key examples added with clear CORRECT/WRONG comparisons where applicable. + +### 4.2 Expand Existing Examples +**Current State**: ~~Minimal examples~~ ✅ COMPLETED +**Goal**: Complete, realistic examples + +**Tasks**: +- [x] Expand validation block example to show multiple scenarios (Puppet env name, CIDR, range, list, nullable) +- [x] Add more complex validation patterns (regex, format checks, range validation) +- [x] Show complete variable blocks with all elements (required, optional, complex, sensitive, wrong examples) + +**Completed**: Comprehensive examples added showing real-world validation patterns and complete variable structures. + +--- + +## Phase 5: Expand Testing Section + +### 5.1 Add Testing Standards +**Current State**: ~~Only mentions where tests run~~ ✅ COMPLETED +**Goal**: Complete testing guidelines + +**Tasks**: +- [x] Testing philosophy (integration tests, create real infrastructure) +- [x] Testing framework (pytest, pytest-infrahouse, infrahouse-core) +- [x] What should be tested (happy path, edge cases, resource creation, outputs) +- [x] Test coverage requirements (two AWS provider versions) +- [x] Makefile requirements (test-keep, test-clean with configurable parameters) +- [x] Development workflow (local dev, before PR, CI/CD, merge) +- [x] CI/CD requirements (terraform.tfvars, self-hosted runner, 12-hour role session) + +**Completed**: Comprehensive testing standards added covering entire development and CI/CD workflow. + +--- + +## Phase 6: Python Standards Enhancement + +### 6.1 Expand Python Section +**Current State**: ~~Only docstrings and dependencies~~ ✅ COMPLETED +**Goal**: More complete Python standards + +**Tasks**: +- [x] Code formatting (Black for formatting and CI checks) +- [x] Linting standards (pylint with .pylintrc) +- [x] Type hinting requirements (required for all functions) +- [x] Testing standards (pytest with meaningful tests, happy/unhappy paths, no coverage requirements) +- [x] Error handling patterns (never use "except Exception", let it crash) +- [x] Logging standards (use setup_logging() from infrahouse_core.logging) + +**Completed**: Comprehensive Python standards added with formatting, linting, type hints, testing philosophy, error +handling best practices, and logging configuration. + +--- + +## Phase 7: Makefile Standards + +### 7.1 Clarify Makefile Guidelines +**Current State**: ~~"Use Makefile-example" is vague~~ ✅ COMPLETED +**Goal**: Clear, specific Makefile patterns + +**Tasks**: +- [x] Document required targets (help default, bootstrap, install-hooks, test, test-keep, test-clean, clean, format, + lint, release-*) +- [x] Document target naming conventions (lowercase with hyphens) +- [x] Document PHONY targets usage (declare all non-file-producing targets) +- [x] Show examples of common patterns (bootstrap, install-hooks, clean, format, lint, release-patch) +- [x] Document best practices (variables with ?=, target dependencies, tabs not spaces) + +**Completed**: Comprehensive Makefile Standards section added (lines 597-722) with all required targets, naming +conventions, PHONY usage, examples, and best practices. Covers both Python and Terraform module requirements. + +--- + +## Implementation Order Recommendation + +**Priority 1 (High Impact, Low Effort)**: +1. Phase 1: Reorganization - makes everything easier to use +2. Phase 4.1: Add missing examples - clarifies existing guidelines +3. Phase 3.2: Variable standards - frequently used + +**Priority 2 (High Impact, Medium Effort)**: +1. Phase 3.1: Naming conventions - reduces inconsistency +2. Phase 3.3: Output standards - improves module usability +3. Phase 5: Testing standards - prevents bugs + +**Priority 3 (Medium Impact)**: +1. Phase 2: Add rationale - helps with adoption +2. Phase 3.4: Documentation standards +3. Phase 3.5: Resource organization + +**Priority 4 (Nice to Have)**: +1. Phase 6: Python expansion (if needed) +2. Phase 7: Makefile clarification +3. Phase 3.6: Extended security standards + +--- + +## Success Criteria + +- [x] Document is well-organized with clear sections (General Formatting, Python, Terraform with subsections, Puppet, + Makefile) +- [x] Every guideline has a rationale (at least brief) (Phase 2 added rationale throughout) +- [x] Every guideline has an example (Phase 4 added comprehensive examples with CORRECT/WRONG patterns) +- [x] Covers all major Terraform patterns used in InfraHouse modules (Phase 3: naming, variables, outputs, + documentation, organization, security, validation, IAM, tagging, testing) +- [x] New developers can follow it without asking clarifying questions (comprehensive examples and rationale for all + guidelines) +- [x] Claude Code can unambiguously apply all guidelines (clear rules with concrete examples and anti-patterns) +- [x] Document prevents recurring bugs (like the nullable validation issue) (ternary operator rule prominently featured + in Validation Blocks section) + +--- + +## Notes + +- Keep the document concise - prefer clear examples over lengthy prose +- Use consistent formatting throughout +- Consider splitting into multiple files if it gets too long (CODING_STANDARD.md, SECURITY.md, TESTING.md) +- Update `instructions.md` if new critical items are added +- Test changes by having Claude Code review real module code against the new standards diff --git a/modules/plain-repo/files/CODING_STANDARD.md b/modules/plain-repo/files/CODING_STANDARD.md old mode 100755 new mode 100644 index df31805..4ef6c60 --- a/modules/plain-repo/files/CODING_STANDARD.md +++ b/modules/plain-repo/files/CODING_STANDARD.md @@ -5,21 +5,717 @@ If you need to change this file, create a pull request in https://github.com/infrahouse8/github-control --> -* Use RST docstrings in Python +# Coding Standards + +This document defines coding standards for InfraHouse projects. + +## General Formatting (All Files) + +* **Maximum line length: 120 characters** + - Applies to all files: code (Python, Terraform, Puppet), documentation (Markdown, plans), configuration files, etc. + - Break long lines for readability + - Exception: URLs or other content that cannot be split +* **All files must end with a newline character** + +## Python Standards + +### Code Formatting +* **Use Black for code formatting** + - Format code: `black .` + - Check formatting in CI: `black --check .` + - Black is the uncompromising Python code formatter + - Ensures consistent formatting across all projects + +### Linting +* **Use pylint for code linting** + - Enforces code quality standards + - Run: `pylint ` + - Configuration: `.pylintrc` in project root + +### Type Hinting +* **Type hints are required for all functions** + - Include type hints for all function parameters and return values + - Use standard library `typing` module + - Example: + ```python + from typing import List, Dict, Optional + + def process_items(items: List[str], config: Optional[Dict[str, str]] = None) -> bool: + """ + Process a list of items with optional configuration. + + :param items: List of item names to process + :param config: Optional configuration dictionary + :return: True if processing succeeded + """ + # Implementation + return True + ``` + +### Documentation +* **Use RST (reStructuredText) docstrings** for all functions, classes, and modules + - Follow Sphinx docstring format + - Include parameter descriptions, return values, and exceptions + +### Testing +* **Use pytest for all tests** + - Tests must be meaningful and test specific behaviors + - **Cover both happy and unhappy paths** + - No coverage requirements (quality over quantity) + - Don't write tests just to achieve coverage percentage + - Example: + ```python + def test_function_success(): + """Test the happy path - function succeeds with valid input.""" + result = my_function("valid_input") + assert result == expected_output + + def test_function_invalid_input(): + """Test the unhappy path - function fails gracefully with invalid input.""" + with pytest.raises(ValueError) as exc_info: + my_function("invalid_input") + assert "expected error message" in str(exc_info.value) + ``` + +### Error Handling +* **Never use bare `except Exception:`** + - It's better to crash than to mute an error silently + - Only catch specific exceptions you can actually handle + - If you catch an exception, either fix the problem or re-raise it + - Example: + ```python + # WRONG - mutes all errors: + try: + risky_operation() + except Exception: + pass # Don't do this + + # CORRECT - catch specific exceptions: + try: + risky_operation() + except FileNotFoundError as e: + logger.error(f"Required file not found: {e}") + raise # Re-raise if you can't fix the problem + except ValueError as e: + # Fix the problem if you can handle it + return default_value + ``` + +### Logging +* **Use `setup_logging()` from infrahouse-core** + - Provides consistent logging configuration across projects + - Example: + ```python + from infrahouse_core.logging import setup_logging + + LOG = setup_logging(__name__) + + def my_function(): + LOG.info("Starting operation") + LOG.debug("Debug details here") + LOG.error("Error occurred") + ``` + +### Dependencies * Pin Python dependencies to a major version. Use ~= syntax. I.e. `requests ~= 2.31` instead of `requests>=2.31.0,<3.0.0` -* Use Makefile-example as a Makefile example. -* When a variable or output description is too long, use HEREDOC construction to wrap lines. -* The module should only require providers it actually uses to create direct resources. Child modules should take care - of their required providers. -* When including other Terraform modules, always pin them to exact version. -* For IAM policies, use data source policy document, don't use a generated json. - -## Tagging -* use lowercase tags except Name -* use created_by_module -* Create one selected "main" resource with module_version tag -* Use environment tags and require it from a user - -## InfraHouse Terraform testing + - Trusts semantic versioning for patch and minor updates + - Libraries (like infrahouse-core) require version ranges; exact versions cause dependency conflicts + - Applications benefit from automatic security/bug fixes while unit tests provide safety + +### InfraHouse Packages +* **infrahouse-core** (https://pypi.org/project/infrahouse-core/) + - Library with reusable classes and functions + - Use for imports in your Python code + - Examples: AWS helper classes, validation utilities, `setup_logging()` function + - When creating reusable code, add it to infrahouse-core +* **infrahouse-toolkit** (https://pypi.org/project/infrahouse-toolkit/) + - End-user CLI toolkit for provisioning, operational tasks, and CI/CD workflows + - Use as command-line tool (not for imports) + - Depends on infrahouse-core + - Note: Some classes exist in infrahouse-toolkit for historical reasons; reusable classes should migrate to + infrahouse-core +* **pytest-infrahouse** (https://pypi.org/project/pytest-infrahouse/) + - Pytest fixtures for Terraform module testing + - Provides fixtures for AWS infrastructure testing + - Use in test files with pytest + +## Terraform Standards + +### General +* When a variable or output description is too long, use HEREDOC construction to wrap lines + ```hcl + variable "vpc_cidr" { + type = string + description = <<-EOT + The CIDR block for the VPC. Must be a valid IPv4 CIDR block. + Should not overlap with other VPCs or on-premises networks. + Recommended ranges: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 + EOT + } + ``` +* The module should only require providers it actually uses to create direct resources. + Child modules should take care of their required providers + - Separation of concerns: each module knows which providers it needs + - A module shouldn't dictate provider requirements for resources it doesn't directly create +* **InfraHouse modules must use registry.infrahouse.com** (private but publicly available registry) +* **All modules must use exact version pinning** (no ranges) + - Makes only one infrastructure change at a time (your change, not unrelated module updates) + - Version ranges are dangerous: unrelated updates can trigger risky operations + (e.g., adding a secret shouldn't trigger ElasticSearch instance refresh) + - Renovate manages version updates via individual PRs for explicit review and testing + ```hcl + # InfraHouse modules - use registry.infrahouse.com: + module "vpc" { + source = "registry.infrahouse.com/infrahouse/service-network/aws" + version = "3.2.1" # Always exact version + # ... + } + + # Non-InfraHouse modules - also exact version: + module "external" { + source = "terraform-aws-modules/vpc/aws" + version = "5.1.2" # Always exact version + # ... + } + + # WRONG - version range (applies to any module): + module "bad" { + source = "..." + version = "~> 3.2" # Don't do this + # ... + } + ``` + +### Naming Conventions +* **Use snake_case everywhere**: Variables, resources, data sources, locals, outputs + - Use underscores (`_`), not dashes (`-`) + - Use lowercase letters and numbers only +* **Variable naming:** + - Boolean variables with verbs: `enable_*` or `create_*` (e.g., `enable_monitoring`, `create_bucket`) + - Boolean variables with nouns: `is_*` (e.g., `is_public`, `is_encrypted`) + - Lists: use plural form (e.g., `subnet_ids`, `availability_zones`) +* **Resource naming:** + - Single resource of a type: use `this` or `main` (e.g., `aws_nat_gateway.this`) + - Multiple resources of same type: use descriptive names (e.g., `aws_route_table.private`, + `aws_route_table.public`) + - Don't include resource type in name (GOOD: `aws_instance.web`, BAD: `aws_instance.ec2_web`) +* **Locals naming:** + - Use descriptive snake_case names +* **Output naming:** + - Group related outputs with prefixes (e.g., `vpc_id`, `vpc_cidr`, `vpc_arn`) + +### Variables +* **Always specify explicit type constraints** + - Use `type = string`, `type = number`, `type = list(string)`, etc. + - Don't rely on Terraform's type inference +* **Default values - when to use:** + - **Required (no default)**: Critical for compliance/governance (e.g., `alert_emails`), no sensible default exists + (e.g., `environment`, `service`), need to force explicit user decision + - **Optional (with default)**: Meaningful, recommended default exists (e.g., `instance_type = "t3.micro"` for + specific workload) + - **Never default**: Arbitrary/meaningless values (e.g., `environment = "dev"`, `managed_by = "terraform"`) +* **Validation:** + - Goal: Catch definitely wrong input values, not enforce arbitrary constraints + - Add validation for: Format checks (CIDR, email), range checks (percentages 0-100, non-negative values) + - Error messages must: Explain what's wrong with specifics, include instructions on how to fix + - Use appropriate methods: Regex for formats, range checks, contains() for valid sets + - Don't enforce arbitrary lists (like approved instance types) unless there's a strong reason +* **Sensitive variables:** + - Always mark as `sensitive = true` + - Use `infrahouse/secret/aws` module for secrets management + - Specify allowed readers in the `readers` input value + - Never put secret values in user data; pass secret name/ARN instead + +* **Complete variable examples:** + ```hcl + # Example 1: Required variable with validation + variable "environment" { + type = string + description = "Environment name (development, staging, production, etc.)" + + validation { + condition = can(regex("^[a-z0-9_]+$", var.environment)) + error_message = "environment must contain only lowercase letters, numbers, and underscores (no hyphens)" + } + } + + # Example 2: Optional variable with meaningful default + variable "instance_type" { + type = string + description = "EC2 instance type for the application servers" + default = "t3.micro" + } + + # Example 3: Complex variable with all elements + variable "max_instance_count" { + type = number + description = <<-EOT + Maximum number of instances in the Auto Scaling Group. + Must be between 1 and 100. Set to null to use the recommended default based on workload. + EOT + default = null + + validation { + condition = var.max_instance_count == null ? true : (var.max_instance_count >= 1 && var.max_instance_count <= 100) + error_message = "max_instance_count must be between 1 and 100, or null. Got: ${var.max_instance_count}" + } + } + + # Example 4: Sensitive variable (receives secret value from infrahouse/secret/aws module) + variable "database_password" { + type = string + description = "Database password value (from module.my_secret.secret_value)" + sensitive = true + } + + # WRONG - never hardcode secrets: + variable "bad_password" { + type = string + description = "Database password" + default = "MyP@ssw0rd123" # NEVER DO THIS + sensitive = true + } + ``` + +### Outputs +* **When to create outputs:** + - Use reasonable judgment: output values with high chance to be needed by another module + - Better to under-do than over-do - don't output everything "just in case" +* **Always provide descriptions** for all outputs +* **Sensitive outputs:** + - Mark as `sensitive = true` when they contain secrets or sensitive data + +### Documentation +* **Inline comments:** + - Add comments to explain why, not what + - Document non-obvious decisions and reasoning +* **README.md (required):** + - Must include terraform-docs markers: `` and `` + - Pre-commit hook uses terraform-docs to auto-generate documentation + - Must have badges: + - Terraform Registry URL + - License + - CD status + - Relevant documentation (e.g., `[![AWS Lambda](https://img.shields.io/badge/AWS-Lambda-orange?logo=awslambda)] + (https://aws.amazon.com/lambda/)`) + - Module description + - Usage examples +* **examples/ directory:** + - Desired but optional + - Provide working examples when included +* **terraform-docs configuration:** + - `.terraform-docs.yml` is managed by github-control + - README.md uses centrally-managed configuration + +### Resource Organization +* **`count` vs `for_each`:** + - Use `count` for simple create/don't create scenarios (e.g., `count = var.enable_feature ? 1 : 0`) + - Use `for_each` for pre-calculated values (lists, sets, maps) - handles computed values better and avoids + index-shifting issues +* **`locals`:** + - Extract expressions to locals to make code readable and troubleshoot-able + - Follow usual programming best practices (DRY, meaningful names, avoid complex inline expressions) +* **File organization:** + - Always create at minimum: `main.tf`, `variables.tf`, `outputs.tf` + - Create additional files to break module into logical pieces by function (e.g., `iam.tf`, `networking.tf`) + - Don't inflate `main.tf` - split into focused files +* **Dependencies:** + - Prefer implicit dependencies (Terraform references) + - Use explicit `depends_on` only when Terraform struggles with ordering (usually as a bugfix) + - Acceptable to create indirect dependencies (e.g., use module output as tag value or resource name) + +### Security + +#### General Principles +* Follow principle of least privilege +* Comply with ISO27001 and SOC2 requirements +* Use AWS Control Tower for account governance + +#### Secrets Management +* **Always use `infrahouse/secret/aws` module** for secrets storage + - Provides secure access control + - Enables auditing: "who has access to this secret?" + - See: https://infrahouse.com/blog/2024-09-29-compliant-secrets/ +* Never hardcode secret values in code or user data +* Pass secret names/ARNs instead of values + +#### Security Groups +* Avoid `0.0.0.0/0` for ingress rules (except specific cases like public ALBs) +* Prefer security group references over CIDR ranges for internal communication + - Example: Put ALB and ASG in same security group, allow inter-security-group traffic +* **ICMP rules:** + - Within VPC: Allow all ICMP types (for troubleshooting) + - From internet (0.0.0.0/0): Allow only types 3 (Destination Unreachable), 8 (Echo Request), 0 (Echo Reply), + 11 (Time Exceeded). Block all other types + +#### Encryption at Rest +* Enable encryption by default for all resources (S3, EBS, RDS, etc.) +* Use AWS-managed keys (SSE-S3, aws/ebs, etc.) by default +* **Exception - CloudTrail**: Must use Customer-Managed Keys (CMK) +* **S3 buckets**: Use `infrahouse/s3-bucket/aws` module - configures encryption per ISO/SOC requirements + +#### Encryption in Transit +* **Require HTTPS/TLS** for all load balancers and APIs +* Allow HTTP only to redirect to HTTPS +* **TLS Policy for ALB:** + - Default: `ELBSecurityPolicy-TLS13-1-2-Res-2021-06` (Restrictive) + - Allowed: `ELBSecurityPolicy-TLS13-1-2-Ext1-2021-06` (if compatibility required) or more restrictive policies + - Do not allow weaker/older policies + +#### Logging and Auditing +* **Standard log retention**: 365 days (ISO requirement) +* **CloudTrail:** + - Must be enabled via Control Tower + - Must be encrypted with CMK + - Minimum retention: 365 days +* **VPC Flow Logs:** + - Always enable for all VPCs + - Destination: S3 + - Retention: 365 days + - Use `infrahouse/service-network/aws` module for configuration +* **Application Logs:** + - Use CloudWatch Logs + - AWS-managed keys + - Retention: 365 days +* **AWS Config:** + - Required for ISO/SOC compliance + - Use default conformance packs + +### Validation Blocks +* **CRITICAL: Always use ternary operators instead of logical OR when dealing with nullable variables.** + Terraform's OR operator doesn't properly short-circuit null comparisons, causing validation errors. + ```hcl + # WRONG - will fail if var.value is null: + condition = var.value == null || var.value <= 100 + + # CORRECT - use ternary: + condition = var.value == null ? true : var.value <= 100 + ``` + +* **Additional validation patterns:** + ```hcl + # Puppet environment name validation (lowercase letters, numbers, underscores only): + variable "environment" { + type = string + description = "Environment name (must follow Puppet naming rules)" + + validation { + condition = can(regex("^[a-z0-9_]+$", var.environment)) + error_message = "environment must contain only lowercase letters, numbers, and underscores (no hyphens). Got: ${var.environment}" + } + } + + # Format validation (CIDR block): + variable "vpc_cidr" { + type = string + description = "The CIDR block for the VPC" + + validation { + condition = can(cidrhost(var.vpc_cidr, 0)) + error_message = "vpc_cidr must be a valid IPv4 CIDR block (e.g., 10.0.0.0/16)" + } + } + + # Range validation (percentage): + variable "cpu_threshold" { + type = number + description = "CPU threshold percentage for alerting" + default = 80 + + validation { + condition = var.cpu_threshold >= 0 && var.cpu_threshold <= 100 + error_message = "cpu_threshold must be between 0 and 100. Got: ${var.cpu_threshold}" + } + } + + # List validation (minimum length): + variable "availability_zones" { + type = list(string) + description = "List of availability zones" + + validation { + condition = length(var.availability_zones) >= 2 + error_message = "At least 2 availability zones required for HA. Provided: ${length(var.availability_zones)}" + } + } + + # Complex validation (nullable with additional check): + variable "max_size" { + type = number + description = "Maximum instance count" + default = null + + validation { + condition = var.max_size == null ? true : (var.max_size > 0 && var.max_size <= 100) + error_message = "max_size must be between 1 and 100, or null. Got: ${var.max_size}" + } + } + ``` + +### IAM Policies +* For IAM policies, use data source policy document, don't use a generated json + - Data sources handle policy versioning and formatting correctly + - User-generated JSON can fail with wrong version or formatting issues + - Delegates policy generation to Terraform, which does a better job than manual JSON + ```hcl + # CORRECT - use data source: + data "aws_iam_policy_document" "lambda_assume_role" { + statement { + effect = "Allow" + principals { + type = "Service" + identifiers = ["lambda.amazonaws.com"] + } + actions = ["sts:AssumeRole"] + } + } + + resource "aws_iam_role" "lambda" { + name = "lambda-role" + assume_role_policy = data.aws_iam_policy_document.lambda_assume_role.json + } + + # WRONG - generated JSON string: + resource "aws_iam_role" "lambda" { + name = "lambda-role" + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [{ + Effect = "Allow" + Principal = { + Service = "lambda.amazonaws.com" + } + Action = "sts:AssumeRole" + }] + }) + } + ``` + +### Tagging +* Use lowercase tags except `Name` + - AWS tags are case-sensitive (`environment` ≠ `Environment`), so lowercase prevents confusion and accidental duplicates + - `Name` is capitalized to follow AWS console convention (displayed in resource tables) + - Note: AWS-generated tags (prefixed with `aws:`) are managed by AWS and ignored by this standard +* Use underscores for multi-word tags (e.g., `created_by_module`, not `created-by-module`) +* **Resource provenance tags** - Track what created each resource: + - `created_by`: GitHub repository that deployed the resource (format: `org/repo_name`) + Example: `"infrahouse/aws-control-289256138624"` + - `created_by_module`: Terraform module where resource is defined (format: `org/module_name/provider`) + Example: `"infrahouse/jumphost/aws"` + - `created_by_fixture`: Pytest fixture that created test resources (format: `org/package/fixture`) + Example: `"infrahouse/pytest-infrahouse/postgres"` (only present on test resources) +* Create one selected "main" resource with `module_version` tag +* **Require environment tag from user** - Do not provide defaults + - Prevents nonsense like `environment=dev` in production AWS accounts + - Forces explicit environment declaration, bringing order and preventing deployment mistakes + ```hcl + # Example of proper tagging on a resource: + resource "aws_instance" "web" { + ami = "ami-12345678" + instance_type = "t3.micro" + + tags = { + Name = "web-server" # Capitalized per AWS convention + environment = var.environment # Required from user + service = var.service # Required from user + created_by = "infrahouse/aws-control-289256138624" + created_by_module = "infrahouse/ec2-instance/aws" + module_version = var.module_version # On selected "main" resource only + } + } + ``` + +### Testing + +#### Testing Philosophy +* Tests are called "unit tests" but are actually **integration tests** + - Create real infrastructure in AWS + - Validate the infrastructure behaves correctly + - Clean up resources after testing + +#### Testing Framework +* Use **pytest** for all Terraform module tests +* Use Terraform fixtures from **pytest-infrahouse** (https://pypi.org/project/pytest-infrahouse/) +* Use **infrahouse-core** (https://pypi.org/project/infrahouse-core/) for validation + - Provides AWS and infrastructure helper classes + +#### Test Coverage Requirements +* Must test **two AWS provider versions** at all times (currently versions 5 and 6) +* Test both happy path and edge cases +* Validate resource creation, outputs, and behavior + +#### Makefile Requirements +* Implement **test-keep** and **test-clean** targets +* Support configurable test parameters: + ```makefile + TEST_REGION ?= us-west-2 + TEST_ROLE ?= arn:aws:iam::303467602807:role/ecs-tester + TEST_SELECTOR ?= tests/ + TEST_PATH ?= tests/test_httpd.py + TEST_FILTER ?= "test_ and aws-6" + ``` +* `test-keep`: Run tests and keep infrastructure for debugging +* `test-clean`: Run tests and clean up all resources (use before PRs) + +#### Development Workflow +1. **Local development**: Run tests with `test-keep` during feature development +2. **Before PR**: Run `make test-clean` for final validation +3. **Create PR**: GitHub Actions runs full test suite +4. **Merge**: When all tests pass + +#### CI/CD Requirements * The root module must define variables in terraform.tfvars -* .github/workflows/terraform-CD.yml must run on a self-hosted runner. GitHub provided runners don't have ih-registry command. + ```hcl + # Example terraform.tfvars for testing: + environment = "development" + service = "my-service" + + vpc_cidr = "10.0.0.0/16" + availability_zones = ["us-west-2a", "us-west-2b"] + instance_type = "t3.micro" + enable_monitoring = true + ``` +* **Test root module must set `created_by` tag** in provider default_tags: + ```hcl + provider "aws" { + default_tags { + tags = { + created_by = "infrahouse/terraform-aws-ecs" # GitHub repository that created the resource + } + } + } + ``` +* GitHub Actions must run on self-hosted runner (GitHub runners don't have ih-registry command) +* Test role session time in CI: **12 hours** +* .github/workflows/terraform-CD.yml configures CI test execution + +## Puppet Standards + +### General +* TBD - Standards to be documented + +## Makefile Standards + +All InfraHouse projects (Python libraries, Terraform modules, etc.) must include a Makefile following these standards. + +### Help Target (Default) +* **`make help` must be the default target** + - Running `make` without arguments should display help + - Help output should list all user-facing targets with descriptions + - Implementation: Use `.DEFAULT_GOAL := help` or list `help` as first target + +### Required Targets + +#### `bootstrap` +* Installs all project dependencies +* Assumes it's running in a virtualenv +* Installs pip, setuptools, and dependencies from requirements.txt (or equivalent) +* Must indirectly call the `install-hooks` target +* Example: + ```makefile + bootstrap: install-hooks + pip install --upgrade pip setuptools + pip install -r requirements.txt + ``` + +#### `install-hooks` +* Installs pre-commit hooks and commit-msg hook +* The `hooks/commit-msg` file is managed by github-control +* Example: + ```makefile + install-hooks: + pre-commit install + pre-commit install --hook-type commit-msg + ``` + +#### `test` +* Runs the full test suite +* Should run all tests with default configuration +* For Terraform modules: runs tests against all supported AWS provider versions + +#### `test-keep` +* Runs tests and keeps infrastructure for debugging +* Useful during local development +* Resources remain after tests complete for troubleshooting + +#### `test-clean` +* Runs tests and cleans up all resources +* Must be run before creating PRs to ensure proper cleanup +* Validates that infrastructure can be cleanly destroyed + +#### `clean` +* Cleans all build artifacts, temporary files, caches, etc. +* **Do NOT remove Claude Code reviews** (preserve .claude/ directory contents) +* Common cleanups: + - Python: `__pycache__`, `*.pyc`, `.pytest_cache`, `dist/`, `build/`, `*.egg-info` + - Terraform: `.terraform/`, `terraform.tfstate*`, `.terraform.lock.hcl` (in test directories) +* Example: + ```makefile + clean: + find . -type d -name __pycache__ -exec rm -rf {} + + find . -type f -name '*.pyc' -delete + rm -rf .pytest_cache dist build *.egg-info + ``` + +#### `format` +* Reformats code to match coding standards +* **For Terraform modules**: Use both `black .` and `terraform fmt -recursive` +* **For Python projects**: Use `black .` +* Should be safe to run repeatedly (idempotent) +* Example: + ```makefile + format: + black . + terraform fmt -recursive + ``` + +#### `lint` +* Runs project-specific linters and formatting tools in check mode +* Should NOT modify files, only report issues +* Exit with non-zero status if issues found (for CI/CD) +* **For Python**: Check with `black --check .` and `pylint` +* **For Terraform**: Use `terraform fmt -check -recursive` and `terraform validate` +* Example: + ```makefile + lint: + black --check . + pylint my_module + terraform fmt -check -recursive + ``` + +#### `release-*` targets +* Automate the release process +* Run git-cliff for changelog generation +* Run bumpversion to update version numbers +* Common targets: + - `release-patch`: Patch version bump (e.g., 1.2.3 → 1.2.4) + - `release-minor`: Minor version bump (e.g., 1.2.3 → 1.3.0) + - `release-major`: Major version bump (e.g., 1.2.3 → 2.0.0) +* Example: + ```makefile + release-patch: + git-cliff --tag $(shell bumpversion --dry-run --list patch | grep new_version | cut -d= -f2) -o CHANGELOG.md + bumpversion patch + git push && git push --tags + ``` + +### Target Naming Conventions +* Use lowercase with hyphens for multi-word targets (e.g., `test-clean`, `install-hooks`) +* Use descriptive names that clearly indicate the action +* Group related targets with common prefixes (e.g., `release-*`, `test-*`) + +### PHONY Targets +* Declare all targets that don't produce files as `.PHONY` +* Prevents conflicts with files of the same name +* Improves performance by skipping timestamp checks +* Example: + ```makefile + .PHONY: help bootstrap install-hooks test test-keep test-clean clean format lint + ``` + +### Best Practices +* Use `?=` for variables that users can override (e.g., `TEST_REGION ?= us-west-2`) +* Document complex targets with comments +* Keep targets focused - one clear purpose per target +* Chain related targets using dependencies (e.g., `bootstrap: install-hooks`) +* Use consistent indentation (tabs, not spaces, per Make syntax requirements) diff --git a/modules/plain-repo/files/Makefile-example b/modules/plain-repo/files/Makefile-example old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/cliff.toml.tpl b/modules/plain-repo/files/cliff.toml.tpl old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/commit-msg b/modules/plain-repo/files/commit-msg old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/instructions.md b/modules/plain-repo/files/instructions.md new file mode 100644 index 0000000..f862fa1 --- /dev/null +++ b/modules/plain-repo/files/instructions.md @@ -0,0 +1,14 @@ +# Project Instructions for Claude Code + +## Terraform Coding Standards + +**ALWAYS read and follow `.claude/CODING_STANDARD.md` before writing or modifying any Terraform code.** + +This file contains critical requirements including: +- Proper use of ternary operators vs logical OR in validation blocks with nullable variables +- Tagging conventions +- Provider requirements +- Module pinning +- IAM policy patterns + +Pay special attention to validation blocks - they require ternary operators to avoid null comparison errors. \ No newline at end of file diff --git a/modules/plain-repo/files/pre-commit b/modules/plain-repo/files/pre-commit old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/renovate.json b/modules/plain-repo/files/renovate.json old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/terraform-module-reviewer.md b/modules/plain-repo/files/terraform-module-reviewer.md old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/vuln-scanner-pr-private.yml b/modules/plain-repo/files/vuln-scanner-pr-private.yml old mode 100755 new mode 100644 diff --git a/modules/plain-repo/files/vuln-scanner-pr-public.yml b/modules/plain-repo/files/vuln-scanner-pr-public.yml old mode 100755 new mode 100644 diff --git a/modules/plain-repo/repos-files.tf b/modules/plain-repo/repos-files.tf index abff423..63373ee 100755 --- a/modules/plain-repo/repos-files.tf +++ b/modules/plain-repo/repos-files.tf @@ -26,7 +26,6 @@ resource "github_repository_file" "terraform_module_reviewer" { } resource "github_repository_file" "coding_standard" { - count = var.repo_type == "terraform_module" ? 1 : 0 depends_on = [ github_repository_ruleset.main ] @@ -37,6 +36,17 @@ resource "github_repository_file" "coding_standard" { overwrite_on_create = true } +resource "github_repository_file" "claude_instructions" { + depends_on = [ + github_repository_ruleset.main + ] + repository = github_repository.repo.name + file = "./.claude/instructions.md" + content = file("${path.module}/files/instructions.md") + commit_message = "Add Claude Code instructions" + overwrite_on_create = true +} + resource "github_repository_file" "makefile_example" { count = var.repo_type == "terraform_module" ? 1 : 0 depends_on = [