Merge pull request #4338 from wojtek-t/implementable_has_milestones

Improve validation for KEP metadata

Author: k8s-ci-robot

Diff for: keps/sig-auth/2907-secrets-store-csi-driver/kep.yaml

@@ -15,3 +15,9 @@ approvers:
   - "@mikedanese"
 status: implemented
 stage: stable
+
+latest-milestone: "v1.19"
+
+# The milestone at which this feature was, or is targeted to be, at each stage.
+milestone:
+  stable: "v1.22"

Diff for: keps/sig-etcd/4326-downgrade/4326.yaml renamed to keps/sig-etcd/4326-downgrade/kep.yaml

@@ -3,7 +3,7 @@ kep-number: 4326
 authors:
   - "@serathius"
 owning-sig: sig-etcd
-status: implementable
+status: provisional
 creation-date: yyyy-mm-dd
 reviewers:
 approvers:

Diff for: keps/sig-etcd/4331-livez-readyz/4331.yaml renamed to keps/sig-etcd/4331-livez-readyz/kep.yaml

@@ -4,7 +4,7 @@ authors:
   - "@siyuanfoundation"
   - "@chaochn47"
 owning-sig: sig-etcd
-status: implementable
+status: provisional
 creation-date: yyyy-mm-dd
 reviewers:
 approvers:

Diff for: keps/sig-network/2829-gateway-api-to-k8s-io/kep.yaml

@@ -17,3 +17,6 @@ approvers:
 
 # The target maturity stage in the current dev cycle for this KEP.
 stage: alpha
+
+# This is out-of-tree and we provide milestone just to pass validation.
+latest-milestone: "v1.22"

Diff for: pkg/kepval/approval.go

@@ -30,7 +30,7 @@ import (
 )
 
 func ValidatePRR(kep *api.Proposal, h *api.PRRHandler, prrDir string) error {
-	requiredPRRApproval, _, _, err := isPRRRequired(kep)
+	requiredPRRApproval, err := isPRRRequired(kep)
 	if err != nil {
 		return errors.Wrap(err, "checking if PRR is required")
 	}
@@ -97,38 +97,37 @@ func ValidatePRR(kep *api.Proposal, h *api.PRRHandler, prrDir string) error {
 	return nil
 }
 
-func isPRRRequired(kep *api.Proposal) (required, missingMilestone, missingStage bool, err error) {
+func isPRRRequired(kep *api.Proposal) (required bool, err error) {
 	logrus.Debug("checking if PRR is required")
 
 	required = kep.Status == api.ImplementableStatus || kep.Status == api.ImplementedStatus
-	missingMilestone = kep.IsMissingMilestone()
-	missingStage = kep.IsMissingStage()
+	if !required {
+		return false, nil
+	}
 
-	if missingMilestone {
-		logrus.Warnf("Missing the latest milestone field: %s", kep.Filename)
-		return required, missingMilestone, missingStage, nil
+	if kep.IsMissingMilestone() {
+		return required, fmt.Errorf("missing the latest milestone field: %s", kep.Filename)
 	}
 
-	if missingStage {
-		logrus.Warnf("Missing the stage field: %s", kep.Filename)
-		return required, missingMilestone, missingStage, nil
+	if kep.IsMissingStage() {
+		return required, fmt.Errorf("missing the stage field: %s", kep.Filename)
 	}
 
 	// TODO: Consider making this a function
 	prrRequiredAtSemVer, err := semver.ParseTolerant("v1.21")
 	if err != nil {
-		return required, missingMilestone, missingStage, errors.Wrap(err, "creating a SemVer object for PRRs")
+		return required, errors.Wrap(err, "creating a SemVer object for PRRs")
 	}
 
 	latestSemVer, err := semver.ParseTolerant(kep.LatestMilestone)
 	if err != nil {
-		return required, missingMilestone, missingStage, errors.Wrap(err, "creating a SemVer object for latest milestone")
+		return required, errors.Wrap(err, "creating a SemVer object for latest milestone")
 	}
 
 	if latestSemVer.LT(prrRequiredAtSemVer) {
 		required = false
-		return required, missingMilestone, missingStage, nil
+		return required, nil
 	}
 
-	return required, missingMilestone, missingStage, nil
+	return required, nil
 }

Diff for: pkg/repo/validate.go

@@ -17,6 +17,7 @@ limitations under the License.
 package repo
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -63,6 +64,7 @@ func (r *Repo) Validate() (
 
 			metadataFilename := ProposalMetadataFilename
 			metadataFilepath := filepath.Join(dir, metadataFilename)
+
 			if _, err := os.Stat(metadataFilepath); err == nil {
 				// There is KEP metadata file in this directory, only that one should be processed.
 				if info.Name() == metadataFilename {
@@ -71,6 +73,14 @@ func (r *Repo) Validate() (
 				}
 			}
 
+			if info.Name() == ProposalFilename && dir != kepDir {
+				// There is a proposal, we require metadata file for it.
+				if _, err := os.Stat(metadataFilepath); os.IsNotExist(err) {
+					kepErr := fmt.Errorf("metadata file missing for KEP: %s", metadataFilepath)
+					valErrMap[metadataFilepath] = append(valErrMap[metadataFilepath], kepErr)
+				}
+			}
+
 			return nil
 		},
 	)