Merged
Conversation
📝 WalkthroughWalkthroughThe PR corrects a misspelled field name from "sock time" to "soak time" across documentation, API client models, and Terraform provider schemas. Additionally, it restructures deployment plan target modeling, removes deployment job agent fields, and introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/provider/policy_resource.go (1)
432-437:⚠️ Potential issue | 🟠 MajorPreserve backward compatibility for
minimum_sock_time_minutesduring migration.This rename is internally consistent, but it is a breaking Terraform surface change: existing configurations using
minimum_sock_time_minuteswill fail with unsupported argument after upgrade. Please keep the old key as a deprecated alias for at least one release and map both keys toMinimumSoakTimeMinutesin API payload generation.Proposed compatibility patch (schema + model + mapping)
@@ - "minimum_soak_time_minutes": schema.Int64Attribute{ + "minimum_soak_time_minutes": schema.Int64Attribute{ Optional: true, Computed: true, Description: "Minimum time in minutes to wait after the dependency environment is in a success state", Default: int64default.StaticInt64(0), }, + "minimum_sock_time_minutes": schema.Int64Attribute{ + Optional: true, + Computed: true, + Description: "Deprecated: use minimum_soak_time_minutes", + Default: int64default.StaticInt64(0), + }, @@ type PolicyEnvironmentProgression struct { @@ MinimumSoakTimeMinutes types.Int64 `tfsdk:"minimum_soak_time_minutes"` + MinimumSockTimeMinutes types.Int64 `tfsdk:"minimum_sock_time_minutes"` MaximumAgeHours types.Int64 `tfsdk:"maximum_age_hours"` } @@ - if int64ValueSet(progression.MinimumSoakTimeMinutes) { - val := int32(progression.MinimumSoakTimeMinutes.ValueInt64()) + soak := progression.MinimumSoakTimeMinutes + if !int64ValueSet(soak) && int64ValueSet(progression.MinimumSockTimeMinutes) { + soak = progression.MinimumSockTimeMinutes + } + if int64ValueSet(soak) { + val := int32(soak.ValueInt64()) rule.MinimumSoakTimeMinutes = &val } @@ model := PolicyEnvironmentProgression{ @@ MinimumSoakTimeMinutes: types.Int64Null(), + MinimumSockTimeMinutes: types.Int64Null(), MaximumAgeHours: types.Int64Null(), } @@ if rule.EnvironmentProgression.MinimumSoakTimeMinutes != nil { model.MinimumSoakTimeMinutes = types.Int64Value(int64(*rule.EnvironmentProgression.MinimumSoakTimeMinutes)) + model.MinimumSockTimeMinutes = model.MinimumSoakTimeMinutes }Also applies to: 818-819, 1068-1070, 1323-1331
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/provider/policy_resource.go` around lines 432 - 437, Rename change broke backward compatibility for the Terraform attribute; add a deprecated alias "minimum_sock_time_minutes" in the resource schema alongside the new "minimum_soak_time_minutes" attribute (mark it Optional, Computed, same Default and Description but include a deprecation note), update the resource/model mapping code that populates the API model field MinimumSoakTimeMinutes to read from either attribute (prefer the new key if both present) and ensure the payload generator uses model.MinimumSoakTimeMinutes only; apply the same alias addition and mapping logic to the other occurrences referenced (around the other three spots) so both keys are accepted for at least one release.internal/api/client.gen.go (1)
386-399:⚠️ Potential issue | 🔴 CriticalDon't land this
Deploymentmodel change without the provider read-path update.This regen makes
JobAgentSelectora plainstringand removes the old deployment job-agent fields.internal/provider/deployment_resource.gostill nil-checks/dereferencesdep.JobAgentSelector, and the current read flow relies on adep.JobAgentIdbranch to preserve Terraform Cloud tokens. As-is, this creates a breaking client/provider mismatch.Minimum provider-side follow-up
- if dep.JobAgentSelector != nil && *dep.JobAgentSelector != "" { - data.JobAgentSelector = types.StringValue(*dep.JobAgentSelector) + if dep.JobAgentSelector != "" { + data.JobAgentSelector = types.StringValue(dep.JobAgentSelector) } else { data.JobAgentSelector = types.StringNull() }The
dep.JobAgentId-based token-preservation path also needs a replacement in the same PR.Based on learnings: token preservation currently depends on the
dep.JobAgentIdbranch ininternal/provider/deployment_resource.go.🤖 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 386 - 399, The generated change to the Deployment model made JobAgentSelector a plain string and removed prior job-agent fields, but internal/provider/deployment_resource.go still nil-checks and dereferences dep.JobAgentSelector and relies on dep.JobAgentId to preserve tokens; update the provider read-path to match the new model by removing nil-checks for pointer fields, stop using dep.JobAgentId (or add a new replacement field) in the token-preservation branch, and implement token-preservation logic that reads the new JobAgentSelector string (or a new explicit token-preservation field) so the client and provider stay consistent (update code paths that reference dep.JobAgentSelector and dep.JobAgentId accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/api/client.gen.go`:
- Around line 386-399: The generated change to the Deployment model made
JobAgentSelector a plain string and removed prior job-agent fields, but
internal/provider/deployment_resource.go still nil-checks and dereferences
dep.JobAgentSelector and relies on dep.JobAgentId to preserve tokens; update the
provider read-path to match the new model by removing nil-checks for pointer
fields, stop using dep.JobAgentId (or add a new replacement field) in the
token-preservation branch, and implement token-preservation logic that reads the
new JobAgentSelector string (or a new explicit token-preservation field) so the
client and provider stay consistent (update code paths that reference
dep.JobAgentSelector and dep.JobAgentId accordingly).
In `@internal/provider/policy_resource.go`:
- Around line 432-437: Rename change broke backward compatibility for the
Terraform attribute; add a deprecated alias "minimum_sock_time_minutes" in the
resource schema alongside the new "minimum_soak_time_minutes" attribute (mark it
Optional, Computed, same Default and Description but include a deprecation
note), update the resource/model mapping code that populates the API model field
MinimumSoakTimeMinutes to read from either attribute (prefer the new key if both
present) and ensure the payload generator uses model.MinimumSoakTimeMinutes
only; apply the same alias addition and mapping logic to the other occurrences
referenced (around the other three spots) so both keys are accepted for at least
one release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2acdf9fc-8441-4f59-a969-525c9fc2e516
📒 Files selected for processing (3)
docs/resources/policy.mdinternal/api/client.gen.gointernal/provider/policy_resource.go
…lplane into soak-time-typo-refactor
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
Release Notes
Breaking Changes
minimum_sock_time_minutestominimum_soak_time_minutes. Update your configurations accordingly.Updates