Merge pull request kubernetes#2991 from spiffxp/add-stage-status

api: add Stage and Status types, require more fields

Author: k8s-ci-robot

api/approval.go

@@ -54,30 +54,29 @@ func (prr *PRRApproval) Validate() error {
 	return nil
 }
 
-func (prr *PRRApproval) ApproverForStage(stage string) (string, error) {
-	isValidStage := IsOneOf(stage, ValidStages)
-	if !isValidStage {
-		return "", ErrKEPStageIsInvalid(stage)
+func (prr *PRRApproval) ApproverForStage(stage Stage) (string, error) {
+	if err := stage.IsValid(); err != nil {
+		return "", err
 	}
 
 	if prr.Alpha == nil && prr.Beta == nil && prr.Stable == nil {
 		return "", ErrPRRMilestonesAllEmpty
 	}
 
 	switch stage {
-	case "alpha":
+	case AlphaStage:
 		if prr.Alpha == nil {
 			return "", ErrPRRMilestoneIsNil
 		}
 
 		return prr.Alpha.Approver, nil
-	case "beta":
+	case BetaStage:
 		if prr.Beta == nil {
 			return "", ErrPRRMilestoneIsNil
 		}
 
 		return prr.Beta.Approver, nil
-	case "stable":
+	case StableStage:
 		if prr.Stable == nil {
 			return "", ErrPRRMilestoneIsNil
 		}

api/approval_test.go

@@ -46,7 +46,7 @@ var (
 func TestPRRApproval_ApproverForStage(t *testing.T) {
 	testcases := []struct {
 		name         string
-		stage        string
+		stage        api.Stage
 		prr          *api.PRRApproval
 		wantApprover string
 		wantErr      bool

api/error.go

@@ -17,8 +17,6 @@ limitations under the License.
 package api
 
 import (
-	"fmt"
-
 	"github.com/pkg/errors"
 )
 
@@ -37,14 +35,3 @@ var (
 		"an unknown error occurred while trying to determine a PRR approver",
 	)
 )
-
-// KEP errors
-func ErrKEPStageIsInvalid(stage string) error {
-	return errors.New(
-		fmt.Sprintf(
-			"the specified stage (%s) should be one of the following: %v",
-			stage,
-			ValidStages,
-		),
-	)
-}

api/proposal.go

@@ -30,10 +30,58 @@ import (
 	"k8s.io/enhancements/pkg/yaml"
 )
 
-var ValidStages = []string{
-	"alpha",
-	"beta",
-	"stable",
+type Stage string
+
+const (
+	AlphaStage  Stage = "alpha"
+	BetaStage   Stage = "beta"
+	StableStage Stage = "stable"
+)
+
+var ValidStages = []Stage{
+	AlphaStage,
+	BetaStage,
+	StableStage,
+}
+
+func (s Stage) IsValid() error {
+	for _, s2 := range ValidStages {
+		if s == s2 {
+			return nil
+		}
+	}
+	return fmt.Errorf("invalid stage: %v, should be one of %v", s, ValidStages)
+}
+
+type Status string
+
+const (
+	ProvisionalStatus   Status = "provisional"
+	ImplementableStatus Status = "implementable"
+	ImplementedStatus   Status = "implemented"
+	DeferredStatus      Status = "deferred"
+	RejectedStatus      Status = "rejected"
+	WithdrawnStatus     Status = "withdrawn"
+	ReplacedStatus      Status = "replaced"
+)
+
+var ValidStatuses = []Status{
+	ProvisionalStatus,
+	ImplementableStatus,
+	ImplementedStatus,
+	DeferredStatus,
+	RejectedStatus,
+	WithdrawnStatus,
+	ReplacedStatus,
+}
+
+func (s Status) IsValid() error {
+	for _, s2 := range ValidStatuses {
+		if s == s2 {
+			return nil
+		}
+	}
+	return fmt.Errorf("invalid status: %v, should be one of %v", s, ValidStatuses)
 }
 
 type Proposals []*Proposal
@@ -50,21 +98,21 @@ type Proposal struct {
 
 	Title             string   `json:"title" yaml:"title" validate:"required"`
 	Number            string   `json:"kep-number" yaml:"kep-number" validate:"required"`
-	Authors           []string `json:"authors" yaml:",flow"`
+	Authors           []string `json:"authors" yaml:",flow" validate:"required"`
 	OwningSIG         string   `json:"owningSig" yaml:"owning-sig" validate:"required"`
 	ParticipatingSIGs []string `json:"participatingSigs" yaml:"participating-sigs,flow,omitempty"`
 	Reviewers         []string `json:"reviewers" yaml:",flow"`
-	Approvers         []string `json:"approvers" yaml:",flow"`
+	Approvers         []string `json:"approvers" yaml:",flow" validate:"required"`
 	PRRApprovers      []string `json:"prrApprovers" yaml:"prr-approvers,flow"`
 	Editor            string   `json:"editor" yaml:"editor,omitempty"`
 	CreationDate      string   `json:"creationDate" yaml:"creation-date"`
 	LastUpdated       string   `json:"lastUpdated" yaml:"last-updated"`
-	Status            string   `json:"status" yaml:"status" validate:"required"`
+	Status            Status   `json:"status" yaml:"status" validate:"required"`
 	SeeAlso           []string `json:"seeAlso" yaml:"see-also,omitempty"`
 	Replaces          []string `json:"replaces" yaml:"replaces,omitempty"`
 	SupersededBy      []string `json:"supersededBy" yaml:"superseded-by,omitempty"`
 
-	Stage           string    `json:"stage" yaml:"stage"`
+	Stage           Stage     `json:"stage" yaml:"stage"`
 	LatestMilestone string    `json:"latestMilestone" yaml:"latest-milestone"`
 	Milestone       Milestone `json:"milestone" yaml:"milestone"`
 
@@ -85,6 +133,18 @@ func (p *Proposal) IsMissingStage() bool {
 	return p.Stage == ""
 }
 
+type Milestone struct {
+	Alpha      string `json:"alpha" yaml:"alpha"`
+	Beta       string `json:"beta" yaml:"beta"`
+	Stable     string `json:"stable" yaml:"stable"`
+	Deprecated string `json:"deprecated" yaml:"deprecated,omitempty"`
+	Removed    string `json:"removed" yaml:"removed,omitempty"`
+}
+
+type FeatureGate struct {
+	Name       string   `json:"name" yaml:"name"`
+	Components []string `json:"components" yaml:"components"`
+}
 type KEPHandler Parser
 
 // TODO(api): Make this a generic parser for all `Document` types
@@ -191,22 +251,15 @@ func (k *KEPHandler) Validate(p *Proposal) []error {
 	if errs := k.validatePRRApprovers(p); errs != nil {
 		allErrs = append(allErrs, errs...)
 	}
+	if err := p.Status.IsValid(); err != nil {
+		allErrs = append(allErrs, err)
+	}
+	if err := p.Stage.IsValid(); err != nil {
+		allErrs = append(allErrs, err)
+	}
 	return allErrs
 }
 
-type Milestone struct {
-	Alpha      string `json:"alpha" yaml:"alpha"`
-	Beta       string `json:"beta" yaml:"beta"`
-	Stable     string `json:"stable" yaml:"stable"`
-	Deprecated string `json:"deprecated" yaml:"deprecated,omitempty"`
-	Removed    string `json:"removed" yaml:"removed,omitempty"`
-}
-
-type FeatureGate struct {
-	Name       string   `json:"name" yaml:"name"`
-	Components []string `json:"components" yaml:"components"`
-}
-
 func hash(s string) string {
 	return fmt.Sprintf("%x", md5.Sum([]byte(s)))
 }

keps/sig-network/2449-move-externalDNS-out-of-kubernetes-incubator/kep.yaml

@@ -2,5 +2,8 @@ title: Move ExternalDNS out of Kubernetes incubator
 kep-number: 2449
 authors:
   - "@njuettner"
+approvers:
+  - bowei
+  - thockin
 owning-sig: sig-network
 status: implemented

keps/sig-scheduling/964-binpacking-priority/kep.yaml

@@ -1,6 +1,8 @@
 title: "Extending RequestedToCapacityRatio Priority Function to support Resource Bin Packing of Extended Resources - @sudeshsh"
 kep-number: 964
 owning-sig: sig-scheduling
+authors:
+  - sudeshsh
 participating-sigs:
   - sig-scheduling
 reviewers:

pkg/kepctl/commands/create.go

@@ -20,6 +20,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
+	"k8s.io/enhancements/api"
 	"k8s.io/enhancements/pkg/proposal"
 	"k8s.io/enhancements/pkg/repo"
 )
@@ -89,7 +90,7 @@ func addCreate(topLevel *cobra.Command) {
 		&co.State,
 		"state",
 		"s",
-		"provisional",
+		string(api.ProvisionalStatus),
 		"KEP State",
 	)
 

pkg/kepval/approval.go

@@ -100,15 +100,10 @@ func ValidatePRR(kep *api.Proposal, h *api.PRRHandler, prrDir string) error {
 func isPRRRequired(kep *api.Proposal) (required, missingMilestone, missingStage bool, err error) {
 	logrus.Debug("checking if PRR is required")
 
-	required = true
+	required = kep.Status == api.ImplementableStatus
 	missingMilestone = kep.IsMissingMilestone()
 	missingStage = kep.IsMissingStage()
 
-	if kep.Status != "implementable" {
-		required = false
-		return required, missingMilestone, missingStage, nil
-	}
-
 	if missingMilestone {
 		logrus.Warnf("Missing the latest milestone field: %s", kep.Filename)
 		return required, missingMilestone, missingStage, nil

pkg/output/output.go

@@ -210,8 +210,8 @@ func DefaultPrintConfigs(names ...string) []PrintConfig {
 
 			return k.OwningSIG
 		}},
-		"Stage":  {"Stage", func(k *api.Proposal) string { return k.Stage }},
-		"Status": {"Status", func(k *api.Proposal) string { return k.Status }},
+		"Stage":  {"Stage", func(k *api.Proposal) string { return string(k.Stage) }},
+		"Status": {"Status", func(k *api.Proposal) string { return string(k.Status) }},
 		"Title": {"Title", func(k *api.Proposal) string {
 			if k.PRNumber == "" {
 				return k.Title

pkg/proposal/create.go

@@ -124,7 +124,7 @@ func populateProposal(p *api.Proposal, opts *CreateOpts) {
 	p.Name = opts.Name
 
 	if opts.State != "" {
-		p.Status = opts.State
+		p.Status = api.Status(opts.State)
 	}
 
 	now := time.Now()