Skip to content

Conversation

@tjholm
Copy link
Member

@tjholm tjholm commented Oct 28, 2025

A proposal for exposing additional suga properties to the platform editor.

e.g. "name": "${suga.stack_id}-vpc" using the stack id for unique naming

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

📝 Walkthrough

Walkthrough

This PR extends the Terraform deployment engine with a new "suga" token source. It introduces a sugaProperties map field to the TerraformDeployment struct initialized with a stack_id entry, adds token resolution logic for the "suga" prefix in the resolve handler, and updates the token regex patterns to recognize tokens starting with "suga" alongside existing prefixes (infra, var, self).

Possibly related PRs

Suggested reviewers

  • jyecusch
  • davemooreuws
  • raksiv

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "add suga properties for platform properties" accurately describes the core changes: introducing a new suga token source and associated properties field to TerraformDeployment. The title directly aligns with the main objective of exposing suga properties to the platform editor, and a developer reviewing history would clearly understand the purpose of this changeset without ambiguity.
Description Check ✅ Passed The description is directly relevant to the changeset, explaining that it exposes suga properties for the platform editor and providing a concrete usage example (${suga.stack_id}-vpc). This example corresponds to the implemented functionality where stack_id is initialized in sugaProperties, demonstrating the author understands and articulates the changes appropriately.

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

@coderabbitai coderabbitai bot requested a review from davemooreuws October 28, 2025 22:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
engines/terraform/resolve.go (1)

127-145: Clean implementation of suga property resolution.

The implementation correctly validates path components, provides helpful error messages listing available properties, and explicitly disallows nested access. The pattern is consistent with other token sources.

Consider adding a brief comment above the case explaining what suga properties are and their intended use (e.g., "platform-provided properties like stack_id for resource naming"). This would help future maintainers understand the purpose.

+	// suga properties are platform-provided values (e.g., stack_id) used for resource naming and configuration
 	case "suga":
 		if len(specRef.Path) < 1 {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bff2b5c and 522f2d1.

📒 Files selected for processing (3)
  • engines/terraform/deployment.go (2 hunks)
  • engines/terraform/resolve.go (1 hunks)
  • engines/terraform/token.go (1 hunks)
🔇 Additional comments (2)
engines/terraform/token.go (1)

48-49: LGTM - regex patterns updated correctly.

Both patterns consistently include "suga" in the alternation group, enabling token recognition for suga.* references.

engines/terraform/deployment.go (1)

31-31: Good foundation for suga properties.

The field and initialization are implemented correctly. The map is properly initialized in the constructor with stack_id, preventing nil access issues. The use of stackId.Result() provides a token reference that will resolve during Terraform synthesis.

Also applies to: 64-66

@tjholm tjholm marked this pull request as draft October 29, 2025 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants