fix(engines): ensure platform libraries can't be defined multiple times#90
fix(engines): ensure platform libraries can't be defined multiple times#90
Conversation
WalkthroughThis PR introduces typed identifiers for libraries and plugins in the Terraform engine. It adds libraryID and libraryVersion types, replaces PlatformSpec.Libraries keys/values accordingly, and updates GetLibrary/GetLibraries signatures to use libraryID. A new pluginSource type is added, and ResourceBlueprint now uses Source instead of PluginId, with ResolvePlugin updated to validate and resolve via the new types. Additional nil checks and stricter error handling are added in TerraformEngine and platform code. Error messages across deployment, resource handling, and plugin resolution paths are expanded to include contextual details. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks❌ Failed checks (3 warnings)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
engines/terraform/terraform.go (1)
59-62: Good nil-guard; prevents panic on malformed specs.Consider mirroring this guard in resolveIdentityPlugin for consistency.
engines/terraform/resource_handler.go (1)
16-19: Runtime nil-check prevents crash. Also validate storage plugin runtimes.Add a similar guard inside the storage loop to avoid panics when a storage plugin lacks Runtime.
for name, plug := range storagePlugins { + if plug.Runtime == nil { + return nil, fmt.Errorf("storage plugin %s has no runtime configuration", name) + } pluginDef.Storage = append(pluginDef.Storage, plugin.GoPlugin{ Alias: fmt.Sprintf("storage_%s", name), Name: name, Import: strings.Split(plug.Runtime.GoModule, "@")[0], }) gets = append(gets, plug.Runtime.GoModule) }Optional: dedupe gets to avoid duplicate module fetches (nice-to-have).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
engines/terraform/deployment.go(3 hunks)engines/terraform/platform.go(4 hunks)engines/terraform/resource_handler.go(5 hunks)engines/terraform/terraform.go(2 hunks)
🔇 Additional comments (9)
engines/terraform/terraform.go (1)
81-81: Clear error wrapping with context.Good use of %w and inclusion of type/intent.
engines/terraform/resource_handler.go (5)
62-63: Better error context for service identities.Message is consistent with the new wrapping pattern.
84-85: Good contextual error for service resource plugin resolution.
122-123: Good contextual error for bucket plugin resolution.
176-177: Good contextual error for entrypoint plugin resolution.
206-208: Good contextual error for database plugin resolution.engines/terraform/deployment.go (3)
70-71: Helpful error wrapping for infra plugin resolution.Keeps infra name in the error path.
88-89: Clearer error when route target missing.Nice inclusion of entrypoint and target names.
98-99: Clearer error for invalid target type.Consistent with other contextual errors.
|
🎉 This PR is included in version 0.1.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.