From 7cd88e6c36ea19044e055038ec2e2efb33827885 Mon Sep 17 00:00:00 2001
From: Vinayak Goyal <vinaygo@google.com>
Date: Fri, 17 Jan 2025 21:54:42 +0000
Subject: [PATCH] Fix KEP validation to correctly handle
 deprecated->disabled->removed releases.

---
 api/approval.go      | 28 ++++++++++++++++++++++++----
 api/approval_test.go | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/api/approval.go b/api/approval.go
index 256a32ebf79..41502ed9c79 100644
--- 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
index 62fae997cf9..fe62f113834 100644
--- 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 {