Skip to content

feat: remove suspend state from release binding#2706

Merged
chalindukodikara merged 1 commit into
openchoreo:mainfrom
chalindukodikara:issue-remove_suspend_state
Mar 14, 2026
Merged

feat: remove suspend state from release binding#2706
chalindukodikara merged 1 commit into
openchoreo:mainfrom
chalindukodikara:issue-remove_suspend_state

Conversation

@chalindukodikara
Copy link
Copy Markdown
Contributor

Purpose

$subject

Approach

Summarize the solution and implementation details.

Related Issues

#1458

Checklist

  • Tests added or updated (unit, integration, etc.)
  • Samples updated (if applicable)

Remarks

Add any additional context, known issues, or TODOs related to this PR.

Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 14, 2026

📝 Walkthrough

File Changes Breakdown

3 files changed across 2 top-level directories:

  • api/ (1 file): api/v1alpha1/types.go
  • internal/ (2 files): internal/openchoreo-api/models/{request.go, request_test.go}

Lines changed: -18 total

  • api/v1alpha1/types.go: -8 lines (removed ReleaseStateSuspend constant + comments)
  • internal/openchoreo-api/models/request.go: +3/-4 lines (updated validation logic)
  • internal/openchoreo-api/models/request_test.go: +1/-6 lines (removed suspend test case)

API/CRD Surface Changes

Compatibility Risk: Medium

ReleaseState enum simplified from 3 to 2 valid values:

  • Removed: ReleaseStateSuspend = "Suspend" from types.go
  • Remaining: ReleaseStateActive = "Active", ReleaseStateUndeploy = "Undeploy"
  • CRD validation updated: api/v1alpha1/releasebinding_types.go already specifies +kubebuilder:validation:Enum=Active;Undeploy (confirmed - suspend not present in CRD validation)

API Request Validation:

  • UpdateBindingRequest.Validate() now restricts releaseState to exactly "Active" or "Undeploy"
  • Error message: "releaseState must be one of: Active, Undeploy"

Test Coverage

Test case removed:

  • Deleted validation test for ReleaseStateSuspend as a valid state
  • Updated error message expectation to exclude "Suspend"
  • Coverage gaps: No explicit test for rejection/error behavior when invalid suspend value is submitted

Scope Assessment

Properly scoped - cleanup of unused enum value:

Controller reconciliation logic verified:

  • Only implements explicit handling for ReleaseStateUndeploy (calls handleUndeploy())
  • No code path for suspend state identified - suggests suspend was never operationally implemented
  • Default behavior implicitly treats undeployed state as "Active" (normal deployment)

Other files referencing ReleaseState do not require changes:

  • api/v1alpha1/releasebinding_types.go: CRD validation already excludes suspend
  • internal/controller/releasebinding/: Only uses Active/Undeploy branches
  • internal/openchoreo-api/models/response.go: Stores releaseState as generic string field
  • Handlers/transforms: Do not enforce enum validation

Risk Assessment

  • Breaking Change: Low-to-Medium (suspend appears to have been non-functional)
  • Authn/Authz: No impact
  • Secrets: No impact
  • Data Migration: Not required if suspend state was never actually used in production
  • Upgrade: Safe if no existing CRs have suspend state values

Walkthrough

The changes remove the "Suspend" release state option from the ReleaseState enum. The valid states are reduced from {Active, Suspend, Undeploy} to {Active, Undeploy}, with corresponding updates to validation logic and test expectations across the codebase.

Changes

Cohort / File(s) Summary
Release State Enum Definition
api/v1alpha1/types.go
Removed ReleaseStateSuspend constant from the ReleaseState enum block.
Validation & Tests
internal/openchoreo-api/models/request.go, internal/openchoreo-api/models/request_test.go
Updated validation logic to only accept Active and Undeploy states; removed test case for Suspend state and updated corresponding error message expectations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete, containing only placeholder text ($subject) in the Purpose section and empty placeholder text in Approach and Remarks sections; the Checklist shows items are unchecked despite tests being modified. Complete the Purpose section with actual details, fill Approach with implementation summary, mark completed Checklist items as checked, and add context in Remarks if needed. Include what the Suspend state was used for and why it was removed.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: remove suspend state from release binding' accurately and specifically describes the primary change in the changeset, which removes ReleaseStateSuspend constant across three files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/openchoreo-api/models/request_test.go`:
- Around line 157-161: Add a dedicated table-driven test case asserting that the
removed "Suspend" value is rejected: in the tests slice in request_test.go (the
table that uses fields releaseState, wantErr, errMsg) add an entry with name
"Suspend invalid", releaseState: "Suspend", wantErr: true and errMsg matching
the existing message (e.g., "releaseState must be one of: Active, Undeploy");
ensure this new case is executed by the same Test... function that iterates the
tests slice so the regression is covered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3533f89d-b2e5-4e59-a688-5007b6bd48a2

📥 Commits

Reviewing files that changed from the base of the PR and between 96b196f and 269af5f.

📒 Files selected for processing (3)
  • api/v1alpha1/types.go
  • internal/openchoreo-api/models/request.go
  • internal/openchoreo-api/models/request_test.go
💤 Files with no reviewable changes (1)
  • api/v1alpha1/types.go

Comment thread internal/openchoreo-api/models/request_test.go
@chalindukodikara chalindukodikara merged commit 928a31f into openchoreo:main Mar 14, 2026
12 checks passed
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants