Skip to content

Conversation

@RoseSecurity
Copy link
Contributor

@RoseSecurity RoseSecurity commented Oct 7, 2025

what

  • Added a new Go test suite in test/component_test.go that uses Cloud Posse's test-helpers to deploy and test Atmos Terraform components, including tests for Datadog monitor components and enabled/disabled flags.
  • Introduced a go.mod file specifying dependencies for the test suite, including test-helpers, testify, and other indirect dependencies required for infrastructure testing.
  • Added detailed Atmos configuration in test/fixtures/atmos.yaml to define base paths, component and stack settings, logging, and template support for tests.
  • Created stack and component fixture files for various test scenarios, such as account maps, Datadog configuration, and Datadog monitor use cases (both enabled and disabled), under test/fixtures/stacks/catalog and test/fixtures/stacks/orgs/default/test.
  • Added a vendor manifest (test/fixtures/vendor.yaml) to specify the sources and versions of vendored Terraform components used in the tests.
  • Updated .gitignore to exclude test state, cache, and other generated files from version control.
  • Removed placeholder files and scripts that are no longer needed, such as the old test/README.md and test/run.sh.

These changes collectively establish a robust and reproducible testing framework for Terraform components using Atmos and Cloud Posse conventions.

why

  • This pull request introduces a test suite for the component, adding Go-based component tests, test fixtures, and supporting configuration files. The main focus is to enable automated testing of Terraform components, particularly those related to Datadog monitors

references

Summary by CodeRabbit

  • Tests

    • Added a comprehensive end-to-end test suite with credential handling, deploy/run workflows, enabled/disabled scenarios, and drift checks; integrated with environment parameter storage and test fixtures.
  • Chores

    • Updated test ignore rules, removed an obsolete test script and placeholder README, and added fixture configurations, a vendor manifest, and module metadata for the test workspace.

Introduce a new Go-based integration test suite for the Datadog monitor
component using cloudposse/test-helpers. Add all necessary fixtures,
stack configs, and vendored dependencies to support local and CI
testing. Remove placeholder files and scripts no longer needed.
@coderabbitai
Copy link

coderabbitai bot commented Oct 7, 2025

Walkthrough

Introduces a test harness and fixtures for Atmos/Terraform: Git ignore updates, Atmos config and vendor manifest, test fixture stacks for Datadog and account-map, a Go-based component test suite using SSM/Terratest, and supporting module/dependency declarations.

Changes

Cohort / File(s) Summary
Git Ignore
test/.gitignore
Adds ignore entries: state/, .cache, test/test-suite.json, .atmos, test_suite.yaml.
Test Suite & Go module
test/component_test.go, test/go.mod
Adds a Go test suite (ComponentSuite) with setup/teardown, Datadog monitor tests (basic, disabled), SSM credential handling, and new Go module/dependencies.
Atmos & Vendor config
test/fixtures/atmos.yaml, test/fixtures/vendor.yaml
Adds Atmos configuration and vendor manifest describing component sources (account-map, datadog-configuration).
Catalog fixtures
test/fixtures/stacks/catalog/account-map.yaml, test/fixtures/stacks/catalog/datadog-configuration.yaml, test/fixtures/stacks/catalog/usecase/basic.yaml, test/fixtures/stacks/catalog/usecase/disabled.yaml
Adds catalog entries for account-map backend, datadog-configuration, and two datadog-monitor variants (basic, disabled).
Org stack defaults & tests manifest
test/fixtures/stacks/orgs/default/test/_defaults.yaml, test/fixtures/stacks/orgs/default/test/tests.yaml
Adds default org stack defaults (account mappings, local backend) and a tests manifest importing the relevant catalog fixtures.
Docs & scripts
test/README.md, test/run.sh
Removes placeholder content from README and deletes obsolete run.sh.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Runner as Test Runner
    participant Env as Env (CI / Local)
    participant SSM as AWS SSM
    participant Atmos as Atmos
    participant TF as Terraform
    participant Terratest as Terratest

    Runner->>Env: SetupSuite (load atmos.yaml, fixtures)
    Runner->>Env: validate DATADOG_API_KEY, DATADOG_APP_KEY
    Runner->>SSM: PutParameter(API_KEY), PutParameter(APP_KEY)
    Runner->>Atmos: Deploy component datadog-monitor/basic
    Atmos->>TF: terraform init/apply
    TF-->>Atmos: apply result
    Atmos-->>Runner: deployment complete
    Runner->>Terratest: run drift checks / assertions
    Terratest-->>Runner: test results
    alt cleanup
        Runner->>SSM: DeleteParameter(API_KEY), DeleteParameter(APP_KEY)
    end
    Runner->>Runner: TearDownSuite
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Suggested labels

