Merge pull request #5049 from vinayakankugoyal/deps

KEP-Template: Fix KEP validation to correctly handle deprecated->disabled->removed releases.
kubernetes · Jan 18, 2025 · a8e9e87 · a8e9e87
2 parents 1d5ad90 + 7cd88e6
commit a8e9e87
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 4 deletions.
diff --git a/api/approval.go b/api/approval.go
@@ -62,7 +62,10 @@ func (prr *PRRApproval) ApproverForStage(stage Stage) (string, error) {
 		return "", err
 	}
 
-	if prr.Alpha == nil && prr.Beta == nil && prr.Stable == nil {
+	// KEPs are usually of 2 types:
+	// 1. Features that go through Alpha->Beta->Stable
+	// 2. Removals that go through Deprecated->Disabled->Removed
+	if prr.Alpha == nil && prr.Beta == nil && prr.Stable == nil && prr.Deprecated == nil && prr.Disabled == nil && prr.Removed == nil {
 		return "", ErrPRRMilestonesAllEmpty
 	}
 
@@ -71,20 +74,37 @@ func (prr *PRRApproval) ApproverForStage(stage Stage) (string, error) {
 		if prr.Alpha == nil {
 			return "", ErrPRRMilestoneIsNil
 		}
-
 		return prr.Alpha.Approver, nil
+
 	case BetaStage:
 		if prr.Beta == nil {
 			return "", ErrPRRMilestoneIsNil
 		}
-
 		return prr.Beta.Approver, nil
+
 	case StableStage:
 		if prr.Stable == nil {
 			return "", ErrPRRMilestoneIsNil
 		}
-
 		return prr.Stable.Approver, nil
+
+	case Deprecated:
+		if prr.Deprecated == nil {
+			return "", ErrPRRMilestoneIsNil
+		}
+		return prr.Deprecated.Approver, nil
+
+	case Disabled:
+		if prr.Disabled == nil {
+			return "", ErrPRRMilestoneIsNil
+		}
+		return prr.Disabled.Approver, nil
+
+	case Removed:
+		if prr.Removed == nil {
+			return "", ErrPRRMilestoneIsNil
+		}
+		return prr.Removed.Approver, nil
 	}
 
 	return "", ErrPRRApproverUnknown

diff --git a/api/approval_test.go b/api/approval_test.go
@@ -40,6 +40,18 @@ var (
 		Stable: prrMilestoneWithApprover,
 	}
 
+	validDeprecatedPRR = &api.PRRApproval{
+		Deprecated: prrMilestoneWithApprover,
+	}
+
+	validDisabledPRR = &api.PRRApproval{
+		Disabled: prrMilestoneWithApprover,
+	}
+
+	validRemovedPRR = &api.PRRApproval{
+		Removed: prrMilestoneWithApprover,
+	}
+
 	invalidPRR = &api.PRRApproval{}
 )
 
@@ -85,6 +97,27 @@ func TestPRRApproval_ApproverForStage(t *testing.T) {
 			wantApprover: "@wojtek-t",
 			wantErr:      false,
 		},
+		{
+			name:         "valid: deprecated",
+			stage:        "deprecated",
+			prr:          validDeprecatedPRR,
+			wantApprover: "@wojtek-t",
+			wantErr:      false,
+		},
+		{
+			name:         "valid: disabled",
+			stage:        "disabled",
+			prr:          validDisabledPRR,
+			wantApprover: "@wojtek-t",
+			wantErr:      false,
+		},
+		{
+			name:         "valid: removed",
+			stage:        "removed",
+			prr:          validRemovedPRR,
+			wantApprover: "@wojtek-t",
+			wantErr:      false,
+		},
 	}
 
 	for _, tc := range testcases {