feat!: split into provider-specific sub-modules to allow excluding unused providers

[gberenice]

what

• Splits the monolithic module into modules/ssm (AWS-only) and modules/sops (SOPS-only) sub-modules so consumers can reference only the backend they need, avoiding unnecessary provider downloads during init
• Adds three export mixins following the secrets.<backend>.mixin.tf naming pattern: secrets.ssm.mixin.tf, secrets.sops.mixin.tf, and secrets.mixin.tf (combined)
• Root module is refactored to delegate to sub-modules internally - fully backward compatible for existing consumers using masterpointio/helper/secrets.

why

• The module unconditionally declares carlpett/sops in required_providers. Terraform/OpenTofu resolves all provider requirements at init time via static analysis before evaluating any expressions like for_each. This means even when no SOPS secrets are configured (e.g. SSM-only consumers), the SOPS provider must still be downloadable, causing init failures in environments where it isn't available.
• HCL does not support conditional required_providers, and for_each = [] or count = 0 on modules/resources does not prevent provider installation. The only way to avoid a provider dependency is to have zero references to it in the loaded configuration.

references

• https://www.notion.so/masterpoint/SPIKE-Investigate-dynamic-removal-of-unused-providers-from-secrets-helper-module-317859758a56818aa572ff795e8fc340?source=copy_link

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced modular architecture enabling independent configuration of AWS Parameter Store and SOPS-based secret backends with flexible mapping options.

• Documentation

  • Enhanced guides with updated architecture details and expanded usage examples for different secret management configurations.

• Chores

  • Updated dependencies: trunk plugin, renovate linter, markdownlint, and trivy scanner to latest patch versions.

• Tests

  • Refined test coverage to validate consolidated secrets validation across integrated backends.

[Gowiem]

@gberenice makes sense and I think I like this. 👍

Let's get a 2nd set of eyes from @oycyc since he's also involved in the project where we're using this and I want additional eyes on due to the major rev.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors the secrets helper from a monolithic design to a modular architecture with provider-specific sub-modules. The root module now delegates secret handling to separate SSM and SOPS modules, each managing their own data sources and configurations. New mixin files (sops and ssm) provide alternative entry points for single-provider usage. The secret_mapping variable is standardized with optional type fields and consistent validations across all modules. Tests are updated to verify merged secrets rather than intermediate collections.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• fix!: file should be optional + actualize docs #28: Modifies the secret_mapping variable schema and adds validations for SOPS/SSM type handling, alongside changes to how secrets are exported and processed through mixins and modules.

Suggested reviewers

• natemccurdy
• Gowiem

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly and accurately describes the main architectural change: splitting into provider-specific sub-modules. It uses a conventional commit format with 'feat!' indicating a breaking change, directly aligning with the refactoring effort described in the PR objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/separate-modules

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (3)

README.md (1)

152-152: Minor note on generated docs.

The Requirements section shows >= 0.7 for sops, but the root versions.tf pins to 1.3.0. This discrepancy is likely fine since the pre-commit hook generates docs from the actual versions.tf, but worth a quick sanity check that the docs hook ran after the latest changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 152, The README's Requirements entry for sops shows ">=
0.7" but the root Terraform pin in versions.tf is "1.3.0"; reconcile them by
regenerating the docs from the canonical source (run the pre-commit/docs
generation hook or the docgen script) or manually update the README entry for
the sops row (the `<a name="requirement_sops"></a>` table entry) to reflect the
actual pinned version in versions.tf (1.3.0) so the docs and versions.tf agree.

main.tf (1)

27-28: Guard against cross-backend name collisions before merge.

Line 28 will silently overwrite duplicates when the same name exists in both SOPS and SSM mappings. Consider validating global uniqueness of secret_mapping[*].name in variables.tf to fail early.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@main.tf` around lines 27 - 28, The locals block merges module.sops.all and
module.ssm.all which silently overwrites secrets with identical keys; add a
validation in variables.tf that checks global uniqueness of
secret_mapping[*].name (or otherwise validate the combined set) and fail early
on duplicates. Specifically, implement a pre-merge uniqueness check referencing
the secret_mapping variable (or the modules' outputs) to detect any duplicate
names across module.sops and module.ssm and surface a clear error message
instead of allowing locals.secrets = merge(...) to overwrite values. Ensure the
validation references secret_mapping[*].name (or the combined keys) so
duplicates are rejected before apply.

tests/locals.tftest.hcl (1)

59-65: Assert resolved values, not only key presence.

These checks pass even when a key exists with a null value. Add non-null (or exact-value) assertions so missing secret resolution is caught by tests.

Example assertion pattern

   assert {
-    condition = alltrue([
-      contains(keys(local.secrets), "db_password"),
-      contains(keys(local.secrets), "api_key"),
-      contains(keys(local.secrets), "redis_password"),
-    ])
-    error_message = "All SOPS secret names should be present"
+    condition = alltrue([
+      try(local.secrets["db_password"], null) != null,
+      try(local.secrets["api_key"], null) != null,
+      try(local.secrets["redis_password"], null) != null,
+    ])
+    error_message = "All SOPS secrets should resolve to non-null values"
   }

Also applies to: 92-97, 129-135

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/locals.tftest.hcl` around lines 59 - 65, The test currently only
asserts key presence using contains(keys(local.secrets), "...") which passes
when values are null; update each condition that checks secret names (e.g., the
block referencing local.secrets for "db_password", "api_key", "redis_password"
and the other similar blocks) to assert resolved, non-null (or exact) values
instead—for example replace the key-presence checks with lookups/field access
that ensure lookup(local.secrets, "<name>", null) != null or
local.secrets.<name> != null (or compare to the expected literal) so the test
fails when a secret resolves to null; make the same change for the other two
secret-check blocks referenced in the comment.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@exports/secrets.mixin.tf`:
- Around line 13-18: The module block for module "secrets" references a
non-existent version 3.0.0; update the version attribute in the module "secrets"
block (the version = "3.0.0" line) to a published version such as "2.0.1" from
the Terraform Registry and verify the source stays as
"masterpointio/helper/secrets"; after updating, run terraform init to ensure the
module resolves successfully.

In `@exports/secrets.sops.mixin.tf`:
- Around line 17-24: The mixin currently filters var.secret_mapping to only
include mappings with mapping.type == "sops" via the secret_mapping
comprehension but still allows "ssm" in the separate validation (lines
referenced) so ssm entries can be silently dropped; update the validation logic
for var.secret_mapping (the same input validated alongside the secret_mapping
comprehension) to reject any mapping where mapping.type != "sops" (explicitly
disallow "ssm"), returning a clear error if a non-"sops" type is present so
users are not silently ignored. Ensure you reference the same var.secret_mapping
and mapping.type symbols when modifying the validation and keep the
comprehension unchanged in behavior.

In `@exports/secrets.ssm.mixin.tf`:
- Around line 17-24: The secret_mapping block filters out non-SSM mappings but
the code at lines 42-48 still processes 'sops' type secrets, allowing invalid
input to pass unnoticed. To fix this, ensure the same filtering logic that
rejects non-SSM types is applied consistently in both secret_mapping and the
processing code at lines 42-48, explicitly rejecting or ignoring 'sops' type
mappings in the SSM-only mixin.

In `@modules/sops/main.tf`:
- Around line 12-15: The current sops_secrets for-expression uses
lookup(local.sops_yamls[mapping.file], mapping.name, null) which silently
returns null for missing keys; change it to use direct indexing so Terraform
errors immediately on a bad mapping (e.g.
local.sops_yamls[mapping.file][mapping.name] for each mapping), keeping the same
keys from var.secret_mapping and ensuring the expression references
sops_secrets, var.secret_mapping and local.sops_yamls to locate the code to
update.

---

Nitpick comments:
In `@main.tf`:
- Around line 27-28: The locals block merges module.sops.all and module.ssm.all
which silently overwrites secrets with identical keys; add a validation in
variables.tf that checks global uniqueness of secret_mapping[*].name (or
otherwise validate the combined set) and fail early on duplicates. Specifically,
implement a pre-merge uniqueness check referencing the secret_mapping variable
(or the modules' outputs) to detect any duplicate names across module.sops and
module.ssm and surface a clear error message instead of allowing locals.secrets
= merge(...) to overwrite values. Ensure the validation references
secret_mapping[*].name (or the combined keys) so duplicates are rejected before
apply.

In `@README.md`:
- Line 152: The README's Requirements entry for sops shows ">= 0.7" but the root
Terraform pin in versions.tf is "1.3.0"; reconcile them by regenerating the docs
from the canonical source (run the pre-commit/docs generation hook or the docgen
script) or manually update the README entry for the sops row (the `<a
name="requirement_sops"></a>` table entry) to reflect the actual pinned version
in versions.tf (1.3.0) so the docs and versions.tf agree.

In `@tests/locals.tftest.hcl`:
- Around line 59-65: The test currently only asserts key presence using
contains(keys(local.secrets), "...") which passes when values are null; update
each condition that checks secret names (e.g., the block referencing
local.secrets for "db_password", "api_key", "redis_password" and the other
similar blocks) to assert resolved, non-null (or exact) values instead—for
example replace the key-presence checks with lookups/field access that ensure
lookup(local.secrets, "<name>", null) != null or local.secrets.<name> != null
(or compare to the expected literal) so the test fails when a secret resolves to
null; make the same change for the other two secret-check blocks referenced in
the comment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 20af5316-d2a6-43a1-91d9-288169d9ed6e

📥 Commits

Reviewing files that changed from the base of the PR and between ed533a6 and 0590f5d.

📒 Files selected for processing (16)

• .trunk/trunk.yaml
• README.md
• exports/secrets.mixin.tf
• exports/secrets.sops.mixin.tf
• exports/secrets.ssm.mixin.tf
• main.tf
• modules/sops/main.tf
• modules/sops/outputs.tf
• modules/sops/variables.tf
• modules/sops/versions.tf
• modules/ssm/main.tf
• modules/ssm/outputs.tf
• modules/ssm/variables.tf
• modules/ssm/versions.tf
• tests/locals.tftest.hcl
• versions.tf