no-release

Poem

🐰 I hopped through fixtures, YAML, and tests,

tucked secrets in SSM for careful nests.
Monitors awake, deploys take flight,
Drift checks hum softly into the night.
A tiny rabbit cheers: the tests pass bright!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "test: add datadog monitor tests" directly and accurately reflects the primary objective of this changeset. According to the PR objectives, the purpose is to "establish a reproducible testing framework for the Terraform component using Atmos and Cloud Posse conventions, enabling automated Go-based component tests focused on Datadog monitors." The title concisely captures this intent by identifying both the nature of the change (adding tests) and the specific focus area (datadog monitor). The title uses a standard prefix ("test:") and avoids vague language or unnecessary details, making it clear and specific enough for a developer scanning history to understand the primary contribution without being misleading.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-tests

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49a376b and 3d34bba.

📒 Files selected for processing (1)
  • test/component_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/component_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mergify
Copy link

mergify bot commented Oct 7, 2025

Important

Do not edit the README.md directly. It's auto-generated from the README.yaml

Please update the README.yaml file instead.

Could you fix it @RoseSecurity? 🙏

@mergify mergify bot added the triage Needs triage label Oct 7, 2025
RoseSecurity and others added 4 commits October 8, 2025 16:17
- Add handling for Datadog App key in test suite, storing it in SSM
  alongside the API key to avoid conflicts in parallel tests.
- Refactor struct field names for consistency and clarity.
- Update SSM parameter paths and cleanup logic for both API and App keys.
- Fix stack fixture metadata and monitor config path globbing for
  improved test reliability.
@RoseSecurity
Copy link
Contributor Author

/terratest

1 similar comment
@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

Comment out Datadog API and App key storage in tests.
@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

2 similar comments
@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

@goruha
Copy link
Contributor

goruha commented Oct 17, 2025

/terratest

@RoseSecurity RoseSecurity marked this pull request as ready for review October 17, 2025 17:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
test/fixtures/vendor.yaml (1)

14-16: Simplify path patterns for clarity.

The included_paths: ["**/**"] pattern is redundant (use ["**"] or omit), and excluded_paths: [] can be removed entirely when empty.

       targets:
         - "components/terraform/account-map"
-      included_paths:
-        - "**/**"
-      excluded_paths: []

Apply the same simplification to lines 23-25.

Also applies to: 23-25

test/component_test.go (2)

28-30: Clarify intent behind redundant variable declarations.

Both test methods redeclare awsRegion and generate a new randomID, even though s.awsRegion and s.randomID are available from SetupSuite. If this is for test isolation (unique SSM paths per test), consider documenting that intent. Otherwise, use the suite-level fields consistently.

Also applies to: 55-57


41-42: Use s.awsRegion consistently.

Lines 41-42 and 68-69 use the local awsRegion constant in cleanup defers, while lines 34, 38, 61, and 65 use s.awsRegion. Since both resolve to "us-east-2", this works but creates unnecessary inconsistency. Use s.awsRegion throughout for clarity.

 	defer func() {
-		awsTerratest.DeleteParameter(s.T(), awsRegion, apiKeyPath)
-		awsTerratest.DeleteParameter(s.T(), awsRegion, appKeyPath)
+		awsTerratest.DeleteParameter(s.T(), s.awsRegion, apiKeyPath)
+		awsTerratest.DeleteParameter(s.T(), s.awsRegion, appKeyPath)
 	}()

Also applies to: 68-69

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02c81ad and 49a376b.

