tools: add metadata linting step to linter job

enhancements/single-node/single-node-openshift-with-workers.md

@@ -7,12 +7,12 @@ reviewers:
   - "@eranco74"
   - "@tsorya"
   - "@dhellmann"
-  - "@Miciah" - edge networking
+  - "@Miciah - edge networking"
   - "@bparees"
-  - "@JoelSpeed" - API
-  - "@staebler" - installer
+  - "@JoelSpeed - API"
+  - "@staebler - installer"
   - "@derekwaynecarr"
-  - "@cgwalters" - MCO
+  - "@cgwalters - MCO"
 approvers:
   - "@dhellmann"
 api-approvers: # in case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers)

guidelines/enhancement_template.md

@@ -1,14 +1,12 @@
 ---
 title: neat-enhancement-idea
 authors:
-  - "@janedoe"
-reviewers: # Include a comment about what domain expertise a reviewer is expected to bring. For example, "- @networkguru, for networking aspects"
   - TBD
-  - "@alicedoe"
+reviewers: # Include a comment about what domain expertise a reviewer is expected to bring and what area of the enhancement you expect them to focus on. For example: - "@networkguru, for networking aspects, please look at IP bootstrapping aspect"
+  - TBD
 approvers:
   - TBD
-  - "@oscardoe"
-api-approvers: # in case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers)
+api-approvers: # In case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers). If there is no API change, use "None"
   - TBD
 creation-date: yyyy-mm-dd
 last-updated: yyyy-mm-dd

hack/Dockerfile.markdownlint

@@ -1,6 +1,6 @@
 FROM fedora
 WORKDIR /workdir
 COPY install-markdownlint.sh /tmp
-RUN dnf install -y git
+RUN dnf install -y git golang
 RUN /tmp/install-markdownlint.sh
 ENTRYPOINT /workdir/hack/markdownlint.sh

hack/markdownlint.sh

@@ -3,3 +3,5 @@
 markdownlint-cli2 '**/*.md'
 
 $(dirname $0)/template-lint.sh
+
+$(dirname $0)/metadata-lint.sh

hack/metadata-lint.sh

@@ -0,0 +1,20 @@
+#!/bin/bash
+
+set -x
+
+# Ensure the enhancement metadata includes the required information
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+# We only look at the files that have changed in the current PR, to
+# avoid problems when the template is changed in a way that is
+# incompatible with existing documents.
+CHANGED_FILES=$(git log --name-only --pretty= "${PULL_BASE_SHA:-origin/master}.." -- \
+                    | (grep '^enhancements.*\.md$' || true) \
+                    | sort -u)
+
+(cd tools && go build -o ../enhancement-tool -mod=mod ./main.go)
+
+./enhancement-tool metadata-lint ${CHANGED_FILES}

tools/cmd/annualSummary.go

@@ -41,6 +41,8 @@ var annualSummaryCmd = &cobra.Command{
 	Short: "Summarize the enhancements work over the previous year",
 	// Long: ``,
 	RunE: func(cmd *cobra.Command, args []string) error {
+		initConfig()
+
 		earliestDate := time.Date(year, time.January, 1, 0, 0, 0, 0, time.UTC)
 		latestDate := time.Date(year+1, time.January, 1, 0, 0, 0, 0, time.UTC)
 

tools/cmd/metadataLint.go

@@ -0,0 +1,85 @@
+/*
+Copyright © 2022 Doug Hellmann <[email protected]>
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package cmd
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"github.com/openshift/enhancements/tools/enhancements"
+
+	"github.com/spf13/cobra"
+)
+
+// metadataLintCmd represents the metadataLint command
+var metadataLintCmd = &cobra.Command{
+	Use:       "metadata-lint",
+	Short:     "Check the metadata of the enhancement files given as arguments",
+	ValidArgs: []string{"enhancement-filename"},
+	Run: func(cmd *cobra.Command, args []string) {
+		errorCount := 0
+		for _, filename := range args {
+			reportError := func(msg string) {
+				fmt.Fprintf(os.Stderr, "%s: %s\n", filename, msg)
+				errorCount++
+			}
+			content, err := ioutil.ReadFile(filename)
+			if err != nil {
+				reportError(fmt.Sprintf("%s", err))
+				continue
+			}
+			metadata, err := enhancements.NewMetaData(content)
+			if err != nil {
+				reportError(fmt.Sprintf("%s", err))
+				continue
+			}
+
+			// Doc title needs to match filename, minus the ".md" extension
+			fileBase := filepath.Base(filename)
+			fileBase = fileBase[0 : len(fileBase)-3]
+			if fileBase != metadata.Title {
+				reportError(fmt.Sprintf("the title %s and the file base name %s must match",
+					metadata.Title, fileBase,
+				))
+			}
+
+			for _, msg := range metadata.Validate() {
+				reportError(msg)
+			}
+		}
+
+		if errorCount > 0 {
+			fmt.Fprintf(os.Stderr, "%d errors", errorCount)
+			os.Exit(1)
+		}
+	},
+}
+
+func init() {
+	rootCmd.AddCommand(metadataLintCmd)
+
+	// Here you will define your flags and configuration settings.
+
+	// Cobra supports Persistent Flags which will work for this command
+	// and all subcommands, e.g.:
+	// metadataLintCmd.PersistentFlags().String("foo", "", "A help for foo")
+
+	// Cobra supports local flags which will only run when this command
+	// is called directly, e.g.:
+	// metadataLintCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle")
+}

tools/cmd/pullRequests.go

@@ -44,6 +44,7 @@ func newPullRequestsCommand() *cobra.Command {
 		Short: "List pull requests and some characteristics in CSV format",
 		Long:  `Produce a CSV list of pull requests suitable for import into a spreadsheet.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			initConfig()
 
 			earliestDate := time.Now().AddDate(0, 0, daysBack*-1)
 

tools/cmd/report.go

@@ -23,6 +23,7 @@ func newReportCommand() *cobra.Command {
 		Use:   "report",
 		Short: "Generate the weekly activity report",
 		RunE: func(cmd *cobra.Command, args []string) error {
+			initConfig()
 
 			earliestDate := time.Now().AddDate(0, 0, daysBack*-1)
 

tools/cmd/reviewers.go

@@ -55,6 +55,8 @@ func newReviewersCommand() *cobra.Command {
 		Short: "List reviewers of PRs in a repo",
 		Long:  ``,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			initConfig()
+
 			earliestDate := time.Now().AddDate(0, 0, daysBack*-1)
 
 			query := &util.PullRequestQuery{

tools/cmd/root.go

@@ -32,8 +32,6 @@ func init() {
 
 	defaultConfigFilename := filepath.Join(configDir, "ocp-enhancements", "config.yml")
 
-	cobra.OnInitialize(initConfig)
-
 	rootCmd.PersistentFlags().BoolVar(&devMode, "dev", false, "dev mode, stop after first page of PRs")
 	rootCmd.PersistentFlags().StringVar(&configFilename, "config", defaultConfigFilename, "config file")
 	rootCmd.PersistentFlags().StringVar(&orgName, "org", "openshift", "github organization")

tools/cmd/show-pr.go

@@ -33,6 +33,8 @@ func newShowPRCommand() *cobra.Command {
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
+			initConfig()
+
 			prID, err := strconv.Atoi(args[0])
 			if err != nil {
 				return errors.Wrap(err,

tools/enhancements/metadata.go

@@ -2,6 +2,7 @@ package enhancements
 
 import (
 	"fmt"
+	"net/url"
 
 	"github.com/pkg/errors"
 	"gopkg.in/yaml.v2"
@@ -43,3 +44,83 @@ func (s *Summarizer) GetMetaData(pr int) (*MetaData, error) {
 	}
 	return NewMetaData(content)
 }
+
+// Validate returns a list of issues found with the metadata
+func (m *MetaData) Validate() []string {
+	results := []string{}
+
+	reportError := func(msg string) {
+		results = append(results, msg)
+	}
+
+	// Must have one valid tracking link and no TBD
+	foundLink := false
+	for _, trackingLink := range m.TrackingLinks {
+		if trackingLink == "TBD" {
+			reportError("'TBD' is not a valid value for tracking-link")
+			continue
+		}
+		if trackingLink == "" {
+			reportError("tracking-link must not be empty")
+			continue
+		}
+		if u, err := url.Parse(trackingLink); err != nil {
+			reportError(fmt.Sprintf("could not parse tracking-link %q: %s",
+				trackingLink, err,
+			))
+			continue
+		} else {
+			if u.Scheme == "" {
+				reportError(fmt.Sprintf("could not parse tracking-link %q",
+					trackingLink,
+				))
+				continue
+			}
+		}
+		foundLink = true
+	}
+	if !foundLink {
+		reportError("tracking-link must contain at least one valid URL")
+	}
+
+	// No TBD in required people fields
+	for _, field := range []struct {
+		Name string
+		Data []string
+	}{
+		{
+			Name: "authors",
+			Data: m.Authors,
+		},
+		{
+			Name: "reviewers",
+			Data: m.Reviewers,
+		},
+		{
+			Name: "approvers",
+			Data: m.Approvers,
+		},
+		{
+			Name: "api-approvers",
+			Data: m.APIApprovers,
+		},
+	} {
+		valid := 0
+		for _, value := range field.Data {
+			if value == "TBD" {
+				reportError(fmt.Sprintf("%s must not have TBD as value", field.Name))
+				continue
+			}
+			if value == "" {
+				reportError(fmt.Sprintf("%s must not be an empty string", field.Name))
+				continue
+			}
+			valid++
+		}
+		if valid < 1 {
+			reportError(fmt.Sprintf("%s must have at least one valid github id", field.Name))
+		}
+	}
+
+	return results
+}