Add Value Based Exclusion for Environment Variables and Secrets#12
Add Value Based Exclusion for Environment Variables and Secrets#12omendra-tomar wants to merge 1 commit intomasterfrom
Conversation
WalkthroughThe update refactors the Terraform configuration for handling environment variables and secrets in a Helm release. It replaces JSON encoding/decoding and intermediate variables with direct merging and stringification of environment variable maps, streamlining the filtering and construction of the final environment variables passed to the Helm chart. Lifecycle ignore settings are also removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Terraform
participant HelmRelease
participant EnvVars
participant Secrets
Terraform->>EnvVars: Merge common and spec env vars
EnvVars->>EnvVars: Convert all values to strings
EnvVars->>EnvVars: Filter out excluded values
Terraform->>Secrets: Filter secrets
Terraform->>HelmRelease: Pass merged env vars and secrets inline
Estimated code review effort2 (~15 minutes) Possibly related PRs
Suggested reviewers
Poem
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
application/main.tf (2)
100-109: Secret filtering may skip intended items due to type mismatch
local.filtered_env_varscompares stringified env-var values against the exclusion list, butlocal.filtered_all_secretscompares the raw secret value (possibly sensitive / non-string) against the same list.
If the secret is already a complex/sensitive type,contains()will never match and the exclusion will silently fail.-filtered_all_secrets = { - for k, v in local.all_secrets : - k => v - if !(contains(local.exclude_env_and_secret_values, v)) -} +filtered_all_secrets = { + for k, v in local.all_secrets : + k => v + if !(contains(local.exclude_env_and_secret_values, tostring(v))) +}
181-186: Pin the external module source to a tag or commit
source = "github.com/Facets-cloud/facets-utility-modules//pvc"pulls the latest main, making the build non-deterministic and violating CKV_TF_1 / CKV_TF_2.-source = "github.com/Facets-cloud/facets-utility-modules//pvc" +source = "github.com/Facets-cloud/facets-utility-modules//pvc?ref=v1.4.3"Replace
v1.4.3with an existing, immutable tag or commit hash to lock dependencies.
🧹 Nitpick comments (1)
application/main.tf (1)
19-23: Usetostring()for safer value conversionString interpolation (
"${value}") implicitly callstoset.ctyToString, but it silently converts non-primitive types to an opaque representation (<cty.Value>).
tos tring(value)is explicit, guarantees a usable string, and fails fast if the value is not convertible – much safer for env-var propagation.-env_vars_str = {for key,value in local.env_vars: key => "${value}"} +env_vars_str = {for key, value in local.env_vars : key => tostring(value)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
application/main.tf(4 hunks)
🪛 Checkov (3.2.334)
application/main.tf
[MEDIUM] 181-198: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 181-198: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
🧰 Additional context used
🪛 Checkov (3.2.334)
application/main.tf
[MEDIUM] 181-198: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 181-198: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
🔇 Additional comments (1)
application/main.tf (1)
152-159: Confirm merge-order precedence of environment variablesThe merge order gives last-writer-wins semantics:
- global cluster vars
- filtered env vars
- build-id
- filtered secrets
- (optional) deployment_id
If a secret key duplicates one from
globalVariables, it will override the cluster value. Ensure this precedence is intentional; reversing lines 155 & 157 would give secrets lower priority.
Terraform module that allows users to exclude specific environment variables and secrets based on their values. The exclusion list is configurable via the input variable advanced.common.app_chart.values.exclude_env_and_secret_values. The update adds logic to filter out specific environment variables and secrets from being merged into Helm release values based on an exclusion list. This is achieved by defining new local variables that exclude entries with values specified in the exclusion list, affecting only the internal configuration of the Terraform module.
same as : https://github.com/Facets-cloud/facets-iac/pull/1840
Summary by CodeRabbit