From 6e748a331fae9dd796c9415ea9da2c843a9ea091 Mon Sep 17 00:00:00 2001 From: Stephen Augustus Date: Sun, 31 Jan 2021 23:46:57 -0500 Subject: [PATCH] api: Validate PRRs by field Signed-off-by: Stephen Augustus --- api/approval.go | 51 +++++++++++++++++++++++++++++++++++- api/document.go | 22 +++++++++++++++- go.mod | 5 +++- go.sum | 10 +++++++ pkg/legacy/keps/proposals.go | 11 ++++---- test/metadata_test.go | 14 +++++++--- 6 files changed, 102 insertions(+), 11 deletions(-) diff --git a/api/approval.go b/api/approval.go index 0d139c43b480..a02395621419 100644 --- a/api/approval.go +++ b/api/approval.go @@ -16,6 +16,16 @@ limitations under the License. package api +import ( + "bufio" + "bytes" + "io" + + "github.com/go-playground/validator" + "github.com/pkg/errors" + "gopkg.in/yaml.v3" +) + type PRRApprovals []*PRRApproval func (p *PRRApprovals) AddPRRApproval(prrApproval *PRRApproval) { @@ -28,10 +38,49 @@ 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 (prr *PRRApproval) Validate() error { + v := validator.New() + if err := v.Struct(prr); err != nil { + return errors.Wrap(err, "running validation") + } + + return nil +} + // 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 } diff --git a/api/document.go b/api/document.go index b1d5934b7b4a..99317ff1a8ee 100644 --- a/api/document.go +++ b/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 +} diff --git a/go.mod b/go.mod index 69cf55a0080e..5af5a0bd5975 100644 --- a/go.mod +++ b/go.mod @@ -3,16 +3,19 @@ module k8s.io/enhancements go 1.15 require ( + github.com/go-playground/universal-translator v0.17.0 // indirect + github.com/go-playground/validator v9.31.0+incompatible 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/go-playground/assert.v1 v1.2.1 // indirect gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c k8s.io/release v0.7.0 k8s.io/test-infra v0.0.0-20200813194141-e9678d500461 diff --git a/go.sum b/go.sum index 4002b3da24e7..4593d5a66a18 100644 --- a/go.sum +++ b/go.sum @@ -482,6 +482,12 @@ 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/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 v9.31.0+incompatible h1:UA72EPEogEnq76ehGdEDp4Mit+3FDh548oRqwVgNsHA= +github.com/go-playground/validator v9.31.0+incompatible/go.mod h1:yrEkQXlcI+PugkyDjY2bRrL/UBU4f3rvrgkN3V8JEig= 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 +828,8 @@ 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.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= @@ -1853,6 +1861,8 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/gcfg.v1 v1.2.0/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o= gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKWaSkCsqBpgog8nAV2xsGOxlo= +gopkg.in/go-playground/assert.v1 v1.2.1 h1:xoYuJVE7KT85PYWrN730RguIQO0ePzVRfFMXadIrXTM= +gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE= gopkg.in/inf.v0 v0.9.0/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/ini.v1 v1.46.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= diff --git a/pkg/legacy/keps/proposals.go b/pkg/legacy/keps/proposals.go index 6b4e9ad38b14..68ca4449384e 100644 --- a/pkg/legacy/keps/proposals.go +++ b/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) diff --git a/test/metadata_test.go b/test/metadata_test.go index a463fda05dea..92ea8b7c56ea 100644 --- a/test/metadata_test.go +++ b/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 == "" {