Skip to content
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

Added tests #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added tests #11

wants to merge 1 commit into from

Conversation

goruha
Copy link
Contributor

@goruha goruha commented Dec 30, 2024

what

  • Added tests

Summary by CodeRabbit

  • Chores

    • Updated module versions in remote state configuration
    • Added new configuration files for testing infrastructure
    • Updated Go module dependencies
    • Refined Atmos CLI configuration for testing components
  • Tests

    • Introduced comprehensive test suite for AWS resource deployment
    • Added test configurations for VPC, DNS, and Aurora MySQL cluster
    • Created new test component and helper functions
  • Documentation

    • Added vendor configuration for managing Terraform components
  • Maintenance

    • Updated .gitignore files to exclude cache and test-related files
    • Removed obsolete test script

@goruha goruha requested a review from a team as a code owner December 30, 2024 14:08
Copy link

coderabbitai bot commented Dec 30, 2024

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 golangci-lint (1.62.2)

level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for "github.com/cloudposse/test-helpers/pkg/atmos""
level=error msg="Running error: can't run linter goanalysis_metalinter\nbuildir: failed to load package : could not load export data: no export data for "github.com/cloudposse/test-helpers/pkg/atmos""

Walkthrough

This pull request introduces comprehensive changes to a testing and infrastructure configuration framework. The modifications span multiple files across the project, focusing on enhancing testing capabilities for AWS resources, updating module versions, and establishing a robust configuration structure for Atmos CLI. Key changes include adding new test configurations, updating remote state module versions, introducing a Go-based testing framework, and creating detailed YAML configurations for infrastructure components.

Changes

File Change Summary
.gitignore Added .cache to ignored files
src/remote-state.tf Updated module versions from 1.5.0 to 1.8.0 for multiple modules
test/.gitignore Added entries for state/, .cache, test-suite.json, and atmos
test/component_test.go New test file with TestComponent and GetNatsByVpcIdE functions
test/fixtures/* Multiple new YAML configuration files for Atmos, stacks, and infrastructure components
test/go.mod New Go module with dependencies for testing
test/run.sh Removed

Sequence Diagram

sequenceDiagram
    participant Test as Component Test
    participant Atmos as Atmos CLI
    participant AWS as AWS Resources
    participant Fixture as Test Fixtures

    Test->>Atmos: Initialize configuration
    Atmos->>Fixture: Load stack configurations
    Atmos->>AWS: Deploy infrastructure
    AWS-->>Atmos: Confirm deployment
    Test->>AWS: Validate resources
    AWS-->>Test: Return resource details
    Test->>Atmos: Teardown infrastructure
Loading

Poem

🐰 Hopping through configs with glee,
Testing AWS, setting resources free!
Modules upgraded, fixtures in line,
Infrastructure dancing, simply divine!
CodeRabbit's magic, testing with might! 🚀


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

mergify bot commented Dec 30, 2024

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟠 Require terratest

Waiting checks: test/terratest.

This rule require terratest status

  • check-success = test/terratest

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: 5

🧹 Nitpick comments (10)
test/component_test.go (1)

142-154: **Improve function naming consistency and error context **

The helper function GetNatsByVpcIdE works fine. However, the name can be updated to clearly convey it returns NAT gateways rather than NAT IDs, and consider wrapping errors to add more context if something fails in the API call.

-func GetNatsByVpcIdE(t *testing.T, vpcId string, awsRegion string) ([]*ec2.NatGateway, error) {
+func GetNatGatewaysByVpcIdE(t *testing.T, vpcId string, awsRegion string) ([]*ec2.NatGateway, error) {
test/fixtures/stacks/orgs/default/test/regional.yaml (1)

8-8: **Remove trailing spaces **

Line 8 has trailing whitespace that can trigger linting issues. Consider removing it to keep the file clean.

-#  - catalog/usecase/with_flow_logs  
+#  - catalog/usecase/with_flow_logs
🧰 Tools
🪛 yamllint (1.35.1)

[error] 8-8: trailing spaces

(trailing-spaces)

test/fixtures/stacks/catalog/vpc.yaml (1)

16-19: Consider test-specific documentation

The configuration includes test-specific settings like custom subnet tagging and CIDR blocks. Consider adding comments explaining the test-specific requirements and constraints for these values.

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

33-46: Document empty configuration objects

The static backend includes several empty configurations (account_info_map, all_accounts, etc.). Consider adding comments explaining:

  • What data is expected in each field during tests
  • Which components require specific data to be added
  • Example values for testing different scenarios
test/fixtures/vendor.yaml (2)

57-66: Document local source path requirements

The aurora-mysql component uses a relative path ../../src. Consider:

  • Adding a comment explaining the expected repository structure
  • Documenting any setup requirements for running tests

1-66: Consider defining excluded paths

While excluded_paths is consistently empty across all components, consider:

  • Documenting why no exclusions are needed
  • Adding common test exclusions (e.g., **/*_test.tf, **/examples/**)
test/fixtures/atmos.yaml (1)

79-87: Test command implementation could be improved

The test-components command uses a complex jq filter. While functional, it could be more maintainable.

Consider:

  1. Breaking down the jq filter into multiple steps with comments
  2. Adding error handling for when jq fails
  3. Adding a --help description of the expected output format

Example refactor:

       - >
         atmos describe stacks --format json --sections=component,metadata --components=component -s sandbox
-        | jq '.[] | .components.terraform | to_entries | 
-        map(select(.value.component == "component" and (.value.metadata.type != "abstract" or .value.metadata.type == null))) 
-        | .[].key' 
+        | jq '
+          # Get terraform components
+          .[] | .components.terraform | to_entries
+          # Filter non-abstract components
+          | map(
+              select(
+                .value.component == "component" 
+                and (.value.metadata.type != "abstract" or .value.metadata.type == null)
+              )
+            )
+          # Extract component keys
+          | .[].key
+        '
🧰 Tools
🪛 yamllint (1.35.1)

[error] 85-85: trailing spaces

(trailing-spaces)


[error] 86-86: trailing spaces

(trailing-spaces)


[error] 87-87: trailing spaces

(trailing-spaces)

test/go.mod (2)

17-17: Remove commented code

The commented replace directive for aws-nuke should be removed if it's not needed.

-// replace github.com/rebuy-de/aws-nuke/v2/mocks => github.com/ekristen/aws-nuke/v3/ v3.29.0

6-13: Consider version constraints for testing dependencies

The terratest and test-helpers versions should be pinned to ensure consistent test behavior across different environments.

Consider adding version constraints in your go.mod:

 require (
-	github.com/cloudposse/test-helpers v0.12.1-0.20241230101202-621e95c4e9aa
+	github.com/cloudposse/test-helpers v0.12.1
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
-	github.com/gruntwork-io/terratest v0.47.1
+	github.com/gruntwork-io/terratest v0.47.1 // pin to exact version
 	github.com/mattn/go-zglob v0.0.3 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/stretchr/testify v1.9.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
test/fixtures/stacks/orgs/default/test/global.yaml (1)

4-4: Remove trailing spaces

There are trailing spaces at the end of line 4.

-  - catalog/dns-delegated  
+  - catalog/dns-delegated
🧰 Tools
🪛 yamllint (1.35.1)

[error] 4-4: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad6ff2d and 4299480.

⛔ Files ignored due to path filters (1)
  • test/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • .gitignore (1 hunks)
  • src/remote-state.tf (5 hunks)
  • test/.gitignore (1 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/dns-delegated.yaml (1 hunks)
  • test/fixtures/stacks/catalog/dns-primary.yaml (1 hunks)
  • test/fixtures/stacks/catalog/usecase/cluster.yaml (1 hunks)
  • test/fixtures/stacks/catalog/vpc.yaml (1 hunks)
  • test/fixtures/stacks/orgs/default/test/_defaults.yaml (1 hunks)
  • test/fixtures/stacks/orgs/default/test/global.yaml (1 hunks)
  • test/fixtures/stacks/orgs/default/test/regional.yaml (1 hunks)
  • test/fixtures/vendor.yaml (1 hunks)
  • test/go.mod (1 hunks)
  • test/run.sh (0 hunks)
💤 Files with no reviewable changes (1)
  • test/run.sh
✅ Files skipped from review due to trivial changes (3)
  • .gitignore
  • test/.gitignore
  • src/remote-state.tf
🧰 Additional context used
🪛 yamllint (1.35.1)
test/fixtures/stacks/orgs/default/test/regional.yaml

[error] 8-8: trailing spaces

(trailing-spaces)

test/fixtures/atmos.yaml

[error] 85-85: trailing spaces

(trailing-spaces)


[error] 86-86: trailing spaces

(trailing-spaces)


[error] 87-87: trailing spaces

(trailing-spaces)

test/fixtures/stacks/orgs/default/test/global.yaml

[error] 4-4: trailing spaces

(trailing-spaces)

🔇 Additional comments (15)
test/component_test.go (2)

42-66: **Validate correctness of checking for an empty cluster ARN **

Line 66 checks whether the aurora_mysql_cluster_arn output is an empty string, implying the cluster was not created. If the intent is to verify a successful creation, consider updating your assertion to match the expected non-empty ARN or confirm the correctness of this negative test scenario.


14-20: **Initialize testing fixture earlier or clarify the test lifecycle **

Currently, you call fixture.TearDown() (line 19) before fixture.SetUp() (line 20). Even though the defer for teardown ensures it runs at the end of the test, placing it physically before the setup logic can be confusing to maintainers. Consider rearranging the order to enhance readability and ensure no hidden lifecycle issues.

-	defer fixture.TearDown()
-	fixture.SetUp(&atmos.Options{})
+	fixture.SetUp(&atmos.Options{})
+	defer fixture.TearDown()
test/fixtures/stacks/catalog/dns-primary.yaml (1)

5-5: **Empty list for domain names **

domain_names is initialized to an empty list. Ensure that this is intentional and that downstream code gracefully handles the absence of domain names.

test/fixtures/stacks/catalog/dns-delegated.yaml (1)

5-8: **Flag default values for variables **

The default values for name, zone_config, request_acm_certificate, and dns_private_zone_enabled look coherent and are a good starting point.

test/fixtures/stacks/catalog/vpc.yaml (2)

11-15: LGTM: Secure network configuration for testing

The configuration correctly disables internet access from private subnets by setting:

  • nat_gateway_enabled: false
  • max_nats: 0
  • nat_instance_enabled: false

This is appropriate for testing isolation.


8-10: Verify availability zone configuration

The configuration only uses AZs 'b' and 'c', skipping 'a'. While this might be intentional, consider if this matches your testing requirements across all AWS regions, as some regions might have different AZ naming or availability.

✅ Verification successful

AZ configuration is appropriate for testing environment

The current availability zone configuration appears to be intentionally minimal for testing purposes, aligning with other test-specific configurations in the file:

  • The VPC is clearly a test environment (indicated by test-specific subnet tag key eg.cptest.co/subnet/type)
  • Other settings are deliberately minimal:
    • NAT and internet access disabled
    • VPC flow logs disabled
    • Limited max subnet count (3)
    • Test-specific CIDR block

Using only two AZs (b and c) is sufficient for testing scenarios and helps minimize costs while still providing multi-AZ coverage. The configuration is consistent with the minimal, test-focused nature of this VPC setup.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check AZ availability in the test region
aws ec2 describe-availability-zones --query 'AvailabilityZones[].ZoneName' --output text

Length of output: 184


Script:

#!/bin/bash
# Let's check if there are any patterns or configurations related to availability zones in the codebase
rg -l "availability_zones?" --type yaml

# Check the content of files that mention availability zones
rg "availability_zones?" -A 5 -B 5 --type yaml

Length of output: 834


Script:

#!/bin/bash
# Let's check the entire VPC configuration file to understand the context better
cat test/fixtures/stacks/catalog/vpc.yaml

# Also search for any test configurations or documentation that might explain the AZ choices
rg -g "!{node_modules,dist,build}/*" -i "test.*vpc|vpc.*test" --type md --type txt

Length of output: 667

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

23-32: LGTM: Well-documented test configuration

The static backend configuration is well-documented, clearly explaining:

  • Its purpose for tests
  • Required environment variables
  • Limitations and requirements
test/fixtures/stacks/catalog/usecase/cluster.yaml (2)

20-20: Security considerations for test configuration

The configuration has potential security implications:

  1. publicly_accessible: true exposes the database to public access
  2. Empty credentials will be randomly generated

While this is acceptable for tests, it should be documented that these are test-specific settings.

Consider adding a comment block at the top of the file explaining these test-specific security considerations and how they differ from production settings.

Also applies to: 38-40


22-33: Excellent documentation of AWS CLI commands

The comments providing AWS CLI commands for finding engine versions and parameter group families are very helpful for testing and configuration.

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

45-55: Test account configuration uses placeholder values

The account configuration appropriately uses environment variables for test account IDs:

id: '{{ getenv "TEST_ACCOUNT_ID" | default "<TEST_ACCOUNT_ID>" }}'

The placeholder <TEST_ACCOUNT_ID> makes it clear when the environment variable is not set.


7-9: Verify test state isolation

The state configuration uses environment variables for test state directory:

path: '{{ getenv "TEST_STATE_DIR" | default "../../../../../state" }}/{{ getenv "TEST_SUITE_NAME" | default "" }}/{{ .component }}/terraform.tfstate'

This ensures test state isolation, but the default relative path (../../../../../state) might be fragile.

Consider:

  1. Using an absolute path as the default
  2. Adding validation for TEST_STATE_DIR to ensure it exists

Also applies to: 12-14

test/fixtures/atmos.yaml (2)

1-10: Well-documented configuration loading behavior

Excellent documentation of the configuration loading order and glob pattern support.


25-32: Test-specific Terraform settings look good

The Terraform configuration appropriately enables:

  • Auto-approve for tests
  • Automatic initialization
  • Backend file generation

These settings are suitable for test automation.

test/fixtures/stacks/orgs/default/test/global.yaml (2)

6-8: LGTM: Environment variable configuration

The environment variable configuration looks good and follows the expected structure.


1-4: Verify imported configurations exist

Please ensure all imported configurations are present in the repository:

  • orgs/default/test/_defaults
  • catalog/dns-primary
  • catalog/dns-delegated
✅ Verification successful

All imported configurations are present

All the imported configurations referenced in global.yaml exist in the repository:

  • orgs/default/test/_defaults
  • catalog/dns-primary
  • catalog/dns-delegated
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if imported configurations exist
echo "Checking for imported configurations..."
for config in "orgs/default/test/_defaults" "catalog/dns-primary" "catalog/dns-delegated"; do
  if fd -t f "${config}.ya?ml" test/fixtures/stacks/; then
    echo "✓ Found: $config"
  else
    echo "✗ Missing: $config"
  fi
done

Length of output: 775

🧰 Tools
🪛 yamllint (1.35.1)

[error] 4-4: trailing spaces

(trailing-spaces)

Comment on lines +11 to +22
# This remote state is only for Cloud Posse internal use.
# It references the Cloud Posse test organizations actual infrastructure.
# remote_state_backend:
# s3:
# bucket: cptest-core-ue2-root-tfstate-core
# dynamodb_table: cptest-core-ue2-root-tfstate-core-lock
# role_arn: arn:aws:iam::822777368227:role/cptest-core-gbl-root-tfstate-core-ro
# encrypt: true
# key: terraform.tfstate
# acl: bucket-owner-full-control
# region: us-east-2

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove sensitive information from comments

The commented section contains actual infrastructure references including:

  • S3 bucket names
  • DynamoDB table names
  • IAM role ARNs

Consider removing or replacing with placeholder values in test fixtures.

Comment on lines +8 to +18
- component: "account-map"
source: github.com/cloudposse/terraform-aws-components.git//modules/account-map?ref={{.Version}}
version: 1.520.0
targets:
- "components/terraform/account-map"
included_paths:
- "**/*.tf"
- "**/*.md"
- "**/*.tftmpl"
- "**/modules/**"
excluded_paths: []
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Version inconsistency confirmed - account-map using older version

The verification confirms that there is indeed a version inconsistency in the vendor.yaml file:

  • account-map component uses version 1.520.0
  • All other components (dns-primary, dns-delegated, vpc) use version v1.534.0

Additionally, the version format is inconsistent:

  • account-map uses 1.520.0 (without 'v' prefix)
  • Others use v1.534.0 (with 'v' prefix)

Recommendations:

  • Update account-map component to version v1.534.0 to maintain consistency
  • Add the 'v' prefix to maintain consistent version formatting
  • If a different version is required for account-map, add a comment explaining why
🔗 Analysis chain

Version inconsistency in account-map component

The account-map component uses version 1.520.0 while other components use v1.534.0. Consider:

  • Updating to match other components if possible
  • Documenting why a different version is needed if intentional
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for breaking changes between versions
gh api repos/cloudposse/terraform-aws-components/releases \
  --jq '.[] | select(.tag_name | test("v1.52[0-9].0|v1.53[0-4].0")) | {tag: .tag_name, body: .body}' \
  | grep -i "breaking"

Length of output: 177


Script:

#!/bin/bash
# Let's check all component versions in vendor.yaml to verify the version inconsistency
cat test/fixtures/vendor.yaml | grep -A1 "source: github.com/cloudposse/terraform-aws-components"

# Also check if there are any specific version constraints or requirements in the repository
fd -t f "README.md|CHANGELOG.md" --exec grep -l "terraform-aws-components" {} \; -x cat {} | grep -i "version\|terraform-aws-components"

Length of output: 3006


Script:

#!/bin/bash
# Let's check all components in vendor.yaml to get a complete picture of versions
cat test/fixtures/vendor.yaml | grep -B5 -A2 "version:"

# Also check if there are any version-related comments or documentation in the vendor.yaml file
cat test/fixtures/vendor.yaml | grep -i "#.*version"

Length of output: 1148

Comment on lines +8 to +9
mysql_deletion_protection: false
mysql_storage_encrypted: true
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the search results, I can now provide a final response:

Document test-specific override of deletion protection

The mysql_deletion_protection setting in the test configuration explicitly overrides the default value (true) defined in the module's documentation. This is a significant security setting that should be clearly documented to prevent misuse.

  • Add a comment in test/fixtures/stacks/catalog/usecase/cluster.yaml to document this test-specific override:
        # Explicitly disabled for test fixtures only. Default is true for all other environments.
        mysql_deletion_protection: false
🔗 Analysis chain

Verify test configuration for deletion protection

Setting mysql_deletion_protection: false is appropriate for test environments but should be documented as a test-only setting to prevent confusion with production configurations.

Consider adding a comment to explicitly indicate this is a test-specific setting:

+        # Disabled for test environments only. Production should have this enabled.
         mysql_deletion_protection: false

Let me verify if there are similar test configurations or documentation that can provide context about the deletion protection setting.


Based on the search results, I found important context in the README.yaml that shows mysql_deletion_protection is actually meant to be true by default, with the description "Set to true to protect the database from deletion". Let me search for more context about test configurations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for similar test configurations and documentation
rg -g '*.{yaml,yml}' -i "mysql_deletion_protection" -B 2 -A 2

# Look for any documentation about test configurations
fd README.md | xargs rg -i "test|deletion protection"

# Search for any comments or documentation about deletion protection in yaml files
rg -g '*.{yaml,yml}' -i "deletion protection" -B 2 -A 2

Length of output: 5764


Script:

#!/bin/bash
# Look for test configuration documentation or patterns
rg -g '*.{yaml,yml,md}' -i "test.*configuration|test.*environment" -B 2 -A 2

# Check if there are other test fixtures with similar patterns
fd . test/fixtures -t f | grep -i "cluster.yaml"

Length of output: 177

@@ -0,0 +1,91 @@
module test

go 1.23.0
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Invalid Go version specified

Go version 1.23.0 doesn't exist yet (latest stable version is 1.22.0). This will prevent the module from being built.

-go 1.23.0
+go 1.22.0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
go 1.23.0
go 1.22.0

gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace github.com/cloudposse/test-helpers => /Users/goruha/projects/cloudposse/current/cloudposse-terraform-components/test-helpers
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace directive uses local path

The replace directive contains a local filesystem path which won't work for other developers or in CI/CD environments.

Either remove the replace directive if it's temporary or update it to use a proper repository reference:

-replace github.com/cloudposse/test-helpers => /Users/goruha/projects/cloudposse/current/cloudposse-terraform-components/test-helpers
+replace github.com/cloudposse/test-helpers => github.com/cloudposse/test-helpers v0.12.1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
replace github.com/cloudposse/test-helpers => /Users/goruha/projects/cloudposse/current/cloudposse-terraform-components/test-helpers
replace github.com/cloudposse/test-helpers => github.com/cloudposse/test-helpers v0.12.1

Comment on lines +68 to +137
// output "aurora_mysql_cluster_id" {
// output "aurora_mysql_cluster_name" {
// output "aurora_mysql_endpoint" {
// output "aurora_mysql_master_hostname" {
// output "aurora_mysql_master_password" {
// output "aurora_mysql_master_password_ssm_key" {
// output "aurora_mysql_master_username" {
// output "aurora_mysql_reader_endpoint" {
// output "aurora_mysql_replicas_hostname" {
// output "cluster_domain" {
// output "kms_key_arn" {

})

// suite.Test(t, "public-private-subnets", func(t *testing.T, atm *helper.Atmos) {
// inputs := map[string]interface{}{
// "name": "vpc-terraform",
// "availability_zones": []string{"b", "c"},
// "public_subnets_enabled": true,
// "nat_gateway_enabled": true,
// "nat_instance_enabled": false,
// "subnet_type_tag_key": "eg.cptest.co/subnet/type",
// "max_nats": 1,
// "max_subnet_count": 3,
// "vpc_flow_logs_enabled": false,
// "ipv4_primary_cidr_block": "172.16.0.0/16",
// }

// defer atm.GetAndDestroy("vpc/public", "default-test", inputs)
// component := atm.GetAndDeploy("vpc/public", "default-test", inputs)

// vpcId := atm.Output(component, "vpc_id")
// require.True(t, strings.HasPrefix(vpcId, "vpc-"))

// vpc := aws.GetVpcById(t, vpcId, awsRegion)

// assert.Equal(t, vpc.Name, fmt.Sprintf("eg-default-ue2-test-vpc-terraform-%s", component.RandomIdentifier))
// assert.Equal(t, *vpc.CidrAssociations[0], "172.16.0.0/16")
// assert.Equal(t, *vpc.CidrBlock, "172.16.0.0/16")
// assert.Nil(t, vpc.Ipv6CidrAssociations)
// assert.Equal(t, vpc.Tags["Environment"], "ue2")
// assert.Equal(t, vpc.Tags["Namespace"], "eg")
// assert.Equal(t, vpc.Tags["Stage"], "test")
// assert.Equal(t, vpc.Tags["Tenant"], "default")

// subnets := vpc.Subnets
// require.Equal(t, 4, len(subnets))

// public_subnet_ids := atm.OutputList(component, "public_subnet_ids")
// assert.Equal(t, 2, len(public_subnet_ids))

// public_subnet_cidrs := atm.OutputList(component, "public_subnet_cidrs")
// assert.Equal(t, 2, len(public_subnet_cidrs))

// private_subnet_ids := atm.OutputList(component, "private_subnet_ids")
// assert.Equal(t, 2, len(private_subnet_ids))

// private_subnet_cidrs := atm.OutputList(component, "private_subnet_cidrs")
// assert.Equal(t, 2, len(private_subnet_cidrs))

// assert.False(t, aws.IsPublicSubnet(t, private_subnet_ids[0], awsRegion))
// assert.False(t, aws.IsPublicSubnet(t, private_subnet_ids[1], awsRegion))

// assert.True(t, aws.IsPublicSubnet(t, public_subnet_ids[0], awsRegion))
// assert.True(t, aws.IsPublicSubnet(t, public_subnet_ids[1], awsRegion))

// nats, err := GetNatsByVpcIdE(t, vpcId, awsRegion)
// assert.NoError(t, err)
// assert.Equal(t, 1, len(nats))
// })
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we commenting all this out?

Copy link

mergify bot commented Jan 29, 2025

Heads up! This pull request looks stale. It will be closed soon, if there are no new commits. ⏳

@mergify mergify bot added stale This PR has gone stale and removed stale This PR has gone stale labels Jan 29, 2025
Copy link

mergify bot commented Feb 5, 2025

Heads up! This pull request looks stale. It will be closed soon, if there are no new commits. ⏳

@mergify mergify bot added stale This PR has gone stale and removed stale This PR has gone stale labels Feb 5, 2025
Copy link

mergify bot commented Feb 12, 2025

Heads up! This pull request looks stale. It will be closed soon, if there are no new commits. ⏳

@mergify mergify bot added stale This PR has gone stale and removed stale This PR has gone stale labels Feb 12, 2025
@goruha goruha self-assigned this Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants