fix: workflows using the same job agent syntax as deployments#55
fix: workflows using the same job agent syntax as deployments#55
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR refactors the job_agent configuration schema from a generic Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 34 minutes and 28 seconds.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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/resources/workflow.md`:
- Around line 40-46: Update the Go schema Description fields for the job_agent
resource so the generated docs clearly state that exactly one nested dispatch
block must be chosen: modify the Description on the job_agent Schema (and/or the
parent resource schema builder function that defines job_agent) to read
something like “Exactly one of the following blocks must be set: argo_workflow,
argocd, github, terraform_cloud, test_runner” and also append a short
mutually-exclusive note to each nested block’s Description (argo_workflow,
argocd, github, terraform_cloud, test_runner) so terraform-plugin-docs will emit
the exactly-one-of constraint in the generated markdown; leave validation logic
as-is.
In `@internal/provider/job_agent_dispatch_config.go`:
- Around line 373-406: The current inferDispatchBlockType is too permissive and
can misclassify partial configs; update it so it only returns a type when
unambiguous discriminator fields are present (otherwise return ""), i.e.
keep/require only truly unique selectors: for github only return "github" when
gh.Owner, gh.Repo, gh.InstallationId, or gh.WorkflowId are set; for
terraform_cloud only when tfc.Organization, tfc.Address, or
tfc.TriggerRunOnChange are set; for test_runner only when tr.DelaySeconds or
tr.Status are set; for argo_workflow only when aw.WebhookSecret,
aw.HttpInsecure, or aw.Name are set; for argocd only when ac.ServerUrl,
ac.Template, or ac.ApiKey are set—do not attempt to infer from ambiguous
single-field payloads like github.ref, test_runner.message, or
terraform_cloud.token, and return "" when none of these unambiguous
discriminators match.
In `@internal/provider/workflow_resource.go`:
- Around line 359-377: priorByName currently collapses duplicate job_agent
entries by using only Name as the map key; either enforce Name uniqueness in
validation or preserve prior state by using a non-colliding key (e.g., composite
key or index). Fix by replacing priorByName with a map keyed by a composite of
unique fields (for example Name+Ref+Selector or Name+Ref+Selector+index) when
building the prior map from data.JobAgents, and look up using the same composite
key when iterating w.JobAgents before calling
setJobAgentDispatchBlocksFromConfig and agent.setDispatchBlocks; alternatively,
if Name is intended to be unique, add validation logic to enforce uniqueness of
Name in the resource schema and keep the existing lookup logic.
🪄 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: c957970f-b1b3-4be6-a4fd-548f71ae8c5e
📒 Files selected for processing (6)
docs/resources/workflow.mdexamples/resources/workflow/workflows.tfinternal/provider/deployment_resource.gointernal/provider/job_agent_dispatch_config.gointernal/provider/workflow_resource.gointernal/provider/workflow_resource_test.go
| Optional: | ||
|
|
||
| - `argo_workflow` (Block, Optional) Argo Workflow job agent configuration (see [below for nested schema](#nestedblock--job_agent--argo_workflow)) | ||
| - `argocd` (Block, Optional) ArgoCD job agent configuration (see [below for nested schema](#nestedblock--job_agent--argocd)) | ||
| - `github` (Block, Optional) GitHub job agent configuration (see [below for nested schema](#nestedblock--job_agent--github)) | ||
| - `terraform_cloud` (Block, Optional) Terraform Cloud job agent configuration (see [below for nested schema](#nestedblock--job_agent--terraform_cloud)) | ||
| - `test_runner` (Block, Optional) Test runner job agent configuration (see [below for nested schema](#nestedblock--job_agent--test_runner)) |
There was a problem hiding this comment.
Document that job_agent must choose exactly one dispatch block.
These generated docs currently read as if all five nested blocks are independently optional, but the provider rejects both zero and multiple selections. Please surface the exactly-one-of constraint through the schema descriptions so users see it in the published docs instead of only at plan time.
Based on learnings docs/resources/policy.md is auto-generated by terraform-plugin-docs, so this needs to be fixed in the Go schema Description fields rather than by editing the generated markdown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/resources/workflow.md` around lines 40 - 46, Update the Go schema
Description fields for the job_agent resource so the generated docs clearly
state that exactly one nested dispatch block must be chosen: modify the
Description on the job_agent Schema (and/or the parent resource schema builder
function that defines job_agent) to read something like “Exactly one of the
following blocks must be set: argo_workflow, argocd, github, terraform_cloud,
test_runner” and also append a short mutually-exclusive note to each nested
block’s Description (argo_workflow, argocd, github, terraform_cloud,
test_runner) so terraform-plugin-docs will emit the exactly-one-of constraint in
the generated markdown; leave validation logic as-is.
| func inferDispatchBlockType(config map[string]interface{}) string { | ||
| data, err := json.Marshal(config) | ||
| if err != nil { | ||
| return "" | ||
| } | ||
|
|
||
| var gh githubConfig | ||
| _ = json.Unmarshal(data, &gh) | ||
| if gh.Owner != "" || gh.Repo != "" || gh.InstallationId != nil || gh.WorkflowId != nil { | ||
| return "github" | ||
| } | ||
|
|
||
| var tfc terraformCloudConfig | ||
| _ = json.Unmarshal(data, &tfc) | ||
| if tfc.Organization != "" || tfc.Address != "" || tfc.TriggerRunOnChange != nil { | ||
| return "terraform_cloud" | ||
| } | ||
|
|
||
| var tr testRunnerConfig | ||
| _ = json.Unmarshal(data, &tr) | ||
| if tr.DelaySeconds != nil || tr.Status != "" { | ||
| return "test_runner" | ||
| } | ||
|
|
||
| var aw argoWorkflowConfig | ||
| _ = json.Unmarshal(data, &aw) | ||
| if aw.WebhookSecret != "" || aw.HttpInsecure != nil || aw.Name != "" { | ||
| return "argo_workflow" | ||
| } | ||
|
|
||
| var ac argoCDConfig | ||
| _ = json.Unmarshal(data, &ac) | ||
| if ac.ServerUrl != "" || ac.Template != "" || ac.ApiKey != "" { | ||
| return "argocd" |
There was a problem hiding this comment.
The fallback block-type inference is unsafe for partial configs.
When there is no prior state, this heuristic misses valid one-field payloads like github.ref, test_runner.message, and terraform_cloud.token, and it can also misclassify Argo Workflow payloads that only use shared keys (apiKey/serverUrl/template) as argocd. That means import/first read can hydrate the wrong block or none at all. The safer fallback is to infer only from unambiguous discriminator fields and otherwise leave the block unset until config provides the type.
Based on learnings the deployment Read path intentionally produces empty nested blocks after terraform import when no prior state exists, so falling back to “unset” on ambiguous payloads is already an accepted pattern in this repository.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/provider/job_agent_dispatch_config.go` around lines 373 - 406, The
current inferDispatchBlockType is too permissive and can misclassify partial
configs; update it so it only returns a type when unambiguous discriminator
fields are present (otherwise return ""), i.e. keep/require only truly unique
selectors: for github only return "github" when gh.Owner, gh.Repo,
gh.InstallationId, or gh.WorkflowId are set; for terraform_cloud only when
tfc.Organization, tfc.Address, or tfc.TriggerRunOnChange are set; for
test_runner only when tr.DelaySeconds or tr.Status are set; for argo_workflow
only when aw.WebhookSecret, aw.HttpInsecure, or aw.Name are set; for argocd only
when ac.ServerUrl, ac.Template, or ac.ApiKey are set—do not attempt to infer
from ambiguous single-field payloads like github.ref, test_runner.message, or
terraform_cloud.token, and return "" when none of these unambiguous
discriminators match.
1 participant
Summary by CodeRabbit
Documentation
job_agentblock schema with provider-specific nested block options.Refactor
job_agentconfiguration changed:configattribute removed in favor of explicit nested blocks forargo_workflow,argocd,github,terraform_cloud, andtest_runner.job_agentrequired fields toname,ref, andselector.