feat: workflow resource provider#39
Merged
adityachoudhari26 merged 4 commits intomainfrom Mar 31, 2026
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRefactors workflow models from job templates/matrices to job agents with selectors, adds release verification/result models and new API operations (UpsertResourceByIdentifier, CreateWorkflowRun), updates client response types, and introduces a Terraform ctrlplane_workflow resource with full CRUD, import support, docs, examples, and acceptance tests. Changes
Sequence DiagramsequenceDiagram
participant Terraform as Terraform CLI
participant Provider as Provider Plugin
participant APIClient as API Client
participant Server as API Server
Terraform->>Provider: Apply (create/update workflow)
Provider->>Provider: parse inputs, build JobAgents (with selector)
Provider->>APIClient: CreateWorkflowWithResponse / UpdateWorkflowWithResponse
APIClient->>Server: POST/PUT /workflows
Server-->>APIClient: 201/202 + Workflow JSON
APIClient-->>Provider: Parsed response
Provider->>Provider: setWorkflowModelFromAPI (serialize inputs)
Provider-->>Terraform: Persist state (id, name, inputs, job_agents)
Terraform->>Provider: Refresh (read)
Provider->>APIClient: GetWorkflowWithResponse
APIClient->>Server: GET /workflows/{id}
Server-->>APIClient: 200 OK + Workflow
APIClient-->>Provider: Parsed response
Provider-->>Terraform: Update state
Terraform->>Provider: Destroy (delete)
Provider->>APIClient: DeleteWorkflowWithResponse
APIClient->>Server: DELETE /workflows/{id}
Server-->>APIClient: 204 NoContent / 202 Accepted
APIClient-->>Provider: Response
Provider-->>Terraform: Remove resource from state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/provider/workflow_resource.go (1)
290-295: Silent fallback on JSON marshal error could hide data corruption.If
json.Marshal(w.Inputs)fails, the code silently falls back to"[]", potentially losing workflow input data. While marshaling a slice rarely fails, consider logging this edge case.Add logging for marshal failure
inputsJSON, err := json.Marshal(w.Inputs) if err != nil { + // This shouldn't happen but log for debugging if it does data.Inputs = types.StringValue("[]") } else { data.Inputs = types.StringValue(string(inputsJSON)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/provider/workflow_resource.go` around lines 290 - 295, When json.Marshal(w.Inputs) fails the code silently assigns data.Inputs = types.StringValue("[]") losing visibility into the failure; update the error branch inside the inputsJSON handling to log the marshal error (including the err value and context such as w.Inputs) before falling back, e.g. call your package logger or log.Printf with a message like "failed to marshal w.Inputs" and the error, then keep the existing fallback to data.Inputs = types.StringValue("[]") so the rest of the flow continues; modify the block around json.Marshal, inputsJSON, w.Inputs and data.Inputs to add that logging.internal/provider/workflow_resource_test.go (1)
72-81: Consider adding test coverage for theinputsattribute.The test configuration exercises
job_agentblocks but does not cover the optionalinputsJSON attribute. Adding a test step with inputs would provide more comprehensive coverage.Example addition to test inputs
resource "ctrlplane_workflow" "test" { name = %q + inputs = jsonencode([ + { name = "env", type = "string", default = "dev" } + ]) job_agent {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/provider/workflow_resource_test.go` around lines 72 - 81, Add a test case that includes the optional inputs JSON attribute on the ctrlplane_workflow resource so the job_agent inputs path is exercised: update the resource block (ctrlplane_workflow.test) in workflow_resource_test.go to include an inputs = jsonencode(...) or inputs = "{\"key\":\"value\"}" on the job_agent (or on the workflow as appropriate), then extend the test assertions to verify the provider parsed/stored those inputs (check the provider state or function that reads inputs). Target the ctrlplane_workflow.test resource and the job_agent block in your changes and add an assertion that the inputs value equals the expected JSON string/object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/client.gen.go`:
- Around line 860-877: The ReleaseTargetStateResponse currently returns full
VerificationMetricStatus including embedded MetricProvider and raw measurement
payloads which can leak secrets (e.g., Datadog.ApiKey, Prometheus OAuth client
secret, Terraform Cloud token); update the OpenAPI response schema / server
payload so that ReleaseTargetStateResponse.latestJob.verifications (and the
VerificationMetricStatus schema) only expose a redacted provider
summary/reference (e.g., provider type/id and non-sensitive metadata) and a
status/metrics summary, and remove raw measurement payload fields and any nested
MetricProvider credential fields from the response model; regenerate the client
so types like ReleaseTargetStateResponse, VerificationMetricStatus and any
MetricProvider variants no longer include credential-bearing fields or raw
measurement data.
In `@internal/provider/workflow_resource.go`:
- Around line 25-27: The provider's Resources() list is missing the new resource
so ctrlplane_workflow won't be exposed; update the provider Resources()
implementation to register the resource by adding an entry mapping the Terraform
type string (e.g., "ctrlplane_workflow") to the constructor
NewWorkflowResource() so NewWorkflowResource is returned from Resources()
alongside the other resource constructors (ensure the Resources() function in
provider.go includes the "ctrlplane_workflow": NewWorkflowResource() entry).
- Around line 269-275: The code silently discards the error from
a.Config.ElementsAs which can hide configuration problems; change the call to
capture the error (err := a.Config.ElementsAs(context.Background(), &decoded,
false)), and if err != nil log the failure with a clear message and the error
(using the package's logger, e.g. r.logger.Errorf or log.Printf) instead of
ignoring it, otherwise proceed to loop over decoded into config; reference
a.Config, ElementsAs, decoded and config when making the change.
---
Nitpick comments:
In `@internal/provider/workflow_resource_test.go`:
- Around line 72-81: Add a test case that includes the optional inputs JSON
attribute on the ctrlplane_workflow resource so the job_agent inputs path is
exercised: update the resource block (ctrlplane_workflow.test) in
workflow_resource_test.go to include an inputs = jsonencode(...) or inputs =
"{\"key\":\"value\"}" on the job_agent (or on the workflow as appropriate), then
extend the test assertions to verify the provider parsed/stored those inputs
(check the provider state or function that reads inputs). Target the
ctrlplane_workflow.test resource and the job_agent block in your changes and add
an assertion that the inputs value equals the expected JSON string/object.
In `@internal/provider/workflow_resource.go`:
- Around line 290-295: When json.Marshal(w.Inputs) fails the code silently
assigns data.Inputs = types.StringValue("[]") losing visibility into the
failure; update the error branch inside the inputsJSON handling to log the
marshal error (including the err value and context such as w.Inputs) before
falling back, e.g. call your package logger or log.Printf with a message like
"failed to marshal w.Inputs" and the error, then keep the existing fallback to
data.Inputs = types.StringValue("[]") so the rest of the flow continues; modify
the block around json.Marshal, inputsJSON, w.Inputs and data.Inputs to add that
logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 723407a7-bc2e-452f-b4e4-258a8353b723
📒 Files selected for processing (3)
internal/api/client.gen.gointernal/provider/workflow_resource.gointernal/provider/workflow_resource_test.go
Comment on lines
+860
to
+877
| // ReleaseTargetStateResponse defines model for ReleaseTargetStateResponse. | ||
| type ReleaseTargetStateResponse struct { | ||
| CurrentRelease *Release `json:"currentRelease,omitempty"` | ||
| DesiredRelease *Release `json:"desiredRelease,omitempty"` | ||
| LatestJob *struct { | ||
| Job Job `json:"job"` | ||
| Verifications []struct { | ||
| CreatedAt time.Time `json:"createdAt"` | ||
| Id string `json:"id"` | ||
| JobId string `json:"jobId"` | ||
| Message *string `json:"message,omitempty"` | ||
| Metrics []VerificationMetricStatus `json:"metrics"` | ||
|
|
||
| // Status Computed aggregate status of this verification | ||
| Status ReleaseTargetStateResponseLatestJobVerificationsStatus `json:"status"` | ||
| } `json:"verifications"` | ||
| } `json:"latestJob,omitempty"` | ||
| } |
There was a problem hiding this comment.
Avoid returning secret-bearing verification details from read responses.
Line 1281 embeds the full MetricProvider into VerificationMetricStatus, and several provider variants in this file carry credentials (DatadogMetricProvider.ApiKey/AppKey, PrometheusMetricProvider.Authentication.Oauth2.ClientSecret, TerraformCloudRunMetricProvider.Token). Combined with the new ReleaseTargetStateResponse on Lines 860-877, a release-target state read can now surface verification credentials into client memory, logs, or Terraform state. Line 1222 also exposes raw measurement payloads on the default read path. Please switch this response model to a redacted provider summary/reference and keep raw measurement data out of the standard state endpoint. Since this file is generated, the fix needs to happen in the OpenAPI response schema/server payload before regenerating.
Also applies to: 1219-1282
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/client.gen.go` around lines 860 - 877, The
ReleaseTargetStateResponse currently returns full VerificationMetricStatus
including embedded MetricProvider and raw measurement payloads which can leak
secrets (e.g., Datadog.ApiKey, Prometheus OAuth client secret, Terraform Cloud
token); update the OpenAPI response schema / server payload so that
ReleaseTargetStateResponse.latestJob.verifications (and the
VerificationMetricStatus schema) only expose a redacted provider
summary/reference (e.g., provider type/id and non-sensitive metadata) and a
status/metrics summary, and remove raw measurement payload fields and any nested
MetricProvider credential fields from the response model; regenerate the client
so types like ReleaseTargetStateResponse, VerificationMetricStatus and any
MetricProvider variants no longer include credential-bearing fields or raw
measurement data.
Comment on lines
+269
to
+275
| if !a.Config.IsNull() && !a.Config.IsUnknown() { | ||
| var decoded map[string]string | ||
| _ = a.Config.ElementsAs(context.Background(), &decoded, false) | ||
| for k, v := range decoded { | ||
| config[k] = v | ||
| } | ||
| } |
There was a problem hiding this comment.
Silently ignoring ElementsAs error could hide configuration issues.
If ElementsAs fails, the error is discarded and an empty config map is used. This could lead to unexpected behavior where the API receives incomplete configuration without any indication of the problem.
Suggested fix to log the error
- _ = a.Config.ElementsAs(context.Background(), &decoded, false)
+ diags := a.Config.ElementsAs(context.Background(), &decoded, false)
+ if diags.HasError() {
+ // Log or handle - for now, proceed with empty config
+ // but this indicates a schema/type mismatch
+ }📝 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.
Suggested change
| if !a.Config.IsNull() && !a.Config.IsUnknown() { | |
| var decoded map[string]string | |
| _ = a.Config.ElementsAs(context.Background(), &decoded, false) | |
| for k, v := range decoded { | |
| config[k] = v | |
| } | |
| } | |
| if !a.Config.IsNull() && !a.Config.IsUnknown() { | |
| var decoded map[string]string | |
| diags := a.Config.ElementsAs(context.Background(), &decoded, false) | |
| if diags.HasError() { | |
| // Log or handle - for now, proceed with empty config | |
| // but this indicates a schema/type mismatch | |
| } | |
| for k, v := range decoded { | |
| config[k] = v | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/provider/workflow_resource.go` around lines 269 - 275, The code
silently discards the error from a.Config.ElementsAs which can hide
configuration problems; change the call to capture the error (err :=
a.Config.ElementsAs(context.Background(), &decoded, false)), and if err != nil
log the failure with a clear message and the error (using the package's logger,
e.g. r.logger.Errorf or log.Printf) instead of ignoring it, otherwise proceed to
loop over decoded into config; reference a.Config, ElementsAs, decoded and
config when making the change.
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
1 participant
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.
Summary by CodeRabbit
New Features
Tests
Documentation