api: Validate KEPs by field

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

Author: justaugustus

api/approval.go

@@ -66,7 +66,7 @@ func (prr *PRRApproval) ApproverForStage(stage string) string {
 
 // TODO(api): Can we refactor the proposal `Milestone` to retrieve this?
 type PRRMilestone struct {
-	Approver string `json:"approver" yaml:"approver" validate:"required"`
+	Approver string `json:"approver" yaml:"approver"`
 }
 
 type PRRHandler Parser

api/proposal.go

@@ -16,6 +16,19 @@ limitations under the License.
 
 package api
 
+import (
+	"bufio"
+	"bytes"
+	"crypto/md5"
+	"fmt"
+	"io"
+	"strings"
+
+	"github.com/go-playground/validator/v10"
+	"github.com/pkg/errors"
+	"gopkg.in/yaml.v3"
+)
+
 type Proposals []*Proposal
 
 func (p *Proposals) AddProposal(proposal *Proposal) {
@@ -56,6 +69,65 @@ type Proposal struct {
 	Contents string `json:"markdown" yaml:"-"`
 }
 
+func (p *Proposal) Validate() error {
+	v := validator.New()
+	if err := v.Struct(p); err != nil {
+		return errors.Wrap(err, "running validation")
+	}
+
+	return nil
+}
+
+type KEPHandler Parser
+
+// TODO(api): Make this a generic parser for all `Document` types
+func (k *KEPHandler) Parse(in io.Reader) (*Proposal, error) {
+	scanner := bufio.NewScanner(in)
+	count := 0
+	metadata := []byte{}
+	var body bytes.Buffer
+	for scanner.Scan() {
+		line := scanner.Text() + "\n"
+		if strings.Contains(line, "---") {
+			count++
+			continue
+		}
+		if count == 1 {
+			metadata = append(metadata, []byte(line)...)
+		} else {
+			body.WriteString(line)
+		}
+	}
+
+	kep := &Proposal{
+		Contents: body.String(),
+	}
+
+	if err := scanner.Err(); err != nil {
+		return kep, errors.Wrap(err, "reading file")
+	}
+
+	// this file is just the KEP metadata
+	if count == 0 {
+		metadata = body.Bytes()
+		kep.Contents = ""
+	}
+
+	if err := yaml.Unmarshal(metadata, &kep); err != nil {
+		k.Errors = append(k.Errors, errors.Wrap(err, "error unmarshalling YAML"))
+		return kep, errors.Wrap(err, "unmarshalling YAML")
+	}
+
+	if valErr := kep.Validate(); valErr != nil {
+		k.Errors = append(k.Errors, errors.Wrap(valErr, "validating KEP"))
+		return kep, errors.Wrap(valErr, "validating KEP")
+	}
+
+	kep.ID = hash(kep.OwningSIG + ":" + kep.Title)
+
+	return kep, nil
+}
+
 type Milestone struct {
 	Alpha  string `json:"alpha" yaml:"alpha"`
 	Beta   string `json:"beta" yaml:"beta"`
@@ -66,3 +138,7 @@ 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)))
+}

test/metadata_test.go

@@ -25,7 +25,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"k8s.io/enhancements/api"
-	"k8s.io/enhancements/pkg/legacy/keps"
 )
 
 const (
@@ -34,37 +33,14 @@ const (
 	kepMetadata = "kep.yaml"
 )
 
-// This is the actual validation check of all keps in this repo
+var files = []string{}
+
+// This is the actual validation check of all KEPs in this repo
 func TestValidation(t *testing.T) {
 	// Find all the keps
-	files := []string{}
 	err := filepath.Walk(
 		filepath.Join("..", kepsDir),
-		func(path string, info os.FileInfo, err error) error {
-			if err != nil {
-				return err
-			}
-			if info.IsDir() {
-				return nil
-			}
-
-			dir := filepath.Dir(path)
-			// true if the file is a symlink
-			if info.Mode()&os.ModeSymlink != 0 {
-				// assume symlink from old KEP location to new
-				newLocation, err := os.Readlink(path)
-				if err != nil {
-					return err
-				}
-				files = append(files, filepath.Join(dir, filepath.Dir(newLocation), kepMetadata))
-				return nil
-			}
-			if ignore(dir, info.Name()) {
-				return nil
-			}
-			files = append(files, path)
-			return nil
-		},
+		walkFn,
 	)
 	// This indicates a problem walking the filepath, not a validation error.
 	if err != nil {
@@ -75,7 +51,7 @@ func TestValidation(t *testing.T) {
 		t.Fatal("must find more than 0 keps")
 	}
 
-	kepParser := &keps.Parser{}
+	kepHandler := &api.KEPHandler{}
 	prrHandler := &api.PRRHandler{}
 	prrsDir := filepath.Join("..", prrsDir)
 
@@ -88,7 +64,9 @@ func TestValidation(t *testing.T) {
 
 			defer kepFile.Close()
 
-			kep := kepParser.Parse(kepFile)
+			kep, kepParseErr := kepHandler.Parse(kepFile)
+			require.Nil(t, kepParseErr)
+
 			if kep.Error != nil {
 				t.Errorf("%v has an error: %v", filename, kep.Error)
 			}
@@ -152,6 +130,36 @@ func TestValidation(t *testing.T) {
 	}
 }
 
+var walkFn = func(path string, info os.FileInfo, err error) error {
+	if err != nil {
+		return err
+	}
+
+	if info.IsDir() {
+		return nil
+	}
+
+	dir := filepath.Dir(path)
+	// true if the file is a symlink
+	if info.Mode()&os.ModeSymlink != 0 {
+		// assume symlink from old KEP location to new
+		newLocation, err := os.Readlink(path)
+		if err != nil {
+			return err
+		}
+
+		files = append(files, filepath.Join(dir, filepath.Dir(newLocation), kepMetadata))
+		return nil
+	}
+
+	if ignore(dir, info.Name()) {
+		return nil
+	}
+
+	files = append(files, path)
+	return nil
+}
+
 // TODO: Consider replacing with a .kepignore file
 // TODO: Is this a duplicate of the package function?
 // ignore certain files in the keps/ subdirectory