Merge pull request #3569 from wojtek-t/remove_prr_approvers_3

Update kep parser to get rid of prr-approvers

Author: k8s-ci-robot

api/approval.go

@@ -37,9 +37,11 @@ type PRRApproval struct {
 	Number string `json:"kepNumber" yaml:"kep-number" validate:"required"`
 
 	// TODO: Need to validate these milestone pointers are not nil
-	Alpha  *PRRMilestone `json:"alpha" yaml:"alpha,omitempty"`
-	Beta   *PRRMilestone `json:"beta" yaml:"beta,omitempty"`
-	Stable *PRRMilestone `json:"stable" yaml:"stable,omitempty"`
+	Alpha      *PRRMilestone `json:"alpha" yaml:"alpha,omitempty"`
+	Beta       *PRRMilestone `json:"beta" yaml:"beta,omitempty"`
+	Stable     *PRRMilestone `json:"stable" yaml:"stable,omitempty"`
+	Deprecated *PRRMilestone `json:"deprecated" yaml:"deprecated,omitempty"`
+	Removed    *PRRMilestone `json:"removed", yaml:"removed,omitempty"`
 
 	// TODO(api): Move to separate struct for handling document parsing
 	Error error `json:"-" yaml:"-"`

api/proposal.go

@@ -103,7 +103,6 @@ type Proposal struct {
 	ParticipatingSIGs []string `json:"participatingSigs" yaml:"participating-sigs,flow,omitempty"`
 	Reviewers         []string `json:"reviewers" 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"`
@@ -221,22 +220,6 @@ func (k *KEPHandler) validateGroups(p *Proposal) []error {
 	return errs
 }
 
-// validatePRRApprovers returns errors for each invalid PRR Approver in the
-// given Proposal, or nil if there are no invalid PRR Approvers
-func (k *KEPHandler) validatePRRApprovers(p *Proposal) []error {
-	var errs []error
-	validPRRApprovers := make(map[string]bool)
-	for _, a := range k.PRRApprovers {
-		validPRRApprovers[a] = true
-	}
-	for _, a := range p.PRRApprovers {
-		if _, ok := validPRRApprovers[a]; !ok {
-			errs = append(errs, fmt.Errorf("invalid prr-approver: %s", a))
-		}
-	}
-	return errs
-}
-
 // Validate returns errors for each reason the given proposal is invalid,
 // or nil if it is valid
 func (k *KEPHandler) Validate(p *Proposal) []error {
@@ -248,9 +231,6 @@ func (k *KEPHandler) Validate(p *Proposal) []error {
 	if errs := k.validateGroups(p); errs != nil {
 		allErrs = append(allErrs, errs...)
 	}
-	if errs := k.validatePRRApprovers(p); errs != nil {
-		allErrs = append(allErrs, errs...)
-	}
 	if err := p.Status.IsValid(); err != nil {
 		allErrs = append(allErrs, err)
 	}

pkg/kepctl/commands/create.go

@@ -122,13 +122,6 @@ func addCreate(topLevel *cobra.Command) {
 		"Participating SIGs",
 	)
 
-	cmd.PersistentFlags().StringArrayVar(
-		&co.PRRApprovers,
-		"prr-approver",
-		[]string{},
-		"PRR Approver",
-	)
-
 	topLevel.AddCommand(cmd)
 }
 

pkg/proposal/create.go

@@ -164,10 +164,6 @@ func populateProposal(p *api.Proposal, opts *CreateOpts) {
 	p.Filename = opts.Name
 	p.LastUpdated = "v1.19"
 	p.Stage = api.Stage(opts.Stage)
-
-	if len(opts.PRRApprovers) > 0 {
-		p.PRRApprovers = updatePersonReference(opts.PRRApprovers)
-	}
 }
 
 func updatePersonReference(names []string) []string {

pkg/repo/query.go

@@ -146,7 +146,8 @@ func (r *Repo) Query(opts *QueryOpts) ([]*api.Proposal, error) {
 		if len(opts.Stage) > 0 && !allowedStage[string(k.Stage)] {
 			continue
 		}
-		if len(opts.PRRApprover) > 0 && !atLeastOne(k.PRRApprovers, allowedPRR) {
+		// TODO: Fetch PRR approvers from approval files.
+		if len(opts.PRRApprover) > 0 && !atLeastOne([]string{}, allowedPRR) {
 			continue
 		}
 		if len(opts.Author) > 0 && !atLeastOne(k.Authors, allowedAuthor) {

pkg/repo/query_test.go

@@ -209,33 +209,6 @@ func TestQuery(t *testing.T) {
 				skip: true,
 			},
 		},
-		"prr-approver": {
-			{
-				name: "results",
-				queryOpts: repo.QueryOpts{
-					PRRApprover: []string{"@dorothy"},
-				},
-				kepNames: []string{
-					"404-question-not-found",
-					"42-the-answer",
-					"13-keps-as-crds",
-				},
-			},
-			{
-				name: "no results",
-				queryOpts: repo.QueryOpts{
-					PRRApprover: []string{"prr-approves-nothing"},
-				},
-			},
-			{
-				name: "TODO: invalid should error but instead returns nothing",
-				queryOpts: repo.QueryOpts{
-					PRRApprover: []string{"prr-approver-does-not-exist"},
-				},
-				err:  fmt.Errorf("something about invalid prr-approver"),
-				skip: true,
-			},
-		},
 		"author": {
 			{
 				name: "results",

pkg/repo/repo.go

@@ -483,45 +483,6 @@ func (r *Repo) loadKEPFromYaml(repoPath, kepPath string) (*api.Proposal, error)
 			"%v",
 			errors.Wrapf(err, "validating PRR for %s", p.Name),
 		)
-	} else {
-		sig := filepath.Base(filepath.Dir(filepath.Dir(kepPath)))
-		prrPath := filepath.Join(
-			prrApprovalPath,
-			sig,
-			p.Number+".yaml",
-		)
-
-		prrFile, err := os.Open(prrPath)
-		if os.IsNotExist(err) {
-			return &p, nil
-		}
-
-		if err != nil {
-			return nil, errors.Wrapf(err, "opening PRR approval %s", prrPath)
-		}
-
-		approval, err := handler.Parse(prrFile)
-		if err != nil {
-			return nil, errors.Wrapf(err, "parsing PRR")
-		}
-
-		approver, err := approval.ApproverForStage(p.Stage)
-		if err != nil {
-			logrus.Errorf(
-				"%v",
-				fmt.Errorf("getting PRR approver for stage '%s' for kep %s: %w", p.Stage, fullKEPPath, err),
-			)
-		}
-
-		for _, a := range p.PRRApprovers {
-			if a == approver {
-				approver = ""
-			}
-		}
-
-		if approver != "" {
-			p.PRRApprovers = append(p.PRRApprovers, approver)
-		}
 	}
 
 	return &p, nil

pkg/repo/repo_test.go

@@ -87,13 +87,6 @@ func TestProposalValidate(t *testing.T) {
 				fmt.Errorf("invalid owning-sig: not-a-sig"),
 			},
 		},
-		{
-			name: "invalid KEP: prr-approver does not exist",
-			file: "testdata/invalid-prr-approver.yaml",
-			errs: []error{
-				fmt.Errorf("invalid prr-approver: not-a-prr-approver"),
-			},
-		},
 		{
 			name: "invalid KEP: status does not exist",
 			file: "testdata/invalid-status.yaml",

pkg/repo/testdata/invalid-prr-approver.yaml

@@ -14,9 +14,6 @@ reviewers:
 approvers:
   - TBD
   - "@oscar.doe"
-prr-approvers:
-  - TBD
-  - "@bob.doe"
 see-also:
   - "/keps/sig-aaa/1234-we-heard-you-like-keps"
   - "/keps/sig-bbb/2345-everyone-gets-a-kep"