api: Validate PRRs by field

Signed-off-by: Stephen Augustus <[email protected]>

Author: justaugustus

api/approval.go

@@ -16,6 +16,16 @@ limitations under the License.
 
 package api
 
+import (
+	"bufio"
+	"bytes"
+	"io"
+
+	"github.com/go-playground/validator/v10"
+	"github.com/pkg/errors"
+	"gopkg.in/yaml.v3"
+)
+
 type PRRApprovals []*PRRApproval
 
 func (p *PRRApprovals) AddPRRApproval(prrApproval *PRRApproval) {
@@ -28,23 +38,62 @@ type PRRApproval struct {
 	Beta   PRRMilestone `json:"beta" yaml:"beta,omitempty"`
 	Stable PRRMilestone `json:"stable" yaml:"stable,omitempty"`
 
+	// TODO(api): Move to separate struct for handling document parsing
 	Error error `json:"-" yaml:"-"`
 }
 
-func (p *PRRApproval) ApproverForStage(stage string) string {
+func (prr *PRRApproval) Validate() error {
+	v := validator.New()
+	if err := v.Struct(prr); err != nil {
+		return errors.Wrap(err, "running validation")
+	}
+
+	return nil
+}
+
+func (prr *PRRApproval) ApproverForStage(stage string) string {
 	switch stage {
 	case "alpha":
-		return p.Alpha.Approver
+		return prr.Alpha.Approver
 	case "beta":
-		return p.Beta.Approver
+		return prr.Beta.Approver
 	case "stable":
-		return p.Stable.Approver
+		return prr.Stable.Approver
 	}
 
 	return ""
 }
 
 // TODO(api): Can we refactor the proposal `Milestone` to retrieve this?
 type PRRMilestone struct {
-	Approver string `json:"approver" yaml:"approver"`
+	Approver string `json:"approver" yaml:"approver" validate:"required"`
+}
+
+type PRRHandler Parser
+
+// TODO(api): Make this a generic parser for all `Document` types
+func (p *PRRHandler) Parse(in io.Reader) (*PRRApproval, error) {
+	scanner := bufio.NewScanner(in)
+	var body bytes.Buffer
+	for scanner.Scan() {
+		line := scanner.Text() + "\n"
+		body.WriteString(line)
+	}
+
+	approval := &PRRApproval{}
+	if err := scanner.Err(); err != nil {
+		return approval, errors.Wrap(err, "reading file")
+	}
+
+	if err := yaml.Unmarshal(body.Bytes(), &approval); err != nil {
+		p.Errors = append(p.Errors, errors.Wrap(err, "error unmarshalling YAML"))
+		return approval, errors.Wrap(err, "unmarshalling YAML")
+	}
+
+	if valErr := approval.Validate(); valErr != nil {
+		p.Errors = append(p.Errors, errors.Wrap(valErr, "validating PRR"))
+		return approval, errors.Wrap(valErr, "validating PRR")
+	}
+
+	return approval, nil
 }

api/document.go

@@ -16,6 +16,26 @@ limitations under the License.
 
 package api
 
+import (
+	"io"
+)
+
 // TODO(api): Populate interface
 // TODO(api): Mock interface
-type Document interface{}
+type File interface {
+	Parse(io.Reader) (Document, error)
+}
+
+// TODO(api): Populate interface
+// TODO(api): Mock interface
+// Document is an interface satisfied by the following types:
+// - `Proposal` (KEP)
+// - `PRRApproval`
+// - `Receipt` (coming soon)
+type Document interface {
+	Validate() error
+}
+
+type Parser struct {
+	Errors []error
+}

go.mod

@@ -3,14 +3,15 @@ module k8s.io/enhancements
 go 1.15
 
 require (
+	github.com/go-playground/validator/v10 v10.4.1
 	github.com/google/go-github/v32 v32.1.0
+	github.com/leodido/go-urn v1.2.1 // indirect
 	github.com/maxbrunsfeld/counterfeiter/v6 v6.3.0
 	github.com/olekukonko/tablewriter v0.0.4
 	github.com/pkg/errors v0.9.1
 	github.com/psampaz/go-mod-outdated v0.7.0
 	github.com/sirupsen/logrus v1.7.0
 	github.com/spf13/cobra v1.1.1
-	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.0
 	golang.org/x/oauth2 v0.0.0-20210112200429-01de73cf58bd
 	gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c

go.sum

@@ -482,6 +482,14 @@ github.com/go-openapi/validate v0.17.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+
 github.com/go-openapi/validate v0.18.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+MYsct2VUrAJ4=
 github.com/go-openapi/validate v0.19.2/go.mod h1:1tRCw7m3jtI8eNWEEliiAqUIcBztB2KDnRCRMUi7GTA=
 github.com/go-openapi/validate v0.19.5/go.mod h1:8DJv2CVJQ6kGNpFW6eV9N3JviE1C85nY1c2z52x1Gk4=
+github.com/go-playground/assert/v2 v2.0.1 h1:MsBgLAaY856+nPRTKrp3/OZK38U/wa0CcBYNjji3q3A=
+github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4=
+github.com/go-playground/locales v0.13.0 h1:HyWk6mgj5qFqCT5fjGBuRArbVDfE4hi8+e8ceBS/t7Q=
+github.com/go-playground/locales v0.13.0/go.mod h1:taPMhCMXrRLJO55olJkUXHZBHCxTMfnGwq/HNwmWNS8=
+github.com/go-playground/universal-translator v0.17.0 h1:icxd5fm+REJzpZx7ZfpaD876Lmtgy7VtROAbHHXk8no=
+github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA=
+github.com/go-playground/validator/v10 v10.4.1 h1:pH2c5ADXtd66mxoE0Zm9SUhxE20r7aM3F26W0hOn+GE=
+github.com/go-playground/validator/v10 v10.4.1/go.mod h1:nlOn6nFhuKACm19sB/8EGNn9GlaMV7XkbRSipzJ0Ii4=
 github.com/go-sql-driver/mysql v0.0.0-20160411075031-7ebe0a500653/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
 github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
 github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
@@ -822,6 +830,9 @@ github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII=
+github.com/leodido/go-urn v1.2.1 h1:BqpAaACuzVSgi/VLzGZIobT2z4v53pjosyNd9Yv6n/w=
+github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY=
 github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/lib/pq v1.1.1/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=

pkg/legacy/keps/proposals.go

@@ -28,7 +28,6 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"k8s.io/enhancements/api"
-	"k8s.io/enhancements/pkg/legacy/keps/validations"
 )
 
 type Parser struct{}
@@ -71,10 +70,12 @@ func (p *Parser) Parse(in io.Reader) *api.Proposal {
 		return proposal
 	}
 
-	if err := validations.ValidateStructure(test); err != nil {
-		proposal.Error = errors.Wrap(err, "error validating KEP metadata")
-		return proposal
-	}
+	/*
+		if err := validations.ValidateStructure(test); err != nil {
+			proposal.Error = errors.Wrap(err, "error validating KEP metadata")
+			return proposal
+		}
+	*/
 
 	proposal.Error = yaml.Unmarshal(metadata, proposal)
 	proposal.ID = hash(proposal.OwningSIG + ":" + proposal.Title)

test/metadata_test.go

@@ -22,8 +22,10 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
+	"k8s.io/enhancements/api"
 	"k8s.io/enhancements/pkg/legacy/keps"
-	"k8s.io/enhancements/pkg/legacy/prrs"
 )
 
 const (
@@ -74,7 +76,7 @@ func TestValidation(t *testing.T) {
 	}
 
 	kepParser := &keps.Parser{}
-	prrParser := &prrs.Parser{}
+	prrHandler := &api.PRRHandler{}
 	prrsDir := filepath.Join("..", prrsDir)
 
 	for _, filename := range files {
@@ -83,6 +85,7 @@ func TestValidation(t *testing.T) {
 			if err != nil {
 				t.Fatalf("could not open file %s: %v\n", filename, err)
 			}
+
 			defer kepFile.Close()
 
 			kep := kepParser.Parse(kepFile)
@@ -114,10 +117,14 @@ func TestValidation(t *testing.T) {
 				t.Errorf("To get PRR approval modify appropriately file %s and have this approved by PRR team", prrFilename)
 				return
 			}
+
 			if err != nil {
 				t.Fatalf("could not open file %s: %v\n", prrFilename, err)
 			}
-			prr := prrParser.Parse(prrFile)
+
+			prr, prrParseErr := prrHandler.Parse(prrFile)
+			require.Nil(t, prrParseErr)
+
 			if prr.Error != nil {
 				t.Errorf("PRR approval file %v has an error: %v", prrFilename, prr.Error)
 				return
@@ -132,6 +139,7 @@ func TestValidation(t *testing.T) {
 			case "stable":
 				stagePRRApprover = prr.Stable.Approver
 			}
+
 			if len(stageMilestone) > 0 && stageMilestone >= "v1.21" {
 				// PRR approval is needed.
 				if stagePRRApprover == "" {