Repository navigation
Expose ProcessTemplates and ProcessYamlFunctions flags in public API - #2161
Merged
Merged
Conversation
…c API Add optional processing controls to ProcessComponentInStack (variadic options) and ProcessComponentFromContext (struct fields) so embedded consumers like terraform-provider-utils can disable template and YAML function resolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Andriy Knysh (aknysh)
requested a review
from Erik Osterman (Cloud Posse) (osterman)
March 10, 2026 18:04
Contributor
📝 WalkthroughWalkthroughAdds a functional-options public API to control template and YAML-function processing for component description. Introduces an unexported Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (CLI / Consumer)
participant API as ProcessComponentInStack / FromContext
participant Core as processComponentInStackWithConfig
participant Worker as ExecuteDescribeComponent
participant Tmpl as TemplateProcessor
participant Yaml as YamlFunctionProcessor
rect rgba(0,128,255,0.5)
Caller->>API: call with opts ...ProcessOption
end
API->>Core: applyProcessOptions(opts) -> resolved opts
Core->>Worker: ExecuteDescribeComponent(..., ProcessTemplates: resolved.processTemplates, ProcessYamlFunctions: resolved.processYamlFunctions)
alt ProcessTemplates enabled
Worker->>Tmpl: run template processing
end
alt ProcessYamlFunctions enabled
Worker->>Yaml: run YAML function processing
end
Worker-->>API: component map result
API-->>Caller: return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/describe/component_processor_test.go (1)
14-22: Prefer testing the API behavior here, not the helper.
boolDefaultis a tiny internal helper, so this mostly locks in implementation detail. The nil/default behavior that matters to callers is already exercised throughProcessComponentInStackandProcessComponentFromContext, which is the better seam to test.As per coding guidelines, "Test behavior, not implementation. Never test stub functions. Avoid tautological tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/describe/component_processor_test.go` around lines 14 - 22, Remove the unit test TestBoolDefault that directly exercises the internal helper boolDefault and instead verify the same nil/default behavior through the public APIs used by callers (e.g., add or extend tests for ProcessComponentInStack and ProcessComponentFromContext to cover cases where the boolean pointer is nil and when it is true/false), so tests assert observable behavior rather than the internal boolDefault implementation; reference TestBoolDefault and boolDefault to locate and delete the tautological test and add equivalent coverage via ProcessComponentInStack/ProcessComponentFromContext tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/describe/component_processor_test.go`:
- Around line 311-419: The tests only exercise both-flags-false and nil
defaults, so add explicit unit tests that verify the flags are independently
wired by adding two new cases for ProcessComponentInStack and
ProcessComponentFromContext: one with ProcessTemplates=false and
ProcessYamlFunctions=true, and the inverse (ProcessTemplates=true and
ProcessYamlFunctions=false); use fixture components that include actual template
expressions and YAML functions (so behavior differs when each flag is toggled),
call ProcessComponentInStack (and ProcessComponentFromContext) with
ProcessComponentInStackOptions/params setting those pointer flags, and assert
that the output reflects only the enabled processor (e.g., template-resolved
values change when templates=true but YAML functions remain unresolved, and vice
versa) so the tests fail if a flag is ignored or swapped.
In `@pkg/describe/component_processor.go`:
- Around line 11-15: The public variadic pointer-bool config type
ProcessComponentInStackOptions should be replaced with a functional options API:
introduce an unexported options struct with concrete booleans defaulting to
true, define a ProcessOption (or similar) type like func(*options), and add
constructor helpers WithProcessTemplates(bool) and
WithProcessYamlFunctions(bool) that return ProcessOption; change the
ProcessComponentInStack signature to accept opts ...ProcessOption, apply
defaults then each option to mutate the internal options struct, and update all
callers to pass WithProcessTemplates(...) / WithProcessYamlFunctions(...)
instead of building ProcessComponentInStackOptions to eliminate *bool plumbing
and support future extensibility.
- Around line 62-66: The code reads only opts[0] and silently ignores additional
ProcessComponentInStackOptions entries; change this by explicitly handling
multiple option structs: either reject multiple options (check if len(opts) > 1
and return an error) or merge them deterministically (iterate over opts and set
processTemplates and processYamlFunctions from later entries or using a defined
precedence). Update the logic around the opts variable and the
processTemplates/processYamlFunctions assignments to perform the chosen
behavior, referencing the ProcessComponentInStackOptions type and the opts slice
so callers get a clear error or predictable merged result.
---
Nitpick comments:
In `@pkg/describe/component_processor_test.go`:
- Around line 14-22: Remove the unit test TestBoolDefault that directly
exercises the internal helper boolDefault and instead verify the same
nil/default behavior through the public APIs used by callers (e.g., add or
extend tests for ProcessComponentInStack and ProcessComponentFromContext to
cover cases where the boolean pointer is nil and when it is true/false), so
tests assert observable behavior rather than the internal boolDefault
implementation; reference TestBoolDefault and boolDefault to locate and delete
the tautological test and add equivalent coverage via
ProcessComponentInStack/ProcessComponentFromContext tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e6721593-5134-4cc7-b3f8-6e1510e966a2
📒 Files selected for processing (3)
docs/fixes/2026-03-10-expose-process-templates-and-yaml-functions-flags-in-public-api.mdpkg/describe/component_processor.gopkg/describe/component_processor_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2161 +/- ##
==========================================
- Coverage 77.17% 77.16% -0.01%
==========================================
Files 951 951
Lines 90358 90375 +17
==========================================
+ Hits 69737 69742 +5
- Misses 16544 16553 +9
- Partials 4077 4080 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Address PR review feedback: - Replace ProcessComponentInStackOptions struct with functional options (WithProcessTemplates, WithProcessYamlFunctions) - Remove TestBoolDefault (tests implementation, not behavior) - Add tests that verify each flag independently using real fixtures (stack-templates for Go templates, atmos-template-yaml-function for YAML functions) - Update fix doc to reflect functional options pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
CodeRabbit (@coderabbitai) full review please. If it looks good, please approve |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me examine the code changes in detail before proceeding. 🏁 Script executed: Length of output: 34612
✅ Overall Assessment: ApproveThe implementation is clean, idiomatic Go, and correctly solves the embedded-consumer crash problem. No blocking issues found. What's done well
Minor nit (non-blocking)
// Before
func WithProcessTemplates(enabled bool) ProcessOption {
defer perf.Track(nil, "describe.WithProcessTemplates")()
return func(o *processOptions) {
o.processTemplates = enabled
}
}
// Suggested
func WithProcessTemplates(enabled bool) ProcessOption {
return func(o *processOptions) {
o.processTemplates = enabled
}
}Up to you — it doesn't affect correctness. Approving now. [approve] 🧠 Learnings used |
WithProcessTemplates and WithProcessYamlFunctions just build a closure over a single bool — sub-microsecond with no meaningful perf signal. Suppress lintroller with nolint directive instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
approved these changes
Mar 10, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/describe/component_processor.go`:
- Around line 159-161: The options (applyProcessOptions) are being resolved
after stackNameTemplate is rendered, so WithProcessTemplates(false) won't
prevent template execution for the stack name; move the resolution/application
of process options earlier (call applyProcessOptions(opts) and ensure
WithProcessTemplates is applied to the context path) before any use of
stackNameTemplate/stack-name branching (references: applyProcessOptions,
WithProcessTemplates, stackNameTemplate, ProcessComponentFromContext,
processComponentInStackWithConfig) so the template-processing flag takes effect
for the stack-name branch; alternatively, if you prefer not to reorder, update
the public contract/docs to state that the flag only affects component body
processing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e1eca3f-d7a1-4856-931a-5d10a4dd65cc
📒 Files selected for processing (1)
pkg/describe/component_processor.go
|
These changes were released in v1.209.0. |
1 similar comment
|
These changes were released in v1.209.0. |
|
These changes were released in v1.209.0. |
|
These changes were released in v1.210.0-test.8. |
|
These changes were released in v1.210.0-test.10. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
ProcessTemplatesandProcessYamlFunctionscontrols to theProcessComponentInStackandProcessComponentFromContextpublic API functions inpkg/describeWithProcessTemplates(bool)andWithProcessYamlFunctions(bool)...ProcessOption— existing callers compile without changestruewhen omitted, matching current behavior (templates processed if enabled inatmos.yaml, YAML functions always processed)why
ProcessTemplates: trueandProcessYamlFunctions: trueinprocessComponentInStackWithConfig, giving embedded consumers no way to disable processing--process-templatesand--process-functionsflags,atmos.yamlhastemplates.settings.enabled, and stack imports haveskip_templates_processing— the public API was the only entry point missing this controlterraform-provider-utilsembeds Atmos and calls these functions from inside a Terraform provider plugin. WhenProcessYamlFunctionsistrue,!terraform.outputtags spawn childterraform initprocesses that conflict with the parent OpenTofu process's plugin cache, causingETXTBSY("text file busy") crashes on LinuxWithProcessTemplates(false)andWithProcessYamlFunctions(false)to avoid the crash entirelyusage
tests
TestProcessComponentInStackTemplatesDisabledOnlyWithProcessTemplates(false)preserves raw Go template strings while YAML functions remain enabledTestProcessComponentInStackTemplatesEnabledOnlyWithProcessTemplates(true)resolves Go templates while YAML functions are disabledTestProcessComponentInStackYamlFunctionsDisabledOnlyWithProcessYamlFunctions(false)preserves raw YAML function tags while templates remain enabledTestProcessComponentInStackYamlFunctionsEnabledOnlyWithProcessYamlFunctions(true)resolves YAML function tags while templates are disabledTestProcessComponentInStackBackwardCompatNoOptionsTestProcessComponentFromContextWithProcessingDisabledProcessComponentFromContextrespectsWithProcessTemplates(false)functional optionEach flag is tested independently against its own fixture (
stack-templatesfor Go templates,atmos-template-yaml-functionfor YAML functions), proving the two flags are wired independently.references
docs/fixes/2026-02-15-restore-component-processor-public-api.mdcloudposse/terraform-provider-utils#523Summary by CodeRabbit
New Features
Documentation
Tests