Cloudaccount form #440
Cloudaccount form #440
Conversation
WalkthroughAdds new cloud-account output facets for AWS, Azure, and GCP and provider-specific cloud_account modules (v1.0) that include Terraform variables, external data sources calling a secret-fetcher script, local output mappings of sensitive attributes, and UI wiring to provider plugins. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Module
participant SecretScript as secret-fetcher.py
participant Provider
User->>Module: supply cloud_account (and region/config)
Module->>SecretScript: run script with (cloud_account, PROVIDER)
SecretScript-->>Module: return JSON attributes (sensitive)
Module->>Provider: configure provider (map attributes -> provider config)
Module-->>User: expose output facet with attributes (sensitive marked)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
aditya-facets
requested review from
anshulsao,
ishaankalra,
pramodh-ayyappan,
rr0hit,
unni-facets and
vishnukv-facets
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (3)
modules/cloud_account/azure-cloudaccount/0.1/locals.tf (1)
1-8: Same portability issue as GCP module – avoid absolute pathsSee previous comment; the Azure module hard-codes the same absolute path. Use
path.moduleor another relative mechanism so that consumers don’t need to replicate your directory structure.modules/cloud_account/gcp-cloudaccount/0.1/variables.tf (1)
1-20: Repeat: strengthen variable typesSee the AWS comment – adopt concrete object types rather than
anyto catch schema drift early.modules/cloud_account/azure-cloudaccount/0.1/variables.tf (1)
1-20: Repeat: strengthen variable typesSee the AWS comment – adopt concrete object types rather than
anyto catch schema drift early.
🧹 Nitpick comments (4)
modules/cloud_account/azure/0.1/facets.yaml (1)
34-51: Heavy duplication – factor with YAML anchors to keep the facet DRYEach provider block repeats identical attribute mappings. You can cut the file size and future maintenance effort by defining the common mapping once and re-using it via YAML anchors/aliases:
attributes_template: &az_attrs features: {} subscription_id: attributes.subscription_id client_id: attributes.client_id client_secret: attributes.client_secret tenant_id: attributes.tenant_id providers: azurerm3_116_0: source: hashicorp/azurerm version: 3.116.0 attributes: *az_attrsmodules/cloud_account/gcp-cloudaccount/0.1/outputs.tf (1)
1-11:projectprobably isn’t a secret—double-check what actually needs redactionPlacing
"project"in thesecretslist hides it from logs/outputs even though the GCP project ID is rarely sensitive. Hiding non-sensitive data hampers operability (e.g. debugging). Consider restrictingsecretsto truly confidential keys (credentials).modules/cloud_account/aws-cloudaccount/0.1/variables.tf (1)
1-20: Loosenedanytyping loses validation – define object schemasAll four variables are typed
any; Terraform loses static validation and IDE autocompletion. Declare explicit object types so errors surface early, e.g.:-variable "instance" { - type = any -} +variable "instance" { + type = object({ + spec = object({ + cloud_account = string + region = string + }) + }) + description = "CRD instance carrying cloud account and region details" +}Apply the same tightening for
environmentandinputswhere possible.modules/cloud_account/gcp-cloudaccount/0.1/facets.yaml (1)
31-34:x-ui-orderlists onlycloud_account; consider adding any future inputs now.If additional inputs (e.g.,
project,region) are likely, pre-ordering them here avoids a breaking UI change later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
modules/cloud_account/aws-cloudaccount/0.1/facets.yaml(1 hunks)modules/cloud_account/aws-cloudaccount/0.1/locals.tf(1 hunks)modules/cloud_account/aws-cloudaccount/0.1/outputs.tf(1 hunks)modules/cloud_account/aws-cloudaccount/0.1/variables.tf(1 hunks)modules/cloud_account/azure-cloudaccount/0.1/facets.yaml(1 hunks)modules/cloud_account/azure-cloudaccount/0.1/locals.tf(1 hunks)modules/cloud_account/azure-cloudaccount/0.1/outputs.tf(1 hunks)modules/cloud_account/azure-cloudaccount/0.1/variables.tf(1 hunks)modules/cloud_account/azure/0.1/facets.yaml(2 hunks)modules/cloud_account/gcp-cloudaccount/0.1/facets.yaml(1 hunks)modules/cloud_account/gcp-cloudaccount/0.1/locals.tf(1 hunks)modules/cloud_account/gcp-cloudaccount/0.1/outputs.tf(1 hunks)modules/cloud_account/gcp-cloudaccount/0.1/variables.tf(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
modules/cloud_account/aws-cloudaccount/0.1/variables.tf (1)
Learnt from: sanmesh-kakade
PR: Facets-cloud/facets-modules#393
File: modules/mongo_user/default/0.3/main.tf:63-110
Timestamp: 2025-06-03T11:10:34.578Z
Learning: In the mongo_user module at modules/mongo_user/default/0.3/, the inputs variable is properly declared with type object containing mongo_details.interfaces.writer.connection_string structure, making var.inputs.mongo_details.interfaces.writer.connection_string a valid reference.
modules/cloud_account/azure-cloudaccount/0.1/variables.tf (1)
Learnt from: sanmesh-kakade
PR: Facets-cloud/facets-modules#393
File: modules/mongo_user/default/0.3/main.tf:63-110
Timestamp: 2025-06-03T11:10:34.578Z
Learning: In the mongo_user module at modules/mongo_user/default/0.3/, the inputs variable is properly declared with type object containing mongo_details.interfaces.writer.connection_string structure, making var.inputs.mongo_details.interfaces.writer.connection_string a valid reference.
modules/cloud_account/gcp-cloudaccount/0.1/variables.tf (1)
Learnt from: sanmesh-kakade
PR: Facets-cloud/facets-modules#393
File: modules/mongo_user/default/0.3/main.tf:63-110
Timestamp: 2025-06-03T11:10:34.578Z
Learning: In the mongo_user module at modules/mongo_user/default/0.3/, the inputs variable is properly declared with type object containing mongo_details.interfaces.writer.connection_string structure, making var.inputs.mongo_details.interfaces.writer.connection_string a valid reference.
🔇 Additional comments (3)
modules/cloud_account/azure/0.1/facets.yaml (1)
34-51: Verify Terraform alias naming for provider map keysThe keys
azurerm3-105-0andazurerm3-116-0in modules/cloud_account/azure/0.1/facets.yaml (lines 34–51) contain hyphens, which Terraform disallows in alias identifiers (only letters, digits and underscores are permitted).Please confirm whether these YAML keys are ever used directly as
provider "azurerm" { alias = "<key>" }.
If so, rename them (e.g.azurerm3_105_0,azurerm3_116_0) to avoid Terraform syntax errors.modules/cloud_account/aws-cloudaccount/0.1/outputs.tf (1)
4-7: Verify thatlocal.script_outputactually containsaws_region
local.script_outputis referenced foraws_region, but the locals file isn’t shown here. Make sure the external script returns that key; otherwise this will benulland may cascade into provider mis-configuration.modules/cloud_account/aws-cloudaccount/0.1/locals.tf (1)
13-15: Key casing mismatch may break JSON lookup
iamRole/externalIdcamel-case assumes the Python script returns those exact keys. Confirm the script output; most AWS IAM examples useiam_role,external_id. A mismatch givesnulllocals and downstream failures.
| azurerm: | ||
| source: hashicorp/azurerm | ||
| version: 2.52.0 | ||
| version: 2.99.0 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
azurerm v2 is effectively end-of-life – consider migrating to v3 for consistency
You bumped to 2.99.0 but the same facet now lists multiple v3 variants below. Mixing provider major versions can trip up dependency resolution and forces consumers to learn two provider blocks. Unless there is a hard blocker, migrate the main azurerm entry to v3.116.0 and drop the v2 stanza.
🏁 Script executed:
#!/bin/bash
# Display versions listed in the facets.yaml to verify v2 and v3 entries
grep -n 'version:' modules/cloud_account/azure/0.1/facets.yaml
echo "----- Context around line 18 -----"
sed -n '1,50p' modules/cloud_account/azure/0.1/facets.yaml | nl -ba | sed -n '1,50p'Length of output: 464
Ensure Consistent Azure Provider Versioning – Migrate to v3
File: modules/cloud_account/azure/0.1/facets.yaml
Primary version (line 18) is set to v2.99.0, but downstream variants include v3.52.0, v3.105.0, and v3.116.0. Mixing major versions can introduce dependency-resolution conflicts and requires consumers to define two provider blocks. To streamline usage and avoid breaking changes:
• Update the primary version on line 18 to the latest v3 variant.
• Remove the v2.99.0 stanza entirely.
Suggested diff:
--- a/modules/cloud_account/azure/0.1/facets.yaml
+++ b/modules/cloud_account/azure/0.1/facets.yaml
@@ line 18
- version: 2.99.0
+ version: 3.116.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.
| version: 2.99.0 | |
| version: 3.116.0 |
🤖 Prompt for AI Agents
In modules/cloud_account/azure/0.1/facets.yaml at line 18, the version is set to
2.99.0, which conflicts with downstream v3 versions. Update this primary version
to the latest v3 variant used downstream and remove the entire v2.99.0 stanza to
ensure consistent Azure provider versioning and avoid dependency conflicts.
| data "external" "gcp_fetch_cloud_secret" { | ||
| program = [ | ||
| "python3", | ||
| "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", | ||
| var.instance.spec.cloud_account, | ||
| "GCP" | ||
| ] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Absolute script path breaks module portability
Hard-coding /sources/primary/.../secret-fetcher.py ties the module to one repository layout and prevents reuse from other directories or Terraform registries.
Prefer a path relative to the module root:
program = [
"python3",
"${path.module}/scripts/secret-fetcher.py",
var.instance.spec.cloud_account,
"GCP",
]🤖 Prompt for AI Agents
In modules/cloud_account/gcp-cloudaccount/0.1/locals.tf lines 1 to 8, the script
path for the external data source is hard-coded with an absolute path, which
reduces module portability. Replace the absolute path with a relative path using
the Terraform variable ${path.module} to reference the module root directory,
updating the program array to use
"${path.module}/scripts/cloudaccount-fetch-secret/secret-fetcher.py" instead of
the fixed absolute path.
| aws_iam_role = sensitive(local.script_output.aws_iam_role) | ||
| session_name = "capillary-cloud-tf-${uuid()}" | ||
| external_id = sensitive(local.script_output.external_id) | ||
| aws_region = local.script_output.aws_region |
There was a problem hiding this comment.
Using uuid() causes perpetual diffs
uuid() returns a new value on every plan/apply, so session_name will constantly change and force downstream resources (or outputs) to replace.
If you only need uniqueness per cloud-account, derive a stable ID (e.g. uuidv5(var.instance.spec.cloud_account, "capillary")) or drop the random suffix entirely.
🤖 Prompt for AI Agents
In modules/cloud_account/aws-cloudaccount/0.1/outputs.tf around lines 4 to 7,
the use of uuid() in session_name causes it to change on every plan/apply,
leading to perpetual diffs. Replace uuid() with a stable unique identifier such
as uuidv5 using a consistent input like var.instance.spec.cloud_account combined
with a namespace string "capillary" to ensure session_name remains stable across
runs.
| ## facetsdemo/backend/accounts/667d6782d4f4f200072d621f | ||
| ## 7d811b1e-b652-4e7c-9902-08c3fa2b5039 No newline at end of file |
There was a problem hiding this comment.
Remove hard-coded tenant/account identifiers – leaks internal data
These two comment lines expose real resource IDs / GUIDs that look like internal tenancy or subscription identifiers. Keeping them in VCS is a direct PII / security leak and breaks the principle of least disclosure.
-## facetsdemo/backend/accounts/667d6782d4f4f200072d621f
-## 7d811b1e-b652-4e7c-9902-08c3fa2b5039Delete them before merging (and consider a history rewrite to keep them out of the git history).
📝 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.
| ## facetsdemo/backend/accounts/667d6782d4f4f200072d621f | |
| ## 7d811b1e-b652-4e7c-9902-08c3fa2b5039 |
🤖 Prompt for AI Agents
In modules/cloud_account/azure-cloudaccount/0.1/outputs.tf at lines 17 to 18,
remove the two comment lines containing hard-coded tenant or account identifiers
to prevent leaking internal resource IDs or GUIDs. Delete these lines entirely
before merging, and consider rewriting the git history to remove these sensitive
details from previous commits.
| locals { | ||
| output_interfaces = {} | ||
| output_attributes = { | ||
| subscription_id = sensitive(data.external.azure_fetch_cloud_secret.result["subscription_id"]) | ||
| client_id = sensitive(data.external.azure_fetch_cloud_secret.result["client_id"]) | ||
| client_secret = sensitive(data.external.azure_fetch_cloud_secret.result["client_secret"]) | ||
| tenant_id = sensitive(data.external.azure_fetch_cloud_secret.result["tenant_id"]) | ||
| secrets = [ | ||
| "subscription_id", | ||
| "client_id", | ||
| "client_secret", | ||
| "tenant_id", | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Module defines only locals – no output blocks published
Down-stream modules/workflows cannot reference subscription_id, client_id, etc. unless they are surfaced via output blocks (or a Facets-specific mechanism I might have missed). Double-check that the consuming layer actually sees local.output_attributes; otherwise add explicit outputs:
+output "output_attributes" {
+ description = "Sensitive Azure account attributes"
+ value = local.output_attributes
+ sensitive = true
+}🏁 Script executed:
#!/bin/bash
set -e
# Check for any defined Terraform output blocks in the outputs.tf file
rg "^\s*output" -n modules/cloud_account/azure-cloudaccount/0.1/outputs.tf || echo "No output blocks found in outputs.tf"Length of output: 138
Add missing Terraform output blocks for Azure credentials
The module currently only defines local values and does not publish any output blocks, so downstream modules cannot reference subscription_id, client_id, etc. Please add explicit outputs, for example:
+output "output_attributes" {
+ description = "Sensitive Azure account attributes"
+ value = local.output_attributes
+ sensitive = true
+}• File: modules/cloud_account/azure-cloudaccount/0.1/outputs.tf
• Lines: after the existing locals { … } block
📝 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.
| locals { | |
| output_interfaces = {} | |
| output_attributes = { | |
| subscription_id = sensitive(data.external.azure_fetch_cloud_secret.result["subscription_id"]) | |
| client_id = sensitive(data.external.azure_fetch_cloud_secret.result["client_id"]) | |
| client_secret = sensitive(data.external.azure_fetch_cloud_secret.result["client_secret"]) | |
| tenant_id = sensitive(data.external.azure_fetch_cloud_secret.result["tenant_id"]) | |
| secrets = [ | |
| "subscription_id", | |
| "client_id", | |
| "client_secret", | |
| "tenant_id", | |
| ] | |
| } | |
| } | |
| locals { | |
| output_interfaces = {} | |
| output_attributes = { | |
| subscription_id = sensitive(data.external.azure_fetch_cloud_secret.result["subscription_id"]) | |
| client_id = sensitive(data.external.azure_fetch_cloud_secret.result["client_id"]) | |
| client_secret = sensitive(data.external.azure_fetch_cloud_secret.result["client_secret"]) | |
| tenant_id = sensitive(data.external.azure_fetch_cloud_secret.result["tenant_id"]) | |
| secrets = [ | |
| "subscription_id", | |
| "client_id", | |
| "client_secret", | |
| "tenant_id", | |
| ] | |
| } | |
| } | |
| output "output_attributes" { | |
| description = "Sensitive Azure account attributes" | |
| value = local.output_attributes | |
| sensitive = true | |
| } |
🤖 Prompt for AI Agents
In modules/cloud_account/azure-cloudaccount/0.1/outputs.tf after the existing
locals block (lines 1-15), add explicit Terraform output blocks for
subscription_id, client_id, client_secret, and tenant_id. Each output should
reference the corresponding local.output_attributes value wrapped with the
sensitive() function to ensure these credentials are exposed properly for
downstream modules while maintaining their sensitivity.
| description: Module to configure and provision resources within a cloud account | ||
| (AWS, GCP, Azure) | ||
| properties: |
There was a problem hiding this comment.
Same YAML breakage as GCP file.
The description is split without a literal block. This renders invalid YAML.
- description: Module to configure and provision resources within a cloud account
- (AWS, GCP, Azure)
+ description: |
+ Module to configure and provision resources within an AWS cloud account📝 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.
| description: Module to configure and provision resources within a cloud account | |
| (AWS, GCP, Azure) | |
| properties: | |
| description: | | |
| Module to configure and provision resources within an AWS cloud account | |
| properties: |
🤖 Prompt for AI Agents
In modules/cloud_account/aws-cloudaccount/0.1/facets.yaml around lines 8 to 10,
the description field is split across multiple lines without using a proper YAML
multiline syntax, causing invalid YAML. Fix this by using a literal block scalar
(|) or folded style (>) to properly format the multiline description so it is
valid YAML.
| attributes: | ||
| region: attributes.aws_region | ||
| skip_region_validation: true | ||
| attributes: | ||
| assume_role: | ||
| role_arn: attributes.aws_iam_role | ||
| session_name: attributes.session_name | ||
| external_id: attributes.external_id |
There was a problem hiding this comment.
Nested attributes: key will be ignored – flatten the map.
Provider blocks expect a single‐level map; the inner attributes: key makes assume_role unreachable.
- region: attributes.aws_region
- skip_region_validation: true
- attributes:
- assume_role:
- role_arn: attributes.aws_iam_role
- session_name: attributes.session_name
- external_id: attributes.external_id
+ region: attributes.aws_region
+ skip_region_validation: true
+ assume_role:
+ role_arn: attributes.aws_iam_role
+ session_name: attributes.session_name
+ external_id: attributes.external_id📝 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.
| attributes: | |
| region: attributes.aws_region | |
| skip_region_validation: true | |
| attributes: | |
| assume_role: | |
| role_arn: attributes.aws_iam_role | |
| session_name: attributes.session_name | |
| external_id: attributes.external_id | |
| attributes: | |
| region: attributes.aws_region | |
| skip_region_validation: true | |
| assume_role: | |
| role_arn: attributes.aws_iam_role | |
| session_name: attributes.session_name | |
| external_id: attributes.external_id |
🤖 Prompt for AI Agents
In modules/cloud_account/aws-cloudaccount/0.1/facets.yaml around lines 56 to 63,
the nested 'attributes:' key inside the 'attributes' map causes the inner
'assume_role' block to be ignored. To fix this, remove the inner 'attributes:'
key and directly place 'assume_role' and its children at the same level as
'region' and 'skip_region_validation' within the top-level 'attributes' map,
ensuring a flat map structure.
| aws593: | ||
| source: hashicorp/aws593 | ||
| version: = 5.93.0 |
There was a problem hiding this comment.
Incorrect provider version syntax.
version: = 5.93.0 is not valid Terraform notation; omit the =.
- version: = 5.93.0
+ version: 5.93.0🤖 Prompt for AI Agents
In modules/cloud_account/aws-cloudaccount/0.1/facets.yaml at lines 53 to 55, the
provider version syntax is incorrect because it uses 'version: = 5.93.0'. Remove
the '=' sign so that the version line reads 'version: 5.93.0' to comply with
valid Terraform notation.
| - cloud_provider | ||
| - cloud_account |
There was a problem hiding this comment.
🛠️ Refactor suggestion
cloud_provider is ordered but not defined.
Drop the entry or add the property to avoid UI errors.
- - cloud_provider
- cloud_account🤖 Prompt for AI Agents
In modules/cloud_account/azure-cloudaccount/0.1/facets.yaml at lines 40 to 41,
the property 'cloud_provider' is listed in the order but is not defined in the
schema. To fix this, either remove 'cloud_provider' from the order list or
define the 'cloud_provider' property properly in the facets.yaml file to prevent
UI errors.
| features: {} | ||
| subscription_id: attributes.subscription_id | ||
| client_id: attributes.client_id | ||
| client_secret: attributes.client_secret | ||
| tenant_id: attributes.tenant_id |
There was a problem hiding this comment.
Provider features block is misplaced.
features {} belongs at the root of the provider config, not under attributes. Move it out so Terraform can parse it.
- features: {}
subscription_id: attributes.subscription_id
client_id: attributes.client_id
client_secret: attributes.client_secret
tenant_id: attributes.tenant_id
+ features: {}📝 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.
| features: {} | |
| subscription_id: attributes.subscription_id | |
| client_id: attributes.client_id | |
| client_secret: attributes.client_secret | |
| tenant_id: attributes.tenant_id | |
| subscription_id: attributes.subscription_id | |
| client_id: attributes.client_id | |
| client_secret: attributes.client_secret | |
| tenant_id: attributes.tenant_id | |
| features: {} |
🤖 Prompt for AI Agents
In modules/cloud_account/azure-cloudaccount/0.1/facets.yaml around lines 50 to
54, the features block is incorrectly placed under attributes. Move the
features: {} block to the root level of the provider configuration, outside and
above the attributes section, so Terraform can correctly parse it.
ishaankalra
left a comment
ishaankalra
left a comment
There was a problem hiding this comment.
There is already a cloud account module in this repo. Update the form in the existin file itself. Also follow the convention intent/flavor/version.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
modules/cloud_account/gcp/0.2/locals.tf (1)
3-4: Same path-hard-coding issue as Azure module
The absolute script path limits portability; please apply the same fix recommended for the Azure variant (usepath.moduleor a variable).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
modules/cloud_account/aws/0.2/facets.yaml(1 hunks)modules/cloud_account/aws/0.2/locals.tf(1 hunks)modules/cloud_account/aws/0.2/outputs.tf(1 hunks)modules/cloud_account/aws/0.2/variables.tf(1 hunks)modules/cloud_account/azure/0.2/facets.yaml(1 hunks)modules/cloud_account/azure/0.2/locals.tf(1 hunks)modules/cloud_account/azure/0.2/outputs.tf(1 hunks)modules/cloud_account/azure/0.2/variables.tf(1 hunks)modules/cloud_account/gcp/0.2/facets.yaml(1 hunks)modules/cloud_account/gcp/0.2/locals.tf(1 hunks)modules/cloud_account/gcp/0.2/outputs.tf(1 hunks)modules/cloud_account/gcp/0.2/variables.tf(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- modules/cloud_account/aws/0.2/variables.tf
- modules/cloud_account/aws/0.2/facets.yaml
- modules/cloud_account/gcp/0.2/variables.tf
- modules/cloud_account/gcp/0.2/outputs.tf
- modules/cloud_account/azure/0.2/variables.tf
- modules/cloud_account/azure/0.2/facets.yaml
- modules/cloud_account/aws/0.2/outputs.tf
- modules/cloud_account/aws/0.2/locals.tf
- modules/cloud_account/azure/0.2/outputs.tf
- modules/cloud_account/gcp/0.2/facets.yaml
🔇 Additional comments (2)
modules/cloud_account/azure/0.2/locals.tf (1)
1-8: Double-check secret handling – mark outputs as sensitive
externalwill surface the fetched JSON in state files and potentially interraform plan. Make sure every downstreamoutputthat exposes values fromdata.external.azure_fetch_cloud_secretis declared withsensitive = true, otherwise credentials can leak.modules/cloud_account/gcp/0.2/locals.tf (1)
1-8: Ensure GCP secrets stay protected
Confirm that any outputs or locals derived fromdata.external.gcp_fetch_cloud_secretare flaggedsensitive = trueto avoid leaking key material.
| "python3", | ||
| "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded absolute path makes the module non-portable
Relying on /sources/primary/.../secret-fetcher.py ties the module to a very specific directory layout and breaks when the repo is checked out elsewhere (e.g. CI runners, downstream consumers, Windows paths).
- "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py",
+ abspath("${path.module}/../../../../scripts/cloudaccount-fetch-secret/secret-fetcher.py"),or, better, expose the path via a variable (var.secret_fetcher_path) with a sane default using path.module.
This keeps the module self-contained and friendly to external reuse.
📝 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.
| "python3", | |
| "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", | |
| "python3", | |
| abspath("${path.module}/../../../../scripts/cloudaccount-fetch-secret/secret-fetcher.py"), |
🤖 Prompt for AI Agents
In modules/cloud_account/azure/0.2/locals.tf around lines 3 to 4, the path to
secret-fetcher.py is hard-coded as an absolute path, making the module
non-portable. Replace the hard-coded path with a variable (e.g.,
var.secret_fetcher_path) that has a default value constructed using path.module
to build a relative path to the script. This change will make the module
self-contained and portable across different environments.
| @@ -0,0 +1,9 @@ | |||
| data "external" "azure_fetch_cloud_secret" { | |||
| program = [ | |||
There was a problem hiding this comment.
why do we have this script in the iac repositort and not here?
| default: | ||
| type: '@outputs/cloud_account' | ||
| providers: | ||
| aws593: |
ishaankalra
left a comment
ishaankalra
left a comment
There was a problem hiding this comment.
added upgraded providers here
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
outputs/azure_cloudaccount/output.facets.yaml (1)
23-23: Add a trailing newlineSeveral CI linters (YAMLlint, git diff-display, etc.) flag missing EOF newlines.
- description: "Azure tenant ID. [Sensitive]" +\ description: "Azure tenant ID. [Sensitive]" +outputs/gcp_cloudaccount/output.facets.yaml (2)
7-8:attributesis null – specify or removeSame concern as the Azure facet: either define it as an object or delete it to avoid ambiguous schemas.
17-17: Add trailing newline- description: "GCP service account key. [Sensitive]" +\ description: "GCP service account key. [Sensitive]" +outputs/aws_cloudaccount/output.facets.yaml (2)
7-8: Define or drop the emptyattributeskeyAs with the other facets, leaving
attributes:without a schema may confuse validation tools.- attributes: + attributes: + type: object + description: Additional generic attributes for the cloud account
23-23: Add trailing newline- description: "AWS region of the cluster." +\ description: "AWS region of the cluster." +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
modules/cloud_account/aws/0.1/facets.yaml(1 hunks)modules/cloud_account/aws/0.2/facets.yaml(1 hunks)modules/cloud_account/azure/0.1/facets.yaml(2 hunks)modules/cloud_account/azure/0.2/facets.yaml(1 hunks)modules/cloud_account/gcp/0.1/facets.yaml(1 hunks)modules/cloud_account/gcp/0.2/facets.yaml(1 hunks)outputs/aws_cloudaccount/output.facets.yaml(1 hunks)outputs/azure_cloudaccount/output.facets.yaml(1 hunks)outputs/gcp_cloudaccount/output.facets.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- modules/cloud_account/azure/0.1/facets.yaml
- modules/cloud_account/gcp/0.2/facets.yaml
- modules/cloud_account/aws/0.2/facets.yaml
- modules/cloud_account/azure/0.2/facets.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
outputs/aws_cloudaccount/output.facets.yaml
[error] 23-23: no new line character at the end of file
(new-line-at-end-of-file)
outputs/azure_cloudaccount/output.facets.yaml
[error] 23-23: no new line character at the end of file
(new-line-at-end-of-file)
outputs/gcp_cloudaccount/output.facets.yaml
[warning] 12-12: wrong indentation: expected 8 but found 10
(indentation)
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
modules/cloud_account/gcp/0.1/facets.yaml (1)
15-23: Ensure new output facet exposesregionreferenced by providersProviders still map
region: attributes.region, but the newly-referenced@outputs/gcp_cloud_accountonly definesprojectandcredentialsunderinterfaces.Either (a) add
regionto the facet, or (b) confirm that downstream code consumes it from a different place. A mismatch will surface at plan/apply time.modules/cloud_account/aws/0.1/facets.yaml (1)
15-23: Change looks good – facet and providers stay in syncThe switch to
@outputs/aws_cloud_accountmatches the properties consumed by the provider configurations (aws_region,aws_iam_role, etc.). No further action required.
| attributes: | ||
| interfaces: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
attributes key is left undefined – add a schema or drop it
Leaving the attributes: key with a null value makes the schema ambiguous and can trigger validation failures in tooling that expects a fully-typed object.
- attributes:
+ attributes:
+ type: object
+ description: Additional generic attributes for the cloud account📝 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.
| attributes: | |
| interfaces: | |
| attributes: | |
| type: object | |
| description: Additional generic attributes for the cloud account | |
| interfaces: |
🤖 Prompt for AI Agents
In outputs/azure_cloudaccount/output.facets.yaml at lines 7 to 8, the attributes
key is present but undefined, which can cause schema ambiguity and validation
errors. To fix this, either define a proper schema for the attributes key or
remove the attributes key entirely if it is not needed, ensuring the YAML schema
remains clear and valid.
| project: | ||
| type: string | ||
| description: "GCP project ID fetched from external data source." | ||
| credentials: | ||
| type: string | ||
| description: "GCP service account key. [Sensitive]" No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix indentation & align with schema conventions
The extra two-space indent under properties: breaks YAML parsing (expected 8 but found 10). While touching this, also keep the attributes placeholder consistent.
- project:
- type: string
- description: "GCP project ID fetched from external data source."
- credentials:
- type: string
- description: "GCP service account key. [Sensitive]"
+ project:
+ type: string
+ description: "GCP project ID fetched from external data source."
+ credentials:
+ type: string
+ description: "GCP service account key. [Sensitive]"📝 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.
| project: | |
| type: string | |
| description: "GCP project ID fetched from external data source." | |
| credentials: | |
| type: string | |
| description: "GCP service account key. [Sensitive]" | |
| project: | |
| type: string | |
| description: "GCP project ID fetched from external data source." | |
| credentials: | |
| type: string | |
| description: "GCP service account key. [Sensitive]" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 12-12: wrong indentation: expected 8 but found 10
(indentation)
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In outputs/gcp_cloudaccount/output.facets.yaml around lines 12 to 17, the
indentation under properties is incorrect, causing YAML parsing errors. Adjust
the indentation of the project and credentials fields to align with the expected
8 spaces under properties, removing the extra two spaces. Also, ensure the
attributes placeholder follows the same indentation and schema conventions for
consistency.
ishaankalra
force-pushed
the
cloudaccount-form
branch
from
f8121c9 to
355a1e8
Compare
| @@ -0,0 +1,83 @@ | |||
| intent: cloud_account | |||
There was a problem hiding this comment.
@mbhandari31 can you please check if any customer is using cloud_account
There was a problem hiding this comment.
@aditya-facets make use of the below suggestions for flavor and version. also as part of this directory structure will also change
| - region | ||
| outputs: | ||
| default: | ||
| type: '@outputs/cloud_account' |
There was a problem hiding this comment.
change the output type
| type: '@outputs/cloud_account' | |
| type: '@facets/aws_cloud_account' |
| default: | ||
| type: '@outputs/cloud_account' | ||
| providers: | ||
| azurerm3-116-0: |
There was a problem hiding this comment.
remove this provider
| project: attributes.project | ||
| region: attributes.region | ||
| credentials: attributes.credentials | ||
| google5: |
There was a problem hiding this comment.
remove this provider
| external_id = sensitive(local.script_output.external_id) | ||
| aws_region = local.script_output.aws_region | ||
| secrets = [ | ||
| "aws_iam_role", |
| aws_region = local.script_output.aws_region | ||
| secrets = [ | ||
| "aws_iam_role", | ||
| "session_name", |
| client_secret = sensitive(data.external.azure_fetch_cloud_secret.result["client_secret"]) | ||
| tenant_id = sensitive(data.external.azure_fetch_cloud_secret.result["tenant_id"]) | ||
| secrets = [ | ||
| "subscription_id", |
There was a problem hiding this comment.
except client_secret all others are not secret so remove it
| } | ||
| } | ||
|
|
||
| ## facetsdemo/backend/accounts/667d6782d4f4f200072d621f |
There was a problem hiding this comment.
remove these comments
|
use mcp to validate all the modules which will include a attribute in facets.yaml so that we are aware that you have validated the modules using ftf |
| @@ -0,0 +1,72 @@ | |||
| intent: cloud_account | |||
| flavor: aws | |||
| version: '0.2' | |||
There was a problem hiding this comment.
| version: '0.2' | |
| version: '1.0' |
| @@ -0,0 +1,72 @@ | |||
| intent: cloud_account | |||
| flavor: aws | |||
There was a problem hiding this comment.
| flavor: aws | |
| flavor: aws_provider |
| @@ -0,0 +1,83 @@ | |||
| intent: cloud_account | |||
There was a problem hiding this comment.
@aditya-facets make use of the below suggestions for flavor and version. also as part of this directory structure will also change
| intent: cloud_account | ||
| alias-flavors: | ||
| - default | ||
| flavor: azure |
There was a problem hiding this comment.
| flavor: azure | |
| flavor: azure_provider |
| - default | ||
| flavor: azure | ||
| lifecycle: ENVIRONMENT | ||
| version: '0.2' |
There was a problem hiding this comment.
| version: '0.2' | |
| version: '1.0' |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
outputs/gcp_cloud_account/output.facets.yaml (1)
18-18: Add a trailing newlineYAML-lint flags the missing newline at EOF.
Tiny but keeps tooling (git diff, linters, concatenation scripts) happy.- description: "Service account key (sensitive)" +\ description: "Service account key (sensitive)" +outputs/aws_cloud_account/output.facets.yaml (2)
13-21: Consider marking secret attributes explicitly
aws_iam_roleandexternal_idare called “(sensitive)” in the description, but the facet spec usually surfaces secrecy via a dedicated flag (e.g.,x-facets-secret: trueor similar) so consuming systems can mask values automatically.
If the Facets schema supports such metadata, add it now; otherwise drop “(sensitive)” to avoid a false sense of protection.
24-24: Missing newline at EOFAdd the trailing newline for linting compliance.
outputs/azure_cloud_account/output.facets.yaml (2)
19-21: Flag secrets explicitly
client_secretis clearly sensitive; prefer an explicit secret flag rather than only a description suffix so that tooling can redact it.Example:
client_secret: type: string description: Azure client secret x-facets-secret: true # or whatever the repo standard uses
24-24: Add trailing newlineKeeps YAML-lint quiet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
modules/cloud_account/aws/0.2/facets.yaml(1 hunks)outputs/aws_cloud_account/output.facets.yaml(1 hunks)outputs/azure_cloud_account/output.facets.yaml(1 hunks)outputs/gcp_cloud_account/output.facets.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/cloud_account/aws/0.2/facets.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sanmesh-kakade
PR: Facets-cloud/facets-modules#393
File: modules/mongo_user/default/0.3/main.tf:63-110
Timestamp: 2025-06-03T11:10:34.578Z
Learning: For the Facets-cloud/facets-modules repository, module sources without specific version tags default to master branch and this is the preferred approach by the team.
🪛 YAMLlint (1.37.1)
outputs/aws_cloud_account/output.facets.yaml
[error] 24-24: no new line character at the end of file
(new-line-at-end-of-file)
outputs/azure_cloud_account/output.facets.yaml
[error] 24-24: no new line character at the end of file
(new-line-at-end-of-file)
outputs/gcp_cloud_account/output.facets.yaml
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
| interfaces: | ||
| attributes: |
There was a problem hiding this comment.
interfaces: placeholder is invalid (null value)
Same issue as in the GCP facet – a null interfaces field violates the facet schema.
- interfaces:📝 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.
| interfaces: | |
| attributes: | |
| attributes: |
🤖 Prompt for AI Agents
In outputs/aws_cloud_account/output.facets.yaml around lines 7 to 8, the
`interfaces:` field is set to null, which violates the facet schema. To fix
this, remove the `interfaces:` line if it is not needed or replace it with a
valid non-null value such as an empty list or appropriate default content
according to the schema requirements.
| interfaces: | ||
| attributes: |
There was a problem hiding this comment.
Null interfaces: field
Drop or fully define the interfaces property; null breaks consumers.
- interfaces:📝 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.
| interfaces: | |
| attributes: | |
| attributes: |
🤖 Prompt for AI Agents
In outputs/azure_cloud_account/output.facets.yaml around lines 7 to 8, the
interfaces property is currently null, which causes issues for consumers. To fix
this, either remove the interfaces field entirely if it is not needed, or
provide a complete and valid definition for the interfaces property to ensure it
is not null.
| interfaces: | ||
| attributes: |
There was a problem hiding this comment.
interfaces: is left null – either drop it or give it a proper schema definition
interfaces: is declared but no value is provided, so YAML will parse it as null.
The facet‐schema used across this repo expects interfaces to be an array (or omit it altogether when empty).
Leaving it null will break downstream validation tooling that assumes a concrete structure.
Recommended minimal fix – remove the placeholder:
- interfaces:If you really need the field, define it explicitly:
interfaces:
type: array
description: Interface names implemented by the facet
items:
type: string🤖 Prompt for AI Agents
In outputs/gcp_cloud_account/output.facets.yaml around lines 7 to 8, the
interfaces field is declared but left null, which breaks downstream validation
expecting an array or omission when empty. Fix this by either removing the
interfaces field entirely if unused, or define it explicitly as an array with a
description and string items as shown in the recommended schema.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
modules/cloud_account/aws_provider/1.0/locals.tf (1)
12-15: Propagate sensitivity earlier to avoid accidental loggingWrap secrets in
sensitive()at the point of assignment so they remain redacted throughout the module lifecycle.- aws_iam_role = data.external.aws_fetch_cloud_secret.result["iamRole"] - external_id = data.external.aws_fetch_cloud_secret.result["externalId"] + aws_iam_role = sensitive(data.external.aws_fetch_cloud_secret.result["iamRole"]) + external_id = sensitive(data.external.aws_fetch_cloud_secret.result["externalId"])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
modules/cloud_account/aws_provider/1.0/facets.yaml(1 hunks)modules/cloud_account/aws_provider/1.0/locals.tf(1 hunks)modules/cloud_account/aws_provider/1.0/outputs.tf(1 hunks)modules/cloud_account/aws_provider/1.0/variables.tf(1 hunks)modules/cloud_account/azure_provider/1.0/facets.yaml(1 hunks)modules/cloud_account/azure_provider/1.0/locals.tf(1 hunks)modules/cloud_account/azure_provider/1.0/outputs.tf(1 hunks)modules/cloud_account/azure_provider/1.0/variables.tf(1 hunks)modules/cloud_account/gcp_provider/1.0/facets.yaml(1 hunks)modules/cloud_account/gcp_provider/1.0/locals.tf(1 hunks)modules/cloud_account/gcp_provider/1.0/outputs.tf(1 hunks)modules/cloud_account/gcp_provider/1.0/variables.tf(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- modules/cloud_account/gcp_provider/1.0/outputs.tf
- modules/cloud_account/azure_provider/1.0/variables.tf
- modules/cloud_account/aws_provider/1.0/facets.yaml
- modules/cloud_account/gcp_provider/1.0/variables.tf
- modules/cloud_account/aws_provider/1.0/variables.tf
- modules/cloud_account/gcp_provider/1.0/facets.yaml
- modules/cloud_account/azure_provider/1.0/outputs.tf
- modules/cloud_account/azure_provider/1.0/facets.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sanmesh-kakade
PR: Facets-cloud/facets-modules#393
File: modules/mongo_user/default/0.3/main.tf:63-110
Timestamp: 2025-06-03T11:10:34.578Z
Learning: For the Facets-cloud/facets-modules repository, module sources without specific version tags default to master branch and this is the preferred approach by the team.
🔇 Additional comments (3)
modules/cloud_account/aws_provider/1.0/outputs.tf (1)
8-10: Verifysecretslist is completeOnly
external_idis listed, yetaws_iam_roleis also markedsensitive. If consumers rely on thesecretsarray to know which outputs to redact, omittingaws_iam_rolecould leak credentials.modules/cloud_account/azure_provider/1.0/locals.tf (2)
1-8: Missingsensitive = true& output validationFetching secrets via
externalwill emit the JSON map into state unless marked sensitive downstream.
Confirm that every output consuming this data source is declaredsensitive = true(or wrap withsensitive(...)). Otherwise, tenant credentials end up in plain-text Terraform state files.Additionally, ensure the Python script prints only a single JSON object to stdout; any extra logging on stdout will break the provider.
2-5: Confirm script location and avoid hard-coded absolute pathIt appears that
secret-fetcher.pyisn’t present under this module, so the absolute reference (/sources/primary/.../secret-fetcher.py) will almost certainly break on other machines. Please verify where the script actually lives:
- If it belongs in this module, move it into a
scripts/directory alongsidelocals.tfand invoke it via"${path.module}/scripts/secret-fetcher.py"- Otherwise, introduce a variable so callers can supply the correct path and document that requirement.
Affected location:
• modules/cloud_account/azure_provider/1.0/locals.tf (lines 2–5)Suggested diff:
program = [ "python3", - "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", + var.secret_fetcher_path, var.instance.spec.cloud_account, "AZURE", ]And add a variable with a sane default:
variable "secret_fetcher_path" { description = "Filesystem path to the secret-fetcher.py script" type = string default = "${path.module}/scripts/secret-fetcher.py" }
| data "external" "aws_fetch_cloud_secret" { | ||
| program = [ | ||
| "python3", | ||
| "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", | ||
| var.instance.spec.cloud_account, | ||
| "AWS" | ||
| ] |
There was a problem hiding this comment.
Hard-coded script path breaks module portability
The absolute path /sources/primary/.../secret-fetcher.py will fail for anyone consuming this module outside your mono-repo or a different directory layout. Make the path relative to path.module (or inject via a variable).
- "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py",
+ "${path.module}/scripts/secret-fetcher.py",📝 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.
| data "external" "aws_fetch_cloud_secret" { | |
| program = [ | |
| "python3", | |
| "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", | |
| var.instance.spec.cloud_account, | |
| "AWS" | |
| ] | |
| data "external" "aws_fetch_cloud_secret" { | |
| program = [ | |
| "python3", | |
| - "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", | |
| + "${path.module}/scripts/secret-fetcher.py", | |
| var.instance.spec.cloud_account, | |
| "AWS" | |
| ] | |
| } |
🤖 Prompt for AI Agents
In modules/cloud_account/aws_provider/1.0/locals.tf lines 1 to 7, the script
path is hard-coded with an absolute path, which breaks module portability.
Replace the absolute path with a path constructed relative to path.module or
pass the script path as a variable to ensure the module works correctly
regardless of the directory structure where it is used.
| aws_iam_role = sensitive(local.script_output.aws_iam_role) | ||
| session_name = "capillary-cloud-tf-${uuid()}" | ||
| external_id = sensitive(local.script_output.external_id) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Session name will change on every plan – persist value via random_uuid
uuid() is evaluated at plan time, so session_name gets a new value on every run, causing perpetual diffs and needless churn for any downstream consumers.
- session_name = "capillary-cloud-tf-${uuid()}"
+# Persist a stable value in state
+ session_name = "capillary-cloud-tf-${random_uuid.session_name.result}"Add once in the same module:
resource "random_uuid" "session_name" {}🤖 Prompt for AI Agents
In modules/cloud_account/aws_provider/1.0/outputs.tf around lines 4 to 6, the
session_name uses uuid() which changes on every plan causing unnecessary diffs.
Replace the dynamic uuid() call with a persistent value by adding a resource
"random_uuid" "session_name" {} in the same module and then reference
random_uuid.session_name.result in session_name to ensure it remains stable
across plans.
| "python3", | ||
| "/sources/primary/capillary-cloud-tf/tfmain/scripts/cloudaccount-fetch-secret/secret-fetcher.py", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hard-coded absolute script paths – use path.module or module files for portability.
Hard-coding /sources/primary/…/secret-fetcher.py ties the module to a very specific directory layout that will not exist for most users running terraform plan/apply locally or in CI/CD runners. Prefer something like:
program = [
"python3",
"${path.module}/scripts/secret-fetcher.py",
var.instance.spec.cloud_account,
"GCP",
]or distribute the script as its own module input so callers can choose the path.
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/locals.tf around lines 3 to 4, the
script path is hard-coded as an absolute path, which reduces portability.
Replace the absolute path with a relative path using Terraform's path.module
variable to reference the script location dynamically, for example, use
"${path.module}/scripts/secret-fetcher.py" instead of the fixed absolute path.
This change ensures the module works correctly regardless of where it is checked
out or run.
| var.instance.spec.cloud_account, | ||
| "GCP" |
There was a problem hiding this comment.
Passing cloud-account identifiers via CLI args leaks them in process listings.
var.instance.spec.cloud_account may contain PII or sensitive account IDs. When supplied as a positional argument, the value is visible to any process that can run ps on the host running Terraform. For the external data source, prefer supplying data on stdin via the query argument:
data "external" "gcp_fetch_cloud_secret" {
program = ["python3", "${path.module}/scripts/secret-fetcher.py"]
query = {
cloud_account = var.instance.spec.cloud_account
provider = "GCP"
}
}The script can then read the JSON payload from stdin, eliminating the exposure.
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/locals.tf around lines 5 to 6, avoid
passing var.instance.spec.cloud_account as a positional argument to prevent
sensitive data exposure in process listings. Instead, refactor the external data
source to pass cloud_account and provider values via the query argument as a
JSON object, which the external script can read from stdin, eliminating the risk
of leaking sensitive identifiers.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
modules/cloud_account/gcp_provider/1.0/facets.yaml (1)
31-31: Optional: Title clarity
Consider making the title GCP-specific to avoid confusion with other providers.Apply this diff:
- title: Cloud Account Manager + title: GCP Cloud Account Manager
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
modules/cloud_account/aws_provider/1.0/facets.yaml(1 hunks)modules/cloud_account/azure_provider/1.0/facets.yaml(1 hunks)modules/cloud_account/gcp_provider/1.0/facets.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/cloud_account/aws_provider/1.0/facets.yaml
- modules/cloud_account/azure_provider/1.0/facets.yaml
🔇 Additional comments (3)
modules/cloud_account/gcp_provider/1.0/facets.yaml (3)
29-34: Spec structure, requireds, and UI order look good
The object schema with a single required field and explicit x-ui-order is consistent and clear.
18-28: API params: includeContent usage verified for consistencyI’ve surveyed all
x-ui-api-sourceentries and confirmed thatincludeContent: falseis used uniformly across modules (e.g.cloud_account/aws_provider/1.0/facets.yaml,cloud_account/azure_provider/1.0/facets.yaml,ecs_service/default/0.2/facets.yaml, etc.). It’s safe to proceed withincludeContent: falsehere, provided the API still returns yourlabelKey/valueKeyfields and handles pagination/auth correctly.• modules/cloud_account/aws_provider/1.0/facets.yaml
• modules/cloud_account/azure_provider/1.0/facets.yaml
• modules/cloud_account/gcp_provider/1.0/facets.yaml
• …and dozens more underx-ui-api-sourcePlease confirm against the API documentation or integration tests that omitting the content payload doesn’t strip out the
name/idfields or alter pagination/auth behavior.
41-41: Provider version pin consistency
The GCP provider version (6.43.0) aligns with existingcloud_account/gcpmodules (which use unprefixed exact versions) and follows the same style as the Azure modules. AWS modules use"= <version>"pins, but for GCP/Azure the plain version syntax is already established. No changes are required here unless you intend to standardize pin syntax or adopt range constraints across all cloud_account facets.
| description: Module to configure and provision resources within a cloud account | ||
| (AWS, GCP, Azure) | ||
| properties: |
There was a problem hiding this comment.
Fix invalid YAML: stray line breaks under description break parsing
The continuation line for description at Line 9 is not a valid YAML scalar continuation and will cause a parse error. Use a folded scalar and, since this facet is GCP-specific, tighten the wording.
Apply this diff:
- description: Module to configure and provision resources within a cloud account
- (AWS, GCP, Azure)
+ description: >
+ Module to configure and provision resources within a GCP cloud account📝 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.
| description: Module to configure and provision resources within a cloud account | |
| (AWS, GCP, Azure) | |
| properties: | |
| description: > | |
| Module to configure and provision resources within a GCP cloud account | |
| properties: |
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/facets.yaml around lines 8 to 10, the
description uses an invalid continuation line which breaks YAML parsing; replace
the multi-line broken description with a single folded scalar (using >) and
tighten the text to be GCP-specific (e.g., "Module to configure and provision
resources within a GCP cloud account"); ensure the folded scalar is indented
correctly under the description key and remove the stray line break so the
description becomes one valid YAML scalar.
| description: Select the cloud account to use (must be previously linked) | ||
| title: Cloud Account | ||
| type: string | ||
| x-ui-typeable: true | ||
| x-ui-overrides-only: true | ||
| x-ui-api-source: | ||
| endpoint: /cc-ui/v1/accounts/ | ||
| method: GET | ||
| labelKey: name | ||
| valueKey: id | ||
| filterConditions: | ||
| - field: accountType | ||
| value: CLOUD | ||
| - field: provider | ||
| value: GCP | ||
| params: | ||
| includeContent: false |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Align UI source config: make selection non-typeable and verify provider filter casing
- If the value must come from the API, allowing arbitrary typing is risky; disable typeable.
- Confirm expected casing for provider filter with the API. Your clouds list uses "gcp" (lowercase) but filter uses "GCP" (uppercase). Inconsistent casing can break filtering.
Apply this diff to disable arbitrary typing:
- x-ui-typeable: true
+ x-ui-typeable: falseIf the API expects lowercase provider values, apply:
- - field: provider
- value: GCP
+ - field: provider
+ value: gcpRun this repo scan to check what other modules use for provider casing so we stay consistent:
🏁 Script executed:
#!/bin/bash
# Inspect existing facets for provider casing in filterConditions
rg -nA2 $'filterConditions:\n.*field: provider' | sed 's/^/>> /'Length of output: 256
To accurately determine the existing casing used for the provider filter across all facets, let’s rerun a simpler grep that matches field: provider and prints the following line:
🏁 Script executed:
#!/bin/bash
# Inspect all YAML facets for provider filter casing
rg -n "field: provider" -A1 --glob '*.yaml' --color neverLength of output: 536
Disable free‐form input on cloud account selection
To ensure users only pick from API‐provided values and avoid invalid entries, set x-ui-typeable to false. No change to the provider casing is needed—every module (AWS, AZURE, GCP) uses uppercase.
• File: modules/cloud_account/gcp_provider/1.0/facets.yaml
• Change at the UI config for the Cloud Account field:
@@ modules/cloud_account/gcp_provider/1.0/facets.yaml
- x-ui-typeable: true
+ x-ui-typeable: false📝 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.
| description: Select the cloud account to use (must be previously linked) | |
| title: Cloud Account | |
| type: string | |
| x-ui-typeable: true | |
| x-ui-overrides-only: true | |
| x-ui-api-source: | |
| endpoint: /cc-ui/v1/accounts/ | |
| method: GET | |
| labelKey: name | |
| valueKey: id | |
| filterConditions: | |
| - field: accountType | |
| value: CLOUD | |
| - field: provider | |
| value: GCP | |
| params: | |
| includeContent: false | |
| description: Select the cloud account to use (must be previously linked) | |
| title: Cloud Account | |
| type: string | |
| - x-ui-typeable: true | |
| + x-ui-typeable: false | |
| x-ui-overrides-only: true | |
| x-ui-api-source: | |
| endpoint: /cc-ui/v1/accounts/ | |
| method: GET | |
| labelKey: name | |
| valueKey: id | |
| filterConditions: | |
| - field: accountType | |
| value: CLOUD | |
| - field: provider | |
| value: GCP | |
| params: | |
| includeContent: false |
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/facets.yaml around lines 12 to 28, the
Cloud Account field currently allows free‑form input via x-ui-typeable: true;
change x-ui-typeable to false so the UI prevents typing and forces selection
from API-provided values (leave provider casing as-is).
| outputs: | ||
| default: | ||
| type: '@outputs/gcp_cloud_account' | ||
| providers: | ||
| google: | ||
| source: hashicorp/google | ||
| version: 6.43.0 | ||
| attributes: | ||
| project: attributes.project | ||
| region: attributes.region | ||
| credentials: attributes.credentials |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard sensitive attributes: ensure credentials are redacted and not persisted/logged
Mapping raw credentials as an attribute can risk exposure if not explicitly marked sensitive in the outputs facet and any UI. Ensure:
- The referenced facet '@outputs/gcp_cloud_account' treats credentials as secret/sensitive.
- Logs and diff views redact the value.
Please verify the outputs facet marks credentials as sensitive. You can locate it with:
If sensitivity flags are supported here, and handled at this level instead, consider:
attributes:
project: attributes.project
region: attributes.region
- credentials: attributes.credentials
+ credentials: attributes.credentials
+ x-sensitive-keys:
+ - credentialsConfirm the correct key name for sensitivity per your schema before applying.
🏁 Script executed:
#!/bin/bash
# Find and print the gcp_cloud_account outputs facet to confirm sensitivity flags
fd -a --hidden --follow -t f 'gcp_cloud_account' | rg -n 'output|credentials|sensitive|x-.*sensitive' -A4 -B2Length of output: 109
I’m not seeing any sensitivity markers in the outputs facet—let’s pull in the full facets.yaml to confirm where (or if) sensitive flags are supported.
🏁 Script executed:
#!/bin/bash
# Locate and dump the outputs facet definition in facets.yaml for gcp_provider
files=$(fd facets.yaml modules/cloud_account/gcp_provider -t f)
if [ -z "$files" ]; then
echo "Error: facets.yaml not found under modules/cloud_account/gcp_provider"
exit 1
fi
for file in $files; do
echo "---- $file ----"
# Print the outputs section
sed -n '/^outputs:/,/^[^ ]/p' "$file"
doneLength of output: 708
To determine the correct sensitivity annotation, let’s scan all facets.yaml files for existing markers:
This will show us how (or if) other facets declare sensitive outputs.
🏁 Script executed:
#!/bin/bash
echo "Searching for 'x-sensitive-keys' in facets definitions..."
rg -n 'x-sensitive-keys' -g '*facets.yaml' -A3 -B3 || echo "→ no x-sensitive-keys found"
echo "Searching for 'sensitive:' in facets definitions..."
rg -n 'sensitive:' -g '*facets.yaml' -A3 -B3 || echo "→ no sensitive: entries found"Length of output: 479
Mark credentials as sensitive in the outputs facet
The outputs definition in modules/cloud_account/gcp_provider/1.0/facets.yaml currently exposes raw credentials without any sensitivity flags. To ensure these aren’t persisted or shown in logs/UI, add an x-sensitive-keys section under the provider attributes:
• File: modules/cloud_account/gcp_provider/1.0/facets.yaml
• Location: lines 35–45 (under outputs.default.providers.google.attributes)
Proposed diff:
outputs:
default:
type: '@outputs/gcp_cloud_account'
providers:
google:
source: hashicorp/google
version: 6.43.0
attributes:
project: attributes.project
region: attributes.region
- credentials: attributes.credentials
+ credentials: attributes.credentials
+ x-sensitive-keys:
+ - credentialsNo other facets declare sensitive keys—please verify your facet schema supports x-sensitive-keys (or the correct annotation) so that credentials are properly redacted.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/facets.yaml around lines 35–45 the
outputs.default.providers.google.attributes block exposes raw credentials; add
an x-sensitive-keys annotation under that attributes block listing credentials
(e.g., x-sensitive-keys: [credentials]) so the credentials field is treated as
sensitive/redacted, and confirm the facet schema supports x-sensitive-keys (or
replace with the project’s required sensitive annotation) before committing.
| - cloud_account | ||
| outputs: | ||
| default: | ||
| type: '@outputs/gcp_cloud_account' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify referenced outputs facet exists
Ensure '@outputs/gcp_cloud_account' exists and matches the expected provider attribute schema.
Use:
🏁 Script executed:
#!/bin/bash
# Locate the referenced outputs facet
fd -a --hidden --follow -t f 'gcp_cloud_account' | sed 's/^/>> /'Length of output: 65
🏁 Script executed:
#!/bin/bash
# Search for a facet file named gcp_cloud_account.yaml anywhere in the repo
echo "Looking for gcp_cloud_account.yaml files:"
fd -t f 'gcp_cloud_account.yaml' | sed 's/^/>> /'
# Search within YAML files for any definition or reference of the outputs facet
echo "Searching for '@outputs/gcp_cloud_account' in file contents:"
rg -n "@outputs/gcp_cloud_account"Length of output: 407
Missing outputs facet: '@outputs/gcp_cloud_account' not found
The type reference in modules/cloud_account/gcp_provider/1.0/facets.yaml (line 37) points to an outputs facet that doesn’t exist in the repository.
• File: modules/cloud_account/gcp_provider/1.0/facets.yaml, line 37
• Current code:
type: '@outputs/gcp_cloud_account'Action required:
- Either create the corresponding outputs facet file (e.g. modules/cloud_account/gcp_provider/1.0/facets/outputs/gcp_cloud_account.yaml) following the provider attribute schema,
- Or update the
typeto reference an existing outputs facet.
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/facets.yaml around line 37, the type
references an outputs facet '@outputs/gcp_cloud_account' that does not exist in
the repo; either add a new outputs facet file at
modules/cloud_account/gcp_provider/1.0/facets/outputs/gcp_cloud_account.yaml
following the provider outputs schema (defining the expected output attributes
and types) or change the type value to point to an existing outputs facet name
in the project; ensure the chosen outputs facet matches the attributes used
elsewhere by this provider.
| iac: | ||
| validated_files: | ||
| - variables.tf |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Track FTF/MCP validation and expand validated files per reviewer request
Per the PR comment, validation should be tracked in facets.yaml. Also include all IaC files, not just variables.tf.
Apply this diff:
iac:
validated_files:
- - variables.tf
+ - variables.tf
+ - locals.tf
+ - outputs.tf
+ validation:
+ tool: FTF
+ status: passed
+ # TODO: update timestamp during validation run
+ timestamp: '2025-08-12T00:00:00Z'If your convention uses a different field name (e.g., x-mcp-validated), adjust accordingly. I can batch-generate similar updates for AWS/Azure provider facets if needed.
📝 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.
| iac: | |
| validated_files: | |
| - variables.tf | |
| iac: | |
| validated_files: | |
| - variables.tf | |
| - locals.tf | |
| - outputs.tf | |
| validation: | |
| tool: FTF | |
| status: passed | |
| # TODO: update timestamp during validation run | |
| timestamp: '2025-08-12T00:00:00Z' |
🤖 Prompt for AI Agents
In modules/cloud_account/gcp_provider/1.0/facets.yaml around lines 52-54, the
file currently only lists variables.tf under iac.validated_files; update the
facets.yaml to track FTF/MCP validation and include all IaC files for the module
(e.g., main.tf, variables.tf, outputs.tf, providers.tf, terraform.tfvars,
modules/* as applicable) under that key so validation coverage is explicit; if
your project uses a different convention (for example x-mcp-validated), use that
field name instead and ensure the list reflects every IaC file in the provider
directory per the reviewer request.
Description
Related issues
Type of change
Checklist
developbranchTesting
Reviewer instructions
Summary by CodeRabbit