⛔ Files ignored due to path filters (1)
  • test/go.sum is excluded by !**/*.sum
📒 Files selected for processing (13)
  • test/.gitignore (1 hunks)
  • test/README.md (0 hunks)
  • test/component_test.go (1 hunks)
  • test/fixtures/atmos.yaml (1 hunks)
  • test/fixtures/stacks/catalog/account-map.yaml (1 hunks)
  • test/fixtures/stacks/catalog/datadog-configuration.yaml (1 hunks)
  • test/fixtures/stacks/catalog/usecase/basic.yaml (1 hunks)
  • test/fixtures/stacks/catalog/usecase/disabled.yaml (1 hunks)
  • test/fixtures/stacks/orgs/default/test/_defaults.yaml (1 hunks)
  • test/fixtures/stacks/orgs/default/test/tests.yaml (1 hunks)
  • test/fixtures/vendor.yaml (1 hunks)
  • test/go.mod (1 hunks)
  • test/run.sh (0 hunks)
💤 Files with no reviewable changes (2)
  • test/run.sh
  • test/README.md
🧰 Additional context used
📓 Path-based instructions (3)
test/fixtures/stacks/catalog/usecase/**

📄 CodeRabbit inference engine (AGENTS.md)

Add test scenarios under test/fixtures/stacks/catalog/usecase/

Files:

  • test/fixtures/stacks/catalog/usecase/basic.yaml
  • test/fixtures/stacks/catalog/usecase/disabled.yaml
**/*.{yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation for YAML and Markdown files

Files:

  • test/fixtures/stacks/catalog/usecase/basic.yaml
  • test/fixtures/stacks/catalog/usecase/disabled.yaml
  • test/fixtures/stacks/catalog/datadog-configuration.yaml
  • test/fixtures/stacks/catalog/account-map.yaml
  • test/fixtures/stacks/orgs/default/test/_defaults.yaml
  • test/fixtures/stacks/orgs/default/test/tests.yaml
  • test/fixtures/vendor.yaml
  • test/fixtures/atmos.yaml
test/**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

test/**/*_test.go: Place Go Terratest files under test/ and name them *_test.go
Use Go Terratest with github.com/cloudposse/test-helpers and Atmos fixtures for integration tests

Files:

  • test/component_test.go
🪛 OSV Scanner (2.2.3)
test/go.mod

[HIGH] 1-1: golang.org/x/crypto 0.33.0: Potential denial of service in golang.org/x/crypto

(GO-2025-3487)


[HIGH] 1-1: golang.org/x/crypto 0.33.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange

(GHSA-hcg3-q754-cr77)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (8)
test/fixtures/stacks/catalog/usecase/basic.yaml (1)

1-17: LGTM!

Fixture is properly structured with 2-space YAML indentation and correctly places the Datadog monitor configuration in the catalog/usecase/ path per guidelines.

test/fixtures/atmos.yaml (1)

1-77: LGTM!

Comprehensive Atmos configuration with correct 2-space indentation throughout. The fixture appropriately enables test-friendly settings (auto-approve, auto-init, template engines) and follows standard stack discovery patterns.

test/fixtures/stacks/orgs/default/test/tests.yaml (1)

1-5: LGTM!

Simple, well-structured import block with correct 2-space indentation. All referenced fixtures exist within the PR.

test/.gitignore (1)

1-5: LGTM!

Standard test artifact ignores appropriate for Atmos/Terraform/Go test workflows.

test/fixtures/stacks/catalog/datadog-configuration.yaml (1)

1-11: LGTM!

Datadog configuration fixture is properly structured with correct 2-space indentation. Configuration values (SSM store, site URL, region) are sensible test defaults.

test/fixtures/stacks/catalog/usecase/disabled.yaml (1)

1-16: LGTM!

Disabled monitor variant is properly structured with correct 2-space indentation. Configuration is consistent with the basic variant, with the enabled flag correctly set to false for testing disabled-component scenarios.

test/fixtures/stacks/catalog/account-map.yaml (1)

1-46: LGTM!

Account-map fixture is appropriately configured for test scenarios with 2-space indentation throughout. Static backend with empty account maps is well-documented as test-specific, and the notes about potential data gaps for certain components are helpful for future maintenance.

test/fixtures/stacks/orgs/default/test/_defaults.yaml (1)

44-44: Clarify the contradictory eks settings.

Line 44 sets eks: true for the account, but line 50 sets tags.eks: false. This appears contradictory—verify whether this is intentional (e.g., account supports EKS but this test scenario doesn't use it).

Also applies to: 50-50

@RoseSecurity
Copy link
Contributor Author

/terratest

@RoseSecurity
Copy link
Contributor Author

I think these should be good for review @goruha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage Needs triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants