Add component tests #19
Conversation
WalkthroughThis pull request updates Terraform configurations and expands testing and configuration files. In the Terraform provider configuration, the intermediate local variable for the role ARN has been removed, and its logic is now applied directly. The eks module version has been upgraded. Additionally, multiple new YAML configuration files have been added in test fixtures for CLI, VPC, EKS cluster, use cases, organization defaults, and vendor settings. A new Go module file for tests has been introduced while a test script file has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant TF as Terraform Config
participant Var as kube_exec_auth_role_arn
participant IAM as module.iam_roles.terraform_role_arn
TF->>Var: Check for role ARN
alt Role ARN is provided
Var-->>TF: Return value
else Role ARN is not provided
TF->>IAM: Fallback to IAM role ARN
IAM-->>TF: Return IAM role ARN
end
sequenceDiagram
participant Runner as TestRunSuite
participant Suite as ComponentSuite
participant Helper as helper.Run
Runner->>Suite: Initialize ComponentSuite
Suite->>Suite: Run SetupSuite() (configure test env)
Runner->>Suite: Execute TestBasic() & TestEnabledFlag()
Suite->>Helper: Run test suite with dependencies ("vpc", "eks/cluster")
Helper-->>Runner: Return test results
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Important Description is necessary and should not be empty.Kindly provide details with what was changed, why it was changed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require terratestWonderful, this rule succeeded.This rule require terratest status
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
test/fixtures/stacks/catalog/usecase/basic.yaml (1)
1-15: EKS Reloader Basic Configuration YAML
This configuration for the "eks/reloader/basic" component is well-structured, specifying essential metadata and variable settings for deploying the reloader. However, YAMLlint has flagged that there is no newline at the end of this file (line 15). Please add a newline at the end for compliance with YAML best practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
test/fixtures/stacks/catalog/usecase/disabled.yaml (1)
1-15: YAML Structure and Trailing NewlineThe configuration is structured clearly to disable the EKS reloader, and the mapping for metadata and variables is self-explanatory. Please add a newline at the end of the file (after line 15) to satisfy YAML linting rules.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
test/fixtures/vendor.yaml (1)
1-43: Comprehensive Vendor ConfigurationThe AtmosVendorConfig manifests are well-defined. All sources, targets, and included/excluded path patterns are clearly specified. As a minor nitpick, please verify that the version strings and source paths (which appear as GitHub repository references) follow your project's standard conventions and include a trailing newline if needed.
test/fixtures/stacks/catalog/account-map.yaml (1)
1-47: Account-Map YAML ClarityThe account-map configuration is clear and well-organized. The metadata, variables, and the static remote state backend structure are appropriately defined. The commented-out section provides useful context for internal use and legacy configuration. Please ensure a trailing newline at the end of the file for uniformity.
test/fixtures/stacks/orgs/default/test/_defaults.yaml (1)
1-67: Robust Default ConfigurationThis defaults file delivers a comprehensive setup for Terraform backend configuration and variable management. The templated expressions using
getenvand the structured definitions of label orders and descriptor formats are well done. Once again, ensure that the file terminates with a newline to align with YAML best practices.src/provider-helm.tf (1)
136-138: Streamlinedexec_roleAssignmentDirectly embedding the
coalescefunction within theexec_rolelist improves clarity by eliminating an unnecessary intermediate local variable. Confirm that all consumers of the provider have been updated to work with this inline expression.test/component_test.go (1)
21-21: Remove the unused constantawsRegion.The variable
awsRegionis declared but never used in your test code. Consider removing it to improve code cleanliness.- const awsRegion = "us-east-2"🧰 Tools
🪛 golangci-lint (1.64.8)
21-21: const
awsRegionis unused(unused)
test/go.mod (1)
17-17: Uncomment or remove the replace directive.The replace directive for aws-nuke is commented out. Either uncomment it if it's needed or remove it entirely to maintain a clean configuration file.
-// replace github.com/rebuy-de/aws-nuke/v2/mocks => github.com/ekristen/aws-nuke/v3/ v3.29.0 +replace github.com/rebuy-de/aws-nuke/v2/mocks => github.com/ekristen/aws-nuke/v3/ v3.29.0or
-// replace github.com/rebuy-de/aws-nuke/v2/mocks => github.com/ekristen/aws-nuke/v3/ v3.29.0test/fixtures/stacks/catalog/eks-cluster.yaml (2)
94-94: Add a newline at the end of the file.The file is missing a newline character at the end. This is a common best practice and is flagged by linters.
aws-efs-csi-driver: addon_version: "v2.0.8-eksbuild.1" +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 94-94: no new line character at the end of file
(new-line-at-end-of-file)
82-94: Consider adding descriptions for EKS add-ons.The EKS add-ons section lists version numbers without any comments explaining their purpose or why specific versions were chosen. Adding brief comments about each add-on would improve code documentation.
addons: vpc-cni: + # Amazon VPC CNI plugin for pod networking addon_version: "v1.18.3-eksbuild.3" kube-proxy: + # Maintains network rules on nodes addon_version: "v1.30.3-eksbuild.5" coredns: + # DNS service for Kubernetes addon_version: "v1.11.3-eksbuild.1" configuration_values: '{"autoScaling":{"enabled":true,"minReplicas":3}}' aws-ebs-csi-driver: + # EBS CSI driver for persistent volumes addon_version: "v1.34.0-eksbuild.1" configuration_values: '{"sidecars":{"snapshotter":{"forceEnable":false}}}' aws-efs-csi-driver: + # EFS CSI driver for persistent volumes addon_version: "v2.0.8-eksbuild.1"🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 94-94: no new line character at the end of file
(new-line-at-end-of-file)
test/fixtures/atmos.yaml (1)
18-18: Consider setting a base_path for better standardization.The
base_pathis currently set to an empty string. Setting a consistent base path could help standardize paths across different environments and make the configuration more portable.-base_path: "" +base_path: "test/fixtures"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
src/provider-helm.tf(1 hunks)src/remote-state.tf(1 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/eks-cluster.yaml(1 hunks)test/fixtures/stacks/catalog/usecase/basic.yaml(1 hunks)test/fixtures/stacks/catalog/usecase/disabled.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/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 (1)
- test/run.sh
🧰 Additional context used
🪛 YAMLlint (1.35.1)
test/fixtures/stacks/catalog/usecase/basic.yaml
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
test/fixtures/stacks/catalog/usecase/disabled.yaml
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
test/fixtures/stacks/catalog/eks-cluster.yaml
[error] 94-94: no new line character at the end of file
(new-line-at-end-of-file)
🪛 golangci-lint (1.64.8)
test/component_test.go
21-21: const awsRegion is unused
(unused)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (15)
src/remote-state.tf (1)
3-3: Module Version Upgrade to 1.8.0
The module version in the remote-state configuration has been updated from "1.5.0" to "1.8.0". Please verify that this upgrade is compatible with the rest of your Terraform configuration and that any potential breaking changes or new features have been accounted for.test/.gitignore (1)
1-5: New .gitignore for Test Directory
The .gitignore file in the test directory now correctly excludes directories and files such asstate/,.cache,test/test-suite.json,.atmos, andtest_suite.yaml. This will help prevent cluttering the repository with temporary or test-related artifacts.test/fixtures/stacks/orgs/default/test/tests.yaml (1)
1-7: Test Imports Configuration
This YAML file lists the necessary import paths for test configurations, including defaults and catalog components (vpc, eks-cluster, basic, and disabled use cases). Ensure that each specified path exists and that the imported configurations are properly maintained.test/fixtures/stacks/catalog/vpc.yaml (1)
1-20: VPC Configuration YAML
The new VPC configuration is clear and comprehensive; it defines metadata and variables such as availability zones, NAT gateway settings, and CIDR blocks. One point to verify: the comment on line 13 states that "Private subnets do not need internet access," which should be consistent with the enabled NAT gateway (line 14). Please double-check that this comment correctly describes the intended configuration.src/provider-helm.tf (3)
164-173: Dynamic Exec Block in Helm ProviderThe dynamic block that configures the
execparameters for the Helm provider is well implemented—it conditionally includes the exec configuration only whenkube_exec_auth_enabledis true and certificate data is available. This approach enhances flexibility.
190-198: Consistent Dynamic Exec Block in Kubernetes ProviderSimilarly, the dynamic block for the Kubernetes provider mirrors the Helm provider configuration, ensuring consistency across providers. This uniform structure aids maintainability and reduces confusion.
75-80:❓ Verification inconclusive
Deprecation Notice for
kube_exec_auth_role_arnVariableWhile the new inline
coalesce(var.kube_exec_auth_role_arn, module.iam_roles.terraform_role_arn)logic in theexec_roleassignment simplifies the code, the public variablekube_exec_auth_role_arnis still declared. If this variable is no longer needed externally, please consider deprecating or removing it and updating the module’s documentation accordingly.
#!/bin/bash # Verify usage of the variable 'kube_exec_auth_role_arn' across the repository. # This command will search for all occurrences of the variable name to see if external dependencies exist. rg "kube_exec_auth_role_arn" -n .Deprecation Suggestion for
kube_exec_auth_role_arnVariableThe new inline logic using
coalesce(var.kube_exec_auth_role_arn, module.iam_roles.terraform_role_arn)in theexec_roleassignment helps simplify the configuration, yet the public variablekube_exec_auth_role_arnremains declared insrc/provider-helm.tf(lines 75–80).
Before removing or deprecating this variable, please verify whether any external dependencies rely on it. If it is indeed unused externally or redundant due to the inline logic, consider deprecating or removing the variable and updating the module’s documentation accordingly.test/component_test.go (4)
18-52: Your implementation of TestBasic looks good.The test properly validates critical metadata of the "eks/reloader/basic" component including app version, chart name, timestamps, and namespace. The test also includes cleanup with
defer s.DestroyAtmosComponentand performs a drift test to ensure the deployed component matches the expected state.🧰 Tools
🪛 golangci-lint (1.64.8)
21-21: const
awsRegionis unused(unused)
54-58: Good implementation of TestEnabledFlag.The test properly verifies the enabled flag for the "eks/reloader/disabled" component using the helper method
VerifyEnabledFlag.
60-64: Well-structured SetupSuite method.The SetupSuite method correctly initializes the test configuration and sets the component destination directory path.
66-71: TestRunSuite properly sets up test dependencies.The function correctly creates a new ComponentSuite instance and adds necessary dependencies for "vpc" and "eks/cluster" before running the suite.
test/fixtures/stacks/catalog/eks-cluster.yaml (2)
52-58: Consider enabling Fargate profiles for Karpenter.The Fargate profiles configuration is commented out but might be necessary for Karpenter functionality. If this is intentional, consider adding a comment explaining why it's disabled or if it should be enabled in certain scenarios.
Is Karpenter functionality important for your test cases? If so, you may need to uncomment and properly configure the Fargate profiles section.
1-94: Well-structured EKS cluster configuration.The configuration file provides a comprehensive setup for an EKS cluster with appropriate settings for node groups, access control, add-ons, and security. The organization of sections with explanatory comments makes the configuration clear and maintainable.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 94-94: no new line character at the end of file
(new-line-at-end-of-file)
test/fixtures/atmos.yaml (2)
58-60: Consider more flexible log configuration.The log file is set to "/dev/stdout" which works on Linux/Unix systems but might not be compatible with Windows environments. Consider adding a comment about platform compatibility or providing conditional configuration.
If your tests need to run on Windows, you might want to use a more platform-independent logging approach.
1-78: Well-documented configuration file with comprehensive settings.The Atmos configuration file includes detailed comments explaining each setting and alternative ways to specify values (env vars, command-line args). This is excellent practice for maintainability and onboarding new developers.
|
/terratest |
|
These changes were released in v1.536.1. |
What
basiccomponent testdisabledcomponent testWhy
enabled: falsesetReferences
Summary by CodeRabbit
New Features
Chores
Tests