Repository navigation
Restore public API for terraform-provider-utils component config - #2078
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
804e3f6 to
2baf1e4
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2baf1e4 to
9b14a18
Compare
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds public component-processing wrappers to Changes
Sequence DiagramsequenceDiagram
actor Client
participant ParamsResolver as "StackResolution"
participant CliConfig as "InitCliConfig"
participant PublicAPI as "pkg/describe (Public API)"
participant Executor as "internal/exec (Executor)"
Client->>PublicAPI: call ProcessComponentFromContext(params) or ProcessComponentInStack(component,stack)
PublicAPI->>ParamsResolver: resolve stack name (template or pattern) [if FromContext]
ParamsResolver-->>PublicAPI: stackName
PublicAPI->>CliConfig: InitCliConfig(atmosCliConfigPath, atmosBasePath)
CliConfig-->>PublicAPI: atmosConfig
PublicAPI->>PublicAPI: perf.Track
PublicAPI->>Executor: ExecuteDescribeComponent(atmosConfig, component, stack)
Executor-->>PublicAPI: component map / error
PublicAPI-->>Client: return map / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2078 +/- ##
==========================================
+ Coverage 76.30% 76.31% +0.01%
==========================================
Files 802 803 +1
Lines 75522 75574 +52
==========================================
+ Hits 57628 57678 +50
- Misses 14311 14312 +1
- Partials 3583 3584 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/fixes/2026-02-15-restore-component-processor-public-api.md`:
- Around line 88-93: The fenced code block containing the error message "Error:
Plugin did not respond\n with
module.iam_roles.module.account_map.data.utils_component_config.config[0],\n
The plugin encountered an error, and failed to respond to the\n
plugin.(*GRPCProvider).ReadDataSource call." needs a language identifier; update
the triple-backtick fence that surrounds that error sample to include a language
token (e.g., ```text or ```plain) so the block is properly tagged for static
analysis and rendering.
- Line 115: The docs claim both function signatures are preserved but only
ProcessComponentInStack is unchanged; ProcessComponentFromContext changed from
positional parameters to a single options struct—update the docs to state this
explicitly by noting that ProcessComponentFromContext now accepts a struct
(e.g., Options/Request type) instead of positional args while
ProcessComponentInStack remains unchanged, and include the new struct name and
its fields so callers know the breaking signature change.
- Around line 106-108: The provider update is more than an import change: the
function ProcessComponentFromContext in pkg/describe now accepts a single
*ComponentFromContextParams instead of seven positional string args, so update
the provider call sites to construct and pass a ComponentFromContextParams
struct literal (populating the same seven fields) and call
ProcessComponentFromContext(*ComponentFromContextParams) from the new
pkg/describe import; search for calls to ProcessComponentFromContext in the
provider and replace the positional-arg invocation with a struct literal
matching ComponentFromContextParams fields.
In `@pkg/describe/component_processor_test.go`:
- Around line 12-31: In TestProcessComponentInStack, replace the initial
assert.Nil/ assert.NotNil checks on the result of ProcessComponentInStack with
require.Nil/ require.NotNil so the test stops immediately if
ProcessComponentInStack fails (avoiding passing a nil result into
u.ConvertToYAML); do the same pattern for the other tests that assert then
immediately use the result (e.g., the similar block around lines referenced in
the review). Also update the test imports to include testify/require.
In `@pkg/describe/component_processor.go`:
- Around line 61-62: ProcessComponentFromContext currently dereferences params
without a nil guard which causes a panic if callers pass nil; add an early
nil-check at the top of ProcessComponentFromContext to return a clear error
(e.g., fmt.Errorf or errors.New) when params == nil before calling perf.Track or
accessing params fields, so callers get a controlled error instead of a panic.
Ensure the function signature and returned types (map[string]any, error) are
preserved and update any callers/tests if they relied on panic behavior.
🧹 Nitpick comments (3)
pkg/describe/component_processor.go (1)
61-103: DoubleInitCliConfigcall inProcessComponentFromContext.
ProcessComponentFromContextinitializes config at line 69, uses it to resolve the stack name (lines 74-101), then delegates toProcessComponentInStackat line 103 — which callsInitCliConfigagain at line 42. Config parsing runs twice per invocation.Consider refactoring so the resolved
atmosConfigis reused. For example, extract the core logic ofProcessComponentInStackinto a private helper that accepts an already-initializedatmosConfig, and call that helper from both public functions.Sketch
+func processComponentInStackWithConfig( + atmosConfig *schema.AtmosConfiguration, + component string, + stack string, +) (map[string]any, error) { + return e.ExecuteDescribeComponent(&e.ExecuteDescribeComponentParams{ + AtmosConfig: atmosConfig, + Component: component, + Stack: stack, + ProcessTemplates: true, + ProcessYamlFunctions: true, + }) +}Then
ProcessComponentFromContextcallsprocessComponentInStackWithConfigwith the config it already has.pkg/describe/component_processor_test.go (2)
22-30: Duplicated debug-on-failure cleanup block.The same
t.Cleanupblock with YAML logging appears four times. A small helper would reduce noise:func logOnFailure(t *testing.T, result map[string]any) { t.Helper() resultYaml, _ := u.ConvertToYAML(result) t.Cleanup(func() { if t.Failed() { if resultYaml != "" { t.Logf("Component section:\n%s", resultYaml) } else { t.Logf("Component section (raw): %+v", result) } } }) }Also applies to: 89-97, 202-210, 223-231
61-75: Consider table-driven tests for the error cases.
TestProcessComponentInStackInvalidComponent,TestProcessComponentInStackInvalidStack,TestProcessComponentFromContextInvalidComponent, andTestProcessComponentFromContextInvalidContextall follow the same shape: call → assert error. These consolidate well into a table.Also applies to: 117-135
- Extract processComponentInStackWithConfig to avoid double InitCliConfig - Add nil guard on ProcessComponentFromContext params - Extract logOnFailure test helper to deduplicate cleanup blocks - Consolidate error cases into table-driven tests - Increase test coverage (name_template branch, default branch, nil params) - Fix fenced code block language identifier in docs - Clarify signature change and provider update scope in docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/describe/component_processor_test.go (1)
229-260: Usefilepath.Joinfor test fixture paths.The hardcoded
"../../tests/fixtures/scenarios/locals-logical-names"string uses forward slashes, which the coding guidelines flag. Same pattern appears at lines 245, 263, and 275. Consider:Proposed fix
- atmosCliConfigPath := "../../tests/fixtures/scenarios/locals-logical-names" + atmosCliConfigPath := filepath.Join("..", "..", "tests", "fixtures", "scenarios", "locals-logical-names")As per coding guidelines: "Never use forward slash concatenation in tests - always use
filepath.Join()with separate arguments."pkg/describe/component_processor.go (1)
60-62: Use the existingErrNilParamsentinel instead of dynamic error.A suitable static error sentinel already exists at
errors/errors.go:657:errUtils.ErrNilParam("parameter cannot be nil"). Replace the dynamicfmt.Errorfwith this sentinel to align with project guidelines for consistent error handling.
- Use errUtils.ErrNilParam sentinel instead of dynamic fmt.Errorf - Use filepath.Join for test fixture paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8b46708
|
These changes were released in v1.206.3-rc.0. |
|
These changes were released in v1.207.0. |
|
These changes were released in v1.208.0-rc.0. |
|
These changes were released in v1.208.0-test.15. |
2 participants
what
ProcessComponentInStackandProcessComponentFromContextpublic functions that were deleted in v1.201.0 (PR feat: Path-based component resolution for all commands #1774)pkg/describe(notpkg/component) to avoid an import cyclecloudposse/terraform-provider-utilsdepends on for thedata "utils_component_config"data sourcewhy
pkg/component/component_processor.gowas deleted entirely. This file containedProcessComponentInStackandProcessComponentFromContext, which are the public API consumed bycloudposse/terraform-provider-utilsinternal/exec, which external Go modules cannot importstores,hooks,gomplatetemplates,!terraform.state/!terraform.outputYAML tags) experience provider crashes ("Plugin did not respond") because the old v1.189.0 code in the provider cannot parse the newer configuration formatcloudposse/stack-config/yaml//modules/remote-state, which callsdata "utils_component_config"internally — in one affected repo, this impacts 56 componentspkg/describeinstead ofpkg/componentbecauseinternal/execnow importspkg/component(for theComponentProviderinterface andComponentRegistry), which would create an import cyclereferences
docs/fixes/2026-02-15-restore-component-processor-public-api.mdpkg/component/component_processor.go)cloudposse/stack-config/yaml//modules/remote-statev1.8.0Summary by CodeRabbit
New Features
Documentation
Tests