From 212a6523614c428682fa30b84c0f423ce185dcef Mon Sep 17 00:00:00 2001 From: Stephen Augustus Date: Thu, 4 Feb 2021 10:45:25 -0500 Subject: [PATCH] Revert "Merge pull request #2425 from justaugustus/refactor-revert" This reverts commit 3e2c87bb7ec87125410631bce9dbd56373ce4aef, reversing changes made to 0a495c8b528a970f136ed53a4310776b7fd17881. --- api/approval.go | 99 +++++++++++++++ cmd/kepctl/helper.go => api/document.go | 32 +++-- api/proposal.go | 76 ++++++++++++ cmd/kepctl/cmd/create.go | 106 ++++++++++++++++ cmd/kepctl/cmd/promote.go | 72 +++++++++++ cmd/kepctl/cmd/query.go | 116 ++++++++++++++++++ cmd/kepctl/cmd/root.go | 73 +++++++++++ cmd/kepctl/create.go | 51 -------- cmd/kepctl/main.go | 35 +----- cmd/kepctl/promote.go | 46 ------- cmd/kepctl/query.go | 55 --------- cmd/kepify/main.go | 19 +-- go.mod | 6 +- go.sum | 11 ++ hack/verify-kep-metadata.sh | 2 +- pkg/kepctl/create.go | 37 ++++-- pkg/kepctl/create_test.go | 24 ++-- pkg/kepctl/kepctl.go | 72 +++++++---- pkg/kepctl/kepctl_test.go | 12 +- pkg/kepctl/promote.go | 10 +- pkg/kepctl/query.go | 23 ++-- pkg/kepctl/query_test.go | 6 +- pkg/{kepval => legacy}/keps/proposals.go | 16 +-- pkg/{kepval => legacy}/keps/proposals_test.go | 2 +- .../keps/validations/yaml.go | 57 ++++++++- .../keps/validations/yaml_test.go | 7 +- pkg/{kepval => legacy}/prrs/approvals.go | 46 ++----- pkg/{kepval => legacy}/prrs/approvals_test.go | 0 .../prrs/validations/yaml.go | 16 ++- .../prrs/validations/yaml_test.go | 9 +- pkg/{kepval => legacy}/util/errors.go | 0 pkg/{kepval => legacy}/util/helpers.go | 5 +- test/OWNERS | 9 ++ .../main_test.go => test/metadata_test.go | 94 ++++++++------ 34 files changed, 869 insertions(+), 375 deletions(-) create mode 100644 api/approval.go rename cmd/kepctl/helper.go => api/document.go (54%) create mode 100644 cmd/kepctl/cmd/create.go create mode 100644 cmd/kepctl/cmd/promote.go create mode 100644 cmd/kepctl/cmd/query.go create mode 100644 cmd/kepctl/cmd/root.go delete mode 100644 cmd/kepctl/create.go delete mode 100644 cmd/kepctl/promote.go delete mode 100644 cmd/kepctl/query.go rename pkg/{kepval => legacy}/keps/proposals.go (86%) rename pkg/{kepval => legacy}/keps/proposals_test.go (97%) rename pkg/{kepval => legacy}/keps/validations/yaml.go (76%) rename pkg/{kepval => legacy}/keps/validations/yaml_test.go (97%) rename pkg/{kepval => legacy}/prrs/approvals.go (57%) rename pkg/{kepval => legacy}/prrs/approvals_test.go (100%) rename pkg/{kepval => legacy}/prrs/validations/yaml.go (82%) rename pkg/{kepval => legacy}/prrs/validations/yaml_test.go (94%) rename pkg/{kepval => legacy}/util/errors.go (100%) rename pkg/{kepval => legacy}/util/helpers.go (96%) create mode 100644 test/OWNERS rename cmd/kepval/main_test.go => test/metadata_test.go (75%) diff --git a/api/approval.go b/api/approval.go new file mode 100644 index 000000000000..5f53bc54a82a --- /dev/null +++ b/api/approval.go @@ -0,0 +1,99 @@ +/* +Copyright 2021 The Kubernetes Authors. + +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 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) { + *p = append(*p, prrApproval) +} + +type PRRApproval struct { + Number string `json:"kep-number" yaml:"kep-number"` + Alpha PRRMilestone `json:"alpha" yaml:"alpha,omitempty"` + 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 +} + +func (prr *PRRApproval) ApproverForStage(stage string) string { + switch stage { + case "alpha": + return prr.Alpha.Approver + case "beta": + return prr.Beta.Approver + case "stable": + 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"` +} + +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/cmd/kepctl/helper.go b/api/document.go similarity index 54% rename from cmd/kepctl/helper.go rename to api/document.go index cec96770a152..99317ff1a8ee 100644 --- a/cmd/kepctl/helper.go +++ b/api/document.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors. +Copyright 2021 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,18 +14,28 @@ See the License for the specific language governing permissions and limitations under the License. */ -package main +package api import ( - flag "github.com/spf13/pflag" - - "k8s.io/enhancements/pkg/kepctl" + "io" ) -func addRepoPathFlag( - f *flag.FlagSet, - opts *kepctl.CommonArgs, -) { - f.StringVar(&opts.RepoPath, "repo-path", "", "Path to kubernetes/enhancements") - f.StringVar(&opts.TokenPath, "gh-token-path", "", "Path to a file with a GitHub API token") +// TODO(api): Populate interface +// TODO(api): Mock 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/api/proposal.go b/api/proposal.go index 1b89319a8dcd..84ccdbfe819b 100644 --- a/api/proposal.go +++ b/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))) +} diff --git a/cmd/kepctl/cmd/create.go b/cmd/kepctl/cmd/create.go new file mode 100644 index 000000000000..71eb390d89c1 --- /dev/null +++ b/cmd/kepctl/cmd/create.go @@ -0,0 +1,106 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 ( + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "k8s.io/enhancements/pkg/kepctl" +) + +// TODO: Struct literal instead? +var createOpts = kepctl.CreateOpts{} + +var createCmd = &cobra.Command{ + Use: "create [KEP]", + Short: "Create a new KEP", + Long: "Create a new KEP using the current KEP template for the given type", + Example: ` kepctl create sig-architecture/000-mykep`, + SilenceUsage: true, + SilenceErrors: true, + PreRunE: func(cmd *cobra.Command, args []string) error { + return createOpts.Validate(args) + }, + RunE: func(*cobra.Command, []string) error { + return runCreate(createOpts) + }, +} + +func init() { + // TODO: Should these all be global args? + createCmd.PersistentFlags().StringVar( + &createOpts.Title, + "title", + "", + "KEP Title", + ) + + createCmd.PersistentFlags().StringArrayVar( + &createOpts.Authors, + "authors", + []string{}, + "Authors", + ) + + createCmd.PersistentFlags().StringArrayVar( + &createOpts.Reviewers, + "reviewers", + []string{}, + "Reviewers", + ) + + createCmd.PersistentFlags().StringVar( + &createOpts.Type, + "type", + "feature", + "KEP Type", + ) + + createCmd.PersistentFlags().StringVarP( + &createOpts.State, + "state", + "s", + "provisional", + "KEP State", + ) + + createCmd.PersistentFlags().StringArrayVar( + &createOpts.SIGS, + "sigs", + []string{}, + "Participating SIGs", + ) + + createCmd.PersistentFlags().StringArrayVar( + &createOpts.PRRApprovers, + "prr-approver", + []string{}, + "PRR Approver", + ) + + rootCmd.AddCommand(createCmd) +} + +func runCreate(createOpts kepctl.CreateOpts) error { + k, err := kepctl.New(createOpts.RepoPath) + if err != nil { + return errors.Wrap(err, "creating kepctl client") + } + + return k.Create(&createOpts) +} diff --git a/cmd/kepctl/cmd/promote.go b/cmd/kepctl/cmd/promote.go new file mode 100644 index 000000000000..b0be4953ddec --- /dev/null +++ b/cmd/kepctl/cmd/promote.go @@ -0,0 +1,72 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 ( + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "k8s.io/enhancements/pkg/kepctl" +) + +// TODO: Struct literal instead? +var promoteOpts = kepctl.PromoteOpts{} + +var promoteCmd = &cobra.Command{ + Use: "promote [KEP]", + Short: "Promote a KEP", + Long: "Promote a KEP to a new stage for a target release", + Example: ` kepctl promote sig-architecture/000-mykep --stage beta --release v1.20`, + SilenceUsage: true, + SilenceErrors: true, + PreRunE: func(cmd *cobra.Command, args []string) error { + return promoteOpts.Validate(args) + }, + RunE: func(*cobra.Command, []string) error { + return runPromote(promoteOpts) + }, +} + +func init() { + // TODO: Should these all be global args? + promoteCmd.PersistentFlags().StringVarP( + &promoteOpts.Stage, + "stage", + "s", + "", + "KEP Stage", + ) + + promoteCmd.PersistentFlags().StringVarP( + &promoteOpts.Release, + "release", + "r", + "", + "Target Release", + ) + + rootCmd.AddCommand(promoteCmd) +} + +func runPromote(opts kepctl.PromoteOpts) error { + k, err := kepctl.New(opts.RepoPath) + if err != nil { + return errors.Wrap(err, "creating kepctl client") + } + + return k.Promote(&opts) +} diff --git a/cmd/kepctl/cmd/query.go b/cmd/kepctl/cmd/query.go new file mode 100644 index 000000000000..382fe390a4a7 --- /dev/null +++ b/cmd/kepctl/cmd/query.go @@ -0,0 +1,116 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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" + + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "k8s.io/enhancements/pkg/kepctl" +) + +// TODO: Struct literal instead? +var queryOpts = kepctl.QueryOpts{} + +var queryCmd = &cobra.Command{ + Use: "query", + Short: "Query KEPs", + Long: "Query the local filesystem, and optionally GitHub PRs for KEPs", + Example: ` kepctl query --sig architecture --status provisional --include-prs`, + SilenceUsage: true, + SilenceErrors: true, + PreRunE: func(*cobra.Command, []string) error { + return queryOpts.Validate() + }, + RunE: func(*cobra.Command, []string) error { + return runQuery(queryOpts) + }, +} + +func init() { + // TODO: Should these all be global args? + queryCmd.PersistentFlags().StringSliceVar( + &queryOpts.SIG, + "sig", + nil, + "SIG. If not specified, KEPs from all SIGs are shown.", + ) + + queryCmd.PersistentFlags().StringSliceVar( + &queryOpts.Status, + "status", + nil, + "Status", + ) + + queryCmd.PersistentFlags().StringSliceVar( + &queryOpts.Stage, + "stage", + nil, + "Stage", + ) + + queryCmd.PersistentFlags().StringSliceVar( + &queryOpts.PRRApprover, + "prr", + nil, + "Prod Readiness Approver", + ) + + queryCmd.PersistentFlags().StringSliceVar( + &queryOpts.Approver, + "approver", + nil, + "Approver", + ) + + queryCmd.PersistentFlags().StringSliceVar( + &queryOpts.Author, + "author", + nil, + "Author", + ) + + queryCmd.PersistentFlags().BoolVar( + &queryOpts.IncludePRs, + "include-prs", + false, + "Include PRs in the results", + ) + + queryCmd.PersistentFlags().StringVar( + &queryOpts.Output, + "output", + kepctl.DefaultOutputOpt, + fmt.Sprintf( + "Output format. Can be %v", kepctl.SupportedOutputOpts, + ), + ) + + rootCmd.AddCommand(queryCmd) +} + +func runQuery(queryOpts kepctl.QueryOpts) error { + k, err := kepctl.New(queryOpts.RepoPath) + if err != nil { + return errors.Wrap(err, "creating kepctl client") + } + + return k.Query(&queryOpts) +} diff --git a/cmd/kepctl/cmd/root.go b/cmd/kepctl/cmd/root.go new file mode 100644 index 000000000000..e9b7a378798f --- /dev/null +++ b/cmd/kepctl/cmd/root.go @@ -0,0 +1,73 @@ +/* +Copyright 2021 The Kubernetes Authors. + +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" + "os" + + "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + + "k8s.io/enhancements/pkg/kepctl" + "k8s.io/release/pkg/log" +) + +// rootCmd represents the base command when called without any subcommands +var rootCmd = &cobra.Command{ + Use: "kepctl", + Short: "kepctl helps you build KEPs", + PersistentPreRunE: initLogging, +} + +var rootOpts = &kepctl.CommonArgs{} + +// Execute adds all child commands to the root command and sets flags appropriately. +// This is called by main.main(). It only needs to happen once to the rootCmd. +func Execute() { + if err := rootCmd.Execute(); err != nil { + logrus.Fatal(err) + } +} + +func init() { + rootCmd.PersistentFlags().StringVar( + &rootOpts.LogLevel, + "log-level", + "info", + fmt.Sprintf("the logging verbosity, either %s", log.LevelNames()), + ) + + // TODO: This should be defaulted in the package instead + rootCmd.PersistentFlags().StringVar( + &rootOpts.RepoPath, + "repo-path", + os.Getenv("ENHANCEMENTS_PATH"), + "path to kubernetes/enhancements", + ) + + rootCmd.PersistentFlags().StringVar( + &rootOpts.TokenPath, + "gh-token-path", + "", + "path to a file with a GitHub API token", + ) +} + +func initLogging(*cobra.Command, []string) error { + return log.SetupGlobalLogger(rootOpts.LogLevel) +} diff --git a/cmd/kepctl/create.go b/cmd/kepctl/create.go deleted file mode 100644 index 8718b83941fb..000000000000 --- a/cmd/kepctl/create.go +++ /dev/null @@ -1,51 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -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 main - -import ( - "github.com/spf13/cobra" - "k8s.io/enhancements/pkg/kepctl" -) - -func buildCreateCommand(k *kepctl.Client) *cobra.Command { - - opts := kepctl.CreateOpts{} - cmd := &cobra.Command{ - Use: "create [KEP]", - Short: "Create a new KEP", - Long: "Create a new KEP using the current KEP template for the given type", - Example: ` kepctl create sig-architecture/000-mykep`, - PreRunE: func(cmd *cobra.Command, args []string) error { - return opts.Validate(args) - }, - RunE: func(cmd *cobra.Command, args []string) error { - return k.Create(opts) - }, - } - - f := cmd.Flags() - f.StringVar(&opts.Title, "title", "", "KEP Title") - f.StringArrayVar(&opts.Authors, "authors", []string{}, "Authors") - f.StringArrayVar(&opts.Reviewers, "reviewers", []string{}, "Reviewers") - f.StringVar(&opts.Type, "type", "feature", "KEP Type") - f.StringVarP(&opts.State, "state", "s", "provisional", "KEP State") - f.StringArrayVar(&opts.SIGS, "sigs", []string{}, "Participating SIGs") - f.StringArrayVar(&opts.PRRApprovers, "prr-approver", []string{}, "PRR Approver") - - addRepoPathFlag(f, &opts.CommonArgs) - return cmd -} diff --git a/cmd/kepctl/main.go b/cmd/kepctl/main.go index 7a37cb287484..568af49a197c 100644 --- a/cmd/kepctl/main.go +++ b/cmd/kepctl/main.go @@ -16,39 +16,8 @@ limitations under the License. package main -import ( - "fmt" - "os" - - "github.com/spf13/cobra" - "k8s.io/enhancements/pkg/kepctl" -) +import "k8s.io/enhancements/cmd/kepctl/cmd" func main() { - cmd, err := buildMainCommand() - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - if err := cmd.Execute(); err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } -} - -func buildMainCommand() (*cobra.Command, error) { - repoPath := os.Getenv("ENHANCEMENTS_PATH") - k, err := kepctl.New(repoPath) - if err != nil { - return nil, err - } - var rootCmd = &cobra.Command{ - Use: "kepctl", - Short: "kepctl helps you build keps", - } - - rootCmd.AddCommand(buildCreateCommand(k)) - rootCmd.AddCommand(buildPromoteCommand(k)) - rootCmd.AddCommand(buildQueryCommand(k)) - return rootCmd, nil + cmd.Execute() } diff --git a/cmd/kepctl/promote.go b/cmd/kepctl/promote.go deleted file mode 100644 index bd5bfdcf6b54..000000000000 --- a/cmd/kepctl/promote.go +++ /dev/null @@ -1,46 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -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 main - -import ( - "github.com/spf13/cobra" - "k8s.io/enhancements/pkg/kepctl" -) - -func buildPromoteCommand(k *kepctl.Client) *cobra.Command { - opts := kepctl.PromoteOpts{} - cmd := &cobra.Command{ - Use: "promote [KEP]", - Short: "Promote a KEP", - Long: "Promote a KEP to a new stage for a target release", - Example: ` kepctl promote sig-architecture/000-mykep --stage beta --release v1.20`, - PreRunE: func(cmd *cobra.Command, args []string) error { - return opts.Validate(args) - }, - RunE: func(cmd *cobra.Command, args []string) error { - return k.Promote(opts) - }, - } - - f := cmd.Flags() - f.StringVarP(&opts.Stage, "stage", "s", "", "KEP Stage") - f.StringVarP(&opts.Release, "release", "r", "", "Target Release") - - addRepoPathFlag(f, &opts.CommonArgs) - - return cmd -} diff --git a/cmd/kepctl/query.go b/cmd/kepctl/query.go deleted file mode 100644 index f6efb0b29aa4..000000000000 --- a/cmd/kepctl/query.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -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 main - -import ( - "fmt" - - "github.com/spf13/cobra" - - "k8s.io/enhancements/pkg/kepctl" -) - -func buildQueryCommand(k *kepctl.Client) *cobra.Command { - opts := kepctl.QueryOpts{} - cmd := &cobra.Command{ - Use: "query", - Short: "Query KEPs", - Long: "Query the local filesystem, and optionally GitHub PRs for KEPs", - Example: ` kepctl query --sig architecture --status provisional --include-prs`, - PreRunE: func(cmd *cobra.Command, args []string) error { - return opts.Validate() - }, - RunE: func(cmd *cobra.Command, args []string) error { - return k.Query(opts) - }, - } - - f := cmd.Flags() - f.StringSliceVar(&opts.SIG, "sig", nil, "SIG. If not specified, KEPs from all SIGs are shown.") - f.StringSliceVar(&opts.Status, "status", nil, "Status") - f.StringSliceVar(&opts.Stage, "stage", nil, "Stage") - f.StringSliceVar(&opts.PRRApprover, "prr", nil, "Prod Readiness Approver") - f.StringSliceVar(&opts.Approver, "approver", nil, "Approver") - f.StringSliceVar(&opts.Author, "author", nil, "Author") - f.BoolVar(&opts.IncludePRs, "include-prs", false, "Include PRs in the results") - f.StringVar(&opts.Output, "output", kepctl.DefaultOutputOpt, fmt.Sprintf("Output format. Can be %v", kepctl.SupportedOutputOpts)) - - addRepoPathFlag(f, &opts.CommonArgs) - - return cmd -} diff --git a/cmd/kepify/main.go b/cmd/kepify/main.go index 133f982ae3be..7acf8f0907ae 100644 --- a/cmd/kepify/main.go +++ b/cmd/kepify/main.go @@ -26,7 +26,7 @@ import ( "strings" "k8s.io/enhancements/api" - "k8s.io/enhancements/pkg/kepval/keps" + "k8s.io/enhancements/pkg/legacy/keps" ) func Usage() { @@ -44,7 +44,7 @@ func main() { flag.Usage = Usage flag.Parse() - if len(*dirPath) == 0 { + if *dirPath == "" { fmt.Fprintf(os.Stderr, "please specify the root directory for KEPs using '--dir'\n") os.Exit(1) } @@ -53,12 +53,12 @@ func main() { os.Exit(1) } - if len(*filePath) == 0 { + if *filePath == "" { fmt.Fprintf(os.Stderr, "please specify the file path for the output json using '--output'\n") os.Exit(1) } - // Find all the keps + // Find all of the KEPs files, err := findMarkdownFiles(dirPath) if err != nil { fmt.Fprintf(os.Stderr, "unable to find markdown files: %v\n", err) @@ -111,14 +111,16 @@ func parseFiles(files []string) (api.Proposals, error) { parser := &keps.Parser{} file, err := os.Open(filename) if err != nil { - return nil, fmt.Errorf("could not open file: %v\n", err) + return nil, fmt.Errorf("could not open file: %v", err) } + defer file.Close() kep := parser.Parse(file) // if error is nil we can move on if kep.Error != nil { - return nil, fmt.Errorf("%v has an error: %q\n", filename, kep.Error.Error()) + return nil, fmt.Errorf("%v has an error: %q", filename, kep.Error.Error()) } + fmt.Printf(">>>> parsed file successfully: %s\n", filename) proposals.AddProposal(kep) } @@ -147,11 +149,13 @@ func printJSONOutput(filePath string, proposals api.Proposals) error { return nil } -/// ignore certain files in the keps/ subdirectory +// TODO: Consider replacing with a .kepignore file +// ignore certain files in the keps/ subdirectory func ignore(name string) bool { if !strings.HasSuffix(name, "md") { return true } + if name == "0023-documentation-for-images.md" || name == "0004-cloud-provider-template.md" || name == "0001a-meta-kep-implementation.md" || @@ -161,5 +165,6 @@ func ignore(name string) bool { name == "kep-faq.md" { return true } + return false } diff --git a/go.mod b/go.mod index cc102786cf54..a7fbcc02be6a 100644 --- a/go.mod +++ b/go.mod @@ -3,16 +3,18 @@ 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.v2 v2.4.0 + gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c k8s.io/release v0.7.0 k8s.io/test-infra v0.0.0-20200813194141-e9678d500461 sigs.k8s.io/yaml v1.2.0 diff --git a/go.sum b/go.sum index 4002b3da24e7..fd5a174a0fdc 100644 --- a/go.sum +++ b/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= diff --git a/hack/verify-kep-metadata.sh b/hack/verify-kep-metadata.sh index 425ac8b81e76..23e4932800b1 100755 --- a/hack/verify-kep-metadata.sh +++ b/hack/verify-kep-metadata.sh @@ -23,4 +23,4 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)" cd "${ROOT}" # run the tests -GO111MODULE=on go test ./cmd/kepval/... +GO111MODULE=on go test ./test/metadata_test.go diff --git a/pkg/kepctl/create.go b/pkg/kepctl/create.go index 9942434a7362..7c3eeec11569 100644 --- a/pkg/kepctl/create.go +++ b/pkg/kepctl/create.go @@ -17,7 +17,6 @@ limitations under the License. package kepctl import ( - "errors" "fmt" "io/ioutil" "os" @@ -25,6 +24,8 @@ import ( "strings" "time" + "github.com/pkg/errors" + "k8s.io/enhancements/api" ) @@ -47,22 +48,25 @@ func (c *CreateOpts) Validate(args []string) error { if err != nil { return err } + if len(c.PRRApprovers) == 0 { - return errors.New("Must provide at least one PRR Approver") + return errors.New("must provide at least one PRR Approver") } + return nil } // Create builds a new KEP based on the README.md and kep.yaml templates in the // path specified by the command args. CreateOpts is used to populate the template -func (c *Client) Create(opts CreateOpts) error { - +func (c *Client) Create(opts *CreateOpts) error { fmt.Fprintf(c.Out, "Creating KEP %s %s %s\n", opts.SIG, opts.Number, opts.Name) - repoPath, err := c.findEnhancementsRepo(opts.CommonArgs) + + repoPath, err := c.findEnhancementsRepo(&opts.CommonArgs) fmt.Fprintf(c.Out, "Looking for enhancements repo in %s\n", repoPath) if err != nil { return fmt.Errorf("unable to create KEP: %s", err) } + t, err := c.getKepTemplate(repoPath) if err != nil { return err @@ -74,6 +78,7 @@ func (c *Client) Create(opts CreateOpts) error { if err != nil { return err } + err = c.createKEP(t, opts) if err != nil { return err @@ -82,7 +87,7 @@ func (c *Client) Create(opts CreateOpts) error { return nil } -func updateTemplate(t *api.Proposal, opts CreateOpts) { +func updateTemplate(t *api.Proposal, opts *CreateOpts) { if opts.State != "" { t.Status = opts.State } @@ -100,8 +105,10 @@ func updateTemplate(t *api.Proposal, opts CreateOpts) { if !strings.HasPrefix(author, "@") { author = fmt.Sprintf("@%s", author) } + authors = append(authors, author) } + t.Authors = authors } @@ -114,13 +121,15 @@ func updateTemplate(t *api.Proposal, opts CreateOpts) { } t.OwningSIG = opts.SIG + + // TODO(lint): appendAssign: append result not assigned to the same slice (gocritic) + //nolint:gocritic t.ParticipatingSIGs = append(opts.SIGS, opts.SIG) t.Filename = opts.Name t.LastUpdated = "v1.19" if len(opts.PRRApprovers) > 0 { t.PRRApprovers = updatePersonReference(opts.PRRApprovers) } - } func updatePersonReference(names []string) []string { @@ -134,25 +143,29 @@ func updatePersonReference(names []string) []string { return persons } -func (c *Client) createKEP(kep *api.Proposal, opts CreateOpts) error { - +func (c *Client) createKEP(kep *api.Proposal, opts *CreateOpts) error { fmt.Fprintf(c.Out, "Generating new KEP %s in %s ===>\n", opts.Name, opts.SIG) - err := c.writeKEP(kep, opts.CommonArgs) + args := &opts.CommonArgs + err := c.writeKEP(kep, args) if err != nil { return fmt.Errorf("unable to create KEP: %s", err) } - path, err := c.findEnhancementsRepo(opts.CommonArgs) + + path, err := c.findEnhancementsRepo(args) if err != nil { return err } + b, err := c.getReadmeTemplate(path) if err != nil { return fmt.Errorf("couldn't find README template: %s", err) } newPath := filepath.Join(path, "keps", opts.SIG, opts.Name, "README.md") - ioutil.WriteFile(newPath, b, os.ModePerm) + if writeErr := ioutil.WriteFile(newPath, b, os.ModePerm); writeErr != nil { + return errors.Wrapf(writeErr, "writing KEP data to file") + } return nil } diff --git a/pkg/kepctl/create_test.go b/pkg/kepctl/create_test.go index 9eadba9c064d..58773bcbb8b8 100644 --- a/pkg/kepctl/create_test.go +++ b/pkg/kepctl/create_test.go @@ -23,7 +23,6 @@ import ( "path/filepath" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/enhancements/api" @@ -31,7 +30,6 @@ import ( ) func TestWriteKep(t *testing.T) { - testcases := []struct { name string kepFile string @@ -84,32 +82,37 @@ func TestWriteKep(t *testing.T) { if repoPath == "" { repoPath = tc.opts.RepoPath } + repoPath = filepath.Join(tempDir, repoPath) c := newTestClient(t, repoPath) + b, err := ioutil.ReadFile(tc.kepFile) require.NoError(t, err) + var p api.Proposal err = yaml.Unmarshal(b, &p) require.NoError(t, err) + tc.opts.CommonArgs.RepoPath = repoPath - err = c.writeKEP(&p, tc.opts.CommonArgs) + err = c.writeKEP(&p, &tc.opts.CommonArgs) + if tc.expectError { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) + computedPath := filepath.Join(tempDir, tc.expectedPath) dirStat, err := os.Stat(computedPath) - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, dirStat) - assert.True(t, dirStat.IsDir()) + require.True(t, dirStat.IsDir()) p := filepath.Join(computedPath, "kep.yaml") fileStat, err := os.Stat(p) - assert.NoError(t, err) - assert.NotNil(t, fileStat) + require.NoError(t, err) + require.NotNil(t, fileStat) } }) } - } type testClient struct { @@ -129,6 +132,7 @@ func newTestClient(t *testing.T, repoPath string) testClient { }, } + // TODO: Parameterize tc.addTemplate("kep.yaml") tc.addTemplate("README.md") return tc diff --git a/pkg/kepctl/kepctl.go b/pkg/kepctl/kepctl.go index d77fa3580a98..03833d506b77 100644 --- a/pkg/kepctl/kepctl.go +++ b/pkg/kepctl/kepctl.go @@ -32,22 +32,27 @@ import ( "github.com/google/go-github/v32/github" "github.com/olekukonko/tablewriter" + "github.com/pkg/errors" "golang.org/x/oauth2" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "k8s.io/enhancements/api" - "k8s.io/enhancements/pkg/kepval/keps" - "k8s.io/enhancements/pkg/kepval/prrs" + "k8s.io/enhancements/pkg/legacy/keps" + "k8s.io/enhancements/pkg/legacy/prrs" "k8s.io/test-infra/prow/git" ) type CommonArgs struct { - RepoPath string //override the default settings + // command options + LogLevel string + RepoPath string // override the default settings TokenPath string - KEP string //KEP name sig-xxx/xxx-name - Name string - Number string - SIG string + + // KEP options + KEP string // KEP name sig-xxx/xxx-name + Name string + Number string + SIG string } func (c *CommonArgs) validateAndPopulateKEP(args []string) error { @@ -56,11 +61,12 @@ func (c *CommonArgs) validateAndPopulateKEP(args []string) error { } if len(args) == 1 { kep := args[0] - re := regexp.MustCompile("([a-z\\-]+)/((\\d+)-.+)") + re := regexp.MustCompile(`([a-z\\-]+)/((\\d+)-.+)`) matches := re.FindStringSubmatch(kep) if matches == nil || len(matches) != 4 { return fmt.Errorf("invalid KEP name: %s", kep) } + c.KEP = kep c.SIG = matches[1] c.Number = matches[3] @@ -79,7 +85,7 @@ type Client struct { Err io.Writer } -func (c *Client) findEnhancementsRepo(opts CommonArgs) (string, error) { +func (c *Client) findEnhancementsRepo(opts *CommonArgs) (string, error) { dir := c.RepoPath if opts.RepoPath != "" { dir = opts.RepoPath @@ -115,14 +121,16 @@ func New(repo string) (*Client, error) { }, nil } -func (c *Client) SetGitHubToken(opts CommonArgs) error { +func (c *Client) SetGitHubToken(opts *CommonArgs) error { if opts.TokenPath != "" { token, err := ioutil.ReadFile(opts.TokenPath) if err != nil { return err } + c.Token = strings.Trim(string(token), "\n\r") } + return nil } @@ -150,11 +158,13 @@ func (c *Client) getReadmeTemplate(repoPath string) ([]byte, error) { return ioutil.ReadFile(path) } +// TODO: Unused? func validateKEP(p *api.Proposal) error { b, err := yaml.Marshal(p) if err != nil { return err } + r := bytes.NewReader(b) parser := &keps.Parser{} @@ -162,6 +172,7 @@ func validateKEP(p *api.Proposal) error { if kep.Error != nil { return fmt.Errorf("kep is invalid: %s", kep.Error) } + return nil } @@ -171,6 +182,8 @@ func findLocalKEPMeta(repoPath, sig string) ([]string, error) { "keps", sig) + // TODO(lint): importShadow: shadow of imported from 'k8s.io/enhancements/pkg/legacy/keps' package 'keps' (gocritic) + //nolint:gocritic keps := []string{} // if the sig doesn't have a dir, it has no KEPs @@ -253,18 +266,21 @@ func (c *Client) loadKEPPullRequests(sig string) ([]*api.Proposal, error) { opt.Page = resp.NextPage } - var kepPRs []*github.PullRequest + kepPRs := make([]*github.PullRequest, 10) for _, pr := range allPulls { foundKind, foundSIG := false, false sigLabel := strings.Replace(sig, "-", "/", 1) + for _, l := range pr.Labels { if *l.Name == "kind/kep" { foundKind = true } + if *l.Name == sigLabel { foundSIG = true } } + if !foundKind || !foundSIG { continue } @@ -340,7 +356,7 @@ func (c *Client) loadKEPPullRequests(sig string) ([]*api.Proposal, error) { return allKEPs, nil } -func (c *Client) readKEP(repoPath string, sig, name string) (*api.Proposal, error) { +func (c *Client) readKEP(repoPath, sig, name string) (*api.Proposal, error) { kepPath := filepath.Join( repoPath, "keps", @@ -427,17 +443,18 @@ func (c *Client) loadKEPFromOldStyle(kepPath string) (*api.Proposal, error) { return kep, nil } -func (c *Client) writeKEP(kep *api.Proposal, opts CommonArgs) error { +func (c *Client) writeKEP(kep *api.Proposal, opts *CommonArgs) error { path, err := c.findEnhancementsRepo(opts) if err != nil { return fmt.Errorf("unable to write KEP: %s", err) } + b, err := yaml.Marshal(kep) if err != nil { return fmt.Errorf("KEP is invalid: %s", err) } - os.MkdirAll( + if mkErr := os.MkdirAll( filepath.Join( path, "keps", @@ -445,9 +462,13 @@ func (c *Client) writeKEP(kep *api.Proposal, opts CommonArgs) error { opts.Name, ), os.ModePerm, - ) + ); mkErr != nil { + return errors.Wrapf(mkErr, "creating KEP directory") + } + newPath := filepath.Join(path, "keps", opts.SIG, opts.Name, "kep.yaml") fmt.Fprintf(c.Out, "writing KEP to %s\n", newPath) + return ioutil.WriteFile(newPath, b, os.ModePerm) } @@ -466,41 +487,43 @@ func (p *printConfig) Value(k *api.Proposal) string { return p.valueFunc(k) } +// TODO: Refactor out anonymous funcs var defaultConfig = map[string]printConfig{ "Authors": {"Authors", func(k *api.Proposal) string { return strings.Join(k.Authors, ", ") }}, "LastUpdated": {"Updated", func(k *api.Proposal) string { return k.LastUpdated }}, "SIG": {"SIG", func(k *api.Proposal) string { if strings.HasPrefix(k.OwningSIG, "sig-") { return k.OwningSIG[4:] - } else { - return k.OwningSIG } + + return k.OwningSIG }}, "Stage": {"Stage", func(k *api.Proposal) string { return k.Stage }}, "Status": {"Status", func(k *api.Proposal) string { return k.Status }}, "Title": {"Title", func(k *api.Proposal) string { if k.PRNumber == "" { return k.Title - } else { - return "PR#" + k.PRNumber + " - " + k.Title } + + return "PR#" + k.PRNumber + " - " + k.Title }}, "Link": {"Link", func(k *api.Proposal) string { if k.PRNumber == "" { return "https://git.k8s.io/enhancements/keps/" + k.OwningSIG + "/" + k.Name - } else { - return "https://github.com/kubernetes/enhancements/pull/" + k.PRNumber } + + return "https://github.com/kubernetes/enhancements/pull/" + k.PRNumber }}, } func DefaultPrintConfigs(names ...string) []PrintConfig { - var configs []PrintConfig + configs := make([]PrintConfig, 10) for _, n := range names { // copy to allow it to be tweaked by the caller c := defaultConfig[n] configs = append(configs, &c) } + return configs } @@ -511,10 +534,11 @@ func (c *Client) PrintTable(configs []PrintConfig, proposals []*api.Proposal) { table := tablewriter.NewWriter(c.Out) - var headers []string + headers := make([]string, 10) for _, c := range configs { headers = append(headers, c.Title()) } + table.SetHeader(headers) table.SetAlignment(tablewriter.ALIGN_LEFT) diff --git a/pkg/kepctl/kepctl_test.go b/pkg/kepctl/kepctl_test.go index 96e7c44e71ad..6586ca302c79 100644 --- a/pkg/kepctl/kepctl_test.go +++ b/pkg/kepctl/kepctl_test.go @@ -21,9 +21,8 @@ import ( "io/ioutil" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "k8s.io/enhancements/api" ) @@ -55,11 +54,10 @@ func TestValidate(t *testing.T) { require.NoError(t, err) err = validateKEP(&p) if tc.err == nil { - assert.NoError(t, err) + require.NoError(t, err) } else { - assert.EqualError(t, err, tc.err.Error()) + require.EqualError(t, err, tc.err.Error()) } - }) } } @@ -79,7 +77,9 @@ func TestFindLocalKEPs(t *testing.T) { }, } - c, _ := New("testdata") + c, clientErr := New("testdata") + require.Nil(t, clientErr) + for i, tc := range testcases { k := c.loadLocalKEPs("testdata", tc.sig) if len(k) != len(tc.keps) { diff --git a/pkg/kepctl/promote.go b/pkg/kepctl/promote.go index 17b420103e1b..b91a763a7640 100644 --- a/pkg/kepctl/promote.go +++ b/pkg/kepctl/promote.go @@ -33,26 +33,30 @@ func (c *PromoteOpts) Validate(args []string) error { // Promote changes the stage and target release for a specified KEP based on the // values specified in PromoteOpts is used to populate the template -func (c *Client) Promote(opts PromoteOpts) error { +func (c *Client) Promote(opts *PromoteOpts) error { fmt.Fprintf(c.Out, "Updating KEP %s/%s\n", opts.SIG, opts.Name) - repoPath, err := c.findEnhancementsRepo(opts.CommonArgs) + + repoPath, err := c.findEnhancementsRepo(&opts.CommonArgs) if err != nil { return fmt.Errorf("unable to promote KEP: %s", err) } + p, err := c.readKEP(repoPath, opts.SIG, opts.Name) if err != nil { return fmt.Errorf("unable to load KEP for promotion: %s", err) } + p.Stage = opts.Stage p.LatestMilestone = opts.Release p.LastUpdated = opts.Release - err = c.writeKEP(p, opts.CommonArgs) + err = c.writeKEP(p, &opts.CommonArgs) if err != nil { return fmt.Errorf("unable to write updated KEP: %s", err) } // TODO: Implement ticketing workflow artifact generation fmt.Fprintf(c.Out, "KEP %s/%s updated\n", opts.SIG, opts.Name) + return nil } diff --git a/pkg/kepctl/query.go b/pkg/kepctl/query.go index b0c209ce10e1..fdce725be7e4 100644 --- a/pkg/kepctl/query.go +++ b/pkg/kepctl/query.go @@ -23,7 +23,7 @@ import ( "github.com/pkg/errors" "k8s.io/enhancements/api" - "k8s.io/enhancements/pkg/kepval/util" + "k8s.io/enhancements/pkg/legacy/util" ) var ( @@ -63,9 +63,11 @@ func (c *QueryOpts) Validate() error { if err != nil { return err } + if len(sigs) == 0 { - return fmt.Errorf("No SIG matches any of the passed regular expressions") + return fmt.Errorf("no SIG matches any of the passed regular expressions") } + c.SIG = sigs } else { // if no SIGs are passed, list KEPs from all SIGs @@ -77,13 +79,13 @@ func (c *QueryOpts) Validate() error { return fmt.Errorf("unsupported output format: %s. Valid values: %v", c.Output, SupportedOutputOpts) } - //TODO: check the valid values of stage, status, etc. + // TODO: check the valid values of stage, status, etc. return nil } // Query searches the local repo and possibly GitHub for KEPs // that match the search criteria. -func (c *Client) Query(opts QueryOpts) error { +func (c *Client) Query(opts *QueryOpts) error { // if output format is json/yaml, suppress other outputs // json/yaml are structured formats, logging events which // do not conform to the spec will create formatting issues @@ -98,14 +100,16 @@ func (c *Client) Query(opts QueryOpts) error { fmt.Fprintf(c.Out, "Searching for KEPs...\n") } - repoPath, err := c.findEnhancementsRepo(opts.CommonArgs) + repoPath, err := c.findEnhancementsRepo(&opts.CommonArgs) if err != nil { return errors.Wrap(err, "unable to search KEPs") } - c.SetGitHubToken(opts.CommonArgs) + if tokenErr := c.SetGitHubToken(&opts.CommonArgs); tokenErr != nil { + return errors.Wrapf(tokenErr, "setting GitHub token") + } - var allKEPs []*api.Proposal + allKEPs := make([]*api.Proposal, 10) // load the KEPs for each listed SIG for _, sig := range opts.SIG { // KEPs in the local filesystem @@ -130,7 +134,7 @@ func (c *Client) Query(opts QueryOpts) error { allowedAuthor := sliceToMap(opts.Author) allowedApprover := sliceToMap(opts.Approver) - var keps []*api.Proposal + keps := make([]*api.Proposal, 10) for _, k := range allKEPs { if len(opts.Status) > 0 && !allowedStatus[k.Status] { continue @@ -147,6 +151,7 @@ func (c *Client) Query(opts QueryOpts) error { if len(opts.Approver) > 0 && !atLeastOne(k.Approvers, allowedApprover) { continue } + keps = append(keps, k) } @@ -185,7 +190,7 @@ func sliceContains(s []string, e string) bool { // returns all strings in vals that match at least one // regexp in regexps -func selectByRegexp(vals []string, regexps []string) ([]string, error) { +func selectByRegexp(vals, regexps []string) ([]string, error) { var matches []string for _, s := range vals { for _, r := range regexps { diff --git a/pkg/kepctl/query_test.go b/pkg/kepctl/query_test.go index f3a8cf061e3f..ff3bdaf4802f 100644 --- a/pkg/kepctl/query_test.go +++ b/pkg/kepctl/query_test.go @@ -69,14 +69,14 @@ func TestValidateQueryOpt(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - var queryOpts = tc.queryOpts - var err = queryOpts.Validate() + queryOpts := tc.queryOpts + err := queryOpts.Validate() + if tc.err == nil { require.Nil(t, err) } else { require.NotNil(t, err, tc.err.Error()) } - }) } } diff --git a/pkg/kepval/keps/proposals.go b/pkg/legacy/keps/proposals.go similarity index 86% rename from pkg/kepval/keps/proposals.go rename to pkg/legacy/keps/proposals.go index 79bb2d1bd725..68ca4449384e 100644 --- a/pkg/kepval/keps/proposals.go +++ b/pkg/legacy/keps/proposals.go @@ -25,10 +25,9 @@ import ( "strings" "github.com/pkg/errors" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "k8s.io/enhancements/api" - "k8s.io/enhancements/pkg/kepval/keps/validations" ) type Parser struct{} @@ -70,12 +69,15 @@ func (p *Parser) Parse(in io.Reader) *api.Proposal { proposal.Error = errors.Wrap(err, "error unmarshaling YAML") return proposal } - if err := validations.ValidateStructure(test); err != nil { - proposal.Error = errors.Wrap(err, "error validating KEP metadata") - return proposal - } - proposal.Error = yaml.UnmarshalStrict(metadata, 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) return proposal } diff --git a/pkg/kepval/keps/proposals_test.go b/pkg/legacy/keps/proposals_test.go similarity index 97% rename from pkg/kepval/keps/proposals_test.go rename to pkg/legacy/keps/proposals_test.go index 2907bbb4f13e..6f1bc36292a7 100644 --- a/pkg/kepval/keps/proposals_test.go +++ b/pkg/legacy/keps/proposals_test.go @@ -20,7 +20,7 @@ import ( "strings" "testing" - "k8s.io/enhancements/pkg/kepval/keps" + "k8s.io/enhancements/pkg/legacy/keps" ) func TestValidParsing(t *testing.T) { diff --git a/pkg/kepval/keps/validations/yaml.go b/pkg/legacy/keps/validations/yaml.go similarity index 76% rename from pkg/kepval/keps/validations/yaml.go rename to pkg/legacy/keps/validations/yaml.go index 7070a7eda34a..15918782b734 100644 --- a/pkg/kepval/keps/validations/yaml.go +++ b/pkg/legacy/keps/validations/yaml.go @@ -21,7 +21,7 @@ import ( "sort" "strings" - "k8s.io/enhancements/pkg/kepval/util" + "k8s.io/enhancements/pkg/legacy/util" ) var ( @@ -30,9 +30,11 @@ var ( reStatus = regexp.MustCompile(strings.Join(statuses, "|")) stages = []string{"alpha", "beta", "stable"} reStages = regexp.MustCompile(strings.Join(stages, "|")) - reMilestone = regexp.MustCompile("v1\\.[1-9][0-9]*") + reMilestone = regexp.MustCompile(`v1\\.[1-9][0-9]*`) ) +// TODO(lint): cyclomatic complexity 50 of func `ValidateStructure` is high (> 30) (gocyclo) +//nolint:gocyclo func ValidateStructure(parsed map[interface{}]interface{}) error { for _, key := range mandatoryKeys { if _, found := parsed[key]; !found { @@ -54,44 +56,67 @@ func ValidateStructure(parsed map[interface{}]interface{}) error { // figure out the types switch strings.ToLower(k) { case "status": + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v, _ := value.(string) if !reStatus.Match([]byte(v)) { return util.NewValueMustBeOneOf(k, v, statuses) } + case "stage": + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v, _ := value.(string) if !reStages.Match([]byte(v)) { return util.NewValueMustBeOneOf(k, v, stages) } + case "owning-sig": + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v, _ := value.(string) index := sort.SearchStrings(listGroups, v) if index >= len(listGroups) || listGroups[index] != v { return util.NewValueMustBeOneOf(k, v, listGroups) } + // optional strings case "editor": if empty { continue } + fallthrough + case "title", "creation-date", "last-updated": + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + v, ok := value.(string) if ok && v == "" { return util.NewMustHaveOneValue(k) @@ -99,6 +124,7 @@ func ValidateStructure(parsed map[interface{}]interface{}) error { if !ok { return util.NewValueMustBeString(k, v) } + // These are optional lists, so skip if there is no value case "participating-sigs", "replaces", "superseded-by", "see-also": if empty { @@ -109,19 +135,25 @@ func ValidateStructure(parsed map[interface{}]interface{}) error { if len(v) == 0 { continue } + case interface{}: // This indicates an empty item is valid continue } + fallthrough + case "authors", "reviewers", "approvers": switch values := value.(type) { case []interface{}: if len(values) == 0 { return util.NewMustHaveAtLeastOneValue(k) } - if strings.ToLower(k) == "participating-sigs" { + + if strings.EqualFold(k, "participating-sigs") { for _, value := range values { + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v := value.(string) index := sort.SearchStrings(listGroups, v) if index >= len(listGroups) || listGroups[index] != v { @@ -129,15 +161,19 @@ func ValidateStructure(parsed map[interface{}]interface{}) error { } } } + case interface{}: return util.NewValueMustBeListOfStrings(k, values) } + case "prr-approvers": switch values := value.(type) { case []interface{}: // prrApprovers must be sorted to use SearchStrings down below... sort.Strings(prrApprovers) for _, value := range values { + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v := value.(string) if len(v) > 0 && v[0] == '@' { // If "@" is appeneded at the beginning, remove it. @@ -149,29 +185,39 @@ func ValidateStructure(parsed map[interface{}]interface{}) error { return util.NewValueMustBeOneOf(k, v, prrApprovers) } } + case interface{}: return util.NewValueMustBeListOfStrings(k, values) } + case "latest-milestone": + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v, _ := value.(string) if !reMilestone.Match([]byte(v)) { return util.NewValueMustBeMilestone(k, v) } + case "milestone": switch v := value.(type) { case map[interface{}]interface{}: if err := validateMilestone(v); err != nil { return err } + case interface{}: return util.NewValueMustBeStruct(k, v) } } } + return nil } @@ -183,12 +229,17 @@ func validateMilestone(parsed map[interface{}]interface{}) error { return util.NewKeyMustBeString(k) } + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch strings.ToLower(k) { case "alpha", "beta", "stable": switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v, _ := value.(string) if !reMilestone.Match([]byte(v)) { return util.NewValueMustBeMilestone(k, v) diff --git a/pkg/kepval/keps/validations/yaml_test.go b/pkg/legacy/keps/validations/yaml_test.go similarity index 97% rename from pkg/kepval/keps/validations/yaml_test.go rename to pkg/legacy/keps/validations/yaml_test.go index 7f4d030cb517..46bc6e08dbd4 100644 --- a/pkg/kepval/keps/validations/yaml_test.go +++ b/pkg/legacy/keps/validations/yaml_test.go @@ -21,9 +21,11 @@ import ( "html/template" "testing" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) +// TODO(lint): Fix field keys and flow flags +//nolint:govet type proposal struct { Title string `yaml:"title"` Authors []string `yaml:,flow` @@ -162,8 +164,7 @@ func TestValidateStructureSuccess(t *testing.T) { } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - - // add required fields + // TODO: Add required fields tc.input["title"] = "this is a title" tc.input["owning-sig"] = "sig-architecture" diff --git a/pkg/kepval/prrs/approvals.go b/pkg/legacy/prrs/approvals.go similarity index 57% rename from pkg/kepval/prrs/approvals.go rename to pkg/legacy/prrs/approvals.go index 6dde640e8c59..eb3fc62e65d9 100644 --- a/pkg/kepval/prrs/approvals.go +++ b/pkg/legacy/prrs/approvals.go @@ -22,32 +22,15 @@ import ( "io" "github.com/pkg/errors" - "gopkg.in/yaml.v2" - "k8s.io/enhancements/pkg/kepval/prrs/validations" -) - -type Approvals []*Approval - -func (a *Approvals) AddApproval(approval *Approval) { - *a = append(*a, approval) -} - -type Milestone struct { - Approver string `json:"approver" yaml:"approver"` -} - -type Approval struct { - Number string `json:"kep-number" yaml:"kep-number"` - Alpha Milestone `json:"alpha" yaml:"alpha"` - Beta Milestone `json:"beta" yaml:"beta"` - Stable Milestone `json:"stable" yaml:"stable"` + "gopkg.in/yaml.v3" - Error error `json:"-" yaml:"-"` -} + "k8s.io/enhancements/api" + "k8s.io/enhancements/pkg/legacy/prrs/validations" +) type Parser struct{} -func (p *Parser) Parse(in io.Reader) *Approval { +func (p *Parser) Parse(in io.Reader) *api.PRRApproval { scanner := bufio.NewScanner(in) var body bytes.Buffer for scanner.Scan() { @@ -55,7 +38,7 @@ func (p *Parser) Parse(in io.Reader) *Approval { body.WriteString(line) } - approval := &Approval{} + approval := &api.PRRApproval{} if err := scanner.Err(); err != nil { approval.Error = errors.Wrap(err, "error reading file") return approval @@ -64,7 +47,7 @@ func (p *Parser) Parse(in io.Reader) *Approval { // First do structural checks test := map[interface{}]interface{}{} if err := yaml.Unmarshal(body.Bytes(), test); err != nil { - approval.Error = errors.Wrap(err, "error unmarshaling YAML") + approval.Error = errors.Wrap(err, "error unmarshalling YAML") return approval } if err := validations.ValidateStructure(test); err != nil { @@ -72,18 +55,7 @@ func (p *Parser) Parse(in io.Reader) *Approval { return approval } - approval.Error = yaml.UnmarshalStrict(body.Bytes(), approval) - return approval -} + approval.Error = yaml.Unmarshal(body.Bytes(), approval) -func (a *Approval) ApproverForStage(stage string) string { - switch stage { - case "alpha": - return a.Alpha.Approver - case "beta": - return a.Beta.Approver - case "stable": - return a.Stable.Approver - } - return "" + return approval } diff --git a/pkg/kepval/prrs/approvals_test.go b/pkg/legacy/prrs/approvals_test.go similarity index 100% rename from pkg/kepval/prrs/approvals_test.go rename to pkg/legacy/prrs/approvals_test.go diff --git a/pkg/kepval/prrs/validations/yaml.go b/pkg/legacy/prrs/validations/yaml.go similarity index 82% rename from pkg/kepval/prrs/validations/yaml.go rename to pkg/legacy/prrs/validations/yaml.go index e2f7b81ee364..40a993b312cb 100644 --- a/pkg/kepval/prrs/validations/yaml.go +++ b/pkg/legacy/prrs/validations/yaml.go @@ -21,12 +21,10 @@ import ( "sort" "strings" - "k8s.io/enhancements/pkg/kepval/util" + "k8s.io/enhancements/pkg/legacy/util" ) -var ( - mandatoryKeys = []string{"kep-number"} -) +var mandatoryKeys = []string{"kep-number"} func ValidateStructure(parsed map[interface{}]interface{}) error { for _, key := range mandatoryKeys { @@ -71,17 +69,25 @@ func validateMilestone(parsed map[interface{}]interface{}) error { } // figure out the types + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch strings.ToLower(k) { case "approver": + // TODO(lint): singleCaseSwitch: should rewrite switch statement to if statement (gocritic) + //nolint:gocritic switch v := value.(type) { case []interface{}: return util.NewValueMustBeString(k, v) } + + // TODO(lint): Error return value is not checked (errcheck) + //nolint:errcheck v, _ := value.(string) if len(v) > 0 && v[0] == '@' { - // If "@" is appeneded at the beginning, remove it. + // If "@" is appended at the beginning, remove it. v = v[1:] } + index := sort.SearchStrings(prrApprovers, v) if index >= len(prrApprovers) || prrApprovers[index] != v { return util.NewValueMustBeOneOf(k, v, prrApprovers) diff --git a/pkg/kepval/prrs/validations/yaml_test.go b/pkg/legacy/prrs/validations/yaml_test.go similarity index 94% rename from pkg/kepval/prrs/validations/yaml_test.go rename to pkg/legacy/prrs/validations/yaml_test.go index ef430d81269e..a02ba46c3666 100644 --- a/pkg/kepval/prrs/validations/yaml_test.go +++ b/pkg/legacy/prrs/validations/yaml_test.go @@ -20,14 +20,13 @@ import ( "testing" ) - func TestValidateStructureSuccess(t *testing.T) { testcases := []struct { name string input map[interface{}]interface{} }{ { - name: "all milestones optional", + name: "all milestones optional", input: map[interface{}]interface{}{}, }, { @@ -86,14 +85,14 @@ func TestValidateStructureFailures(t *testing.T) { }, }, { - name: "invalid milestone", + name: "invalid milestone", input: map[interface{}]interface{}{ "kep-number": "12345", - "alpha": "@wojtek-t", + "alpha": "@wojtek-t", }, }, { - name: "invalid approver", + name: "invalid approver", input: map[interface{}]interface{}{ "kep-number": "12345", "alpha": map[interface{}]interface{}{ diff --git a/pkg/kepval/util/errors.go b/pkg/legacy/util/errors.go similarity index 100% rename from pkg/kepval/util/errors.go rename to pkg/legacy/util/errors.go diff --git a/pkg/kepval/util/helpers.go b/pkg/legacy/util/helpers.go similarity index 96% rename from pkg/kepval/util/helpers.go rename to pkg/legacy/util/helpers.go index 8b5850cc90d7..e790f50ffaf3 100644 --- a/pkg/kepval/util/helpers.go +++ b/pkg/legacy/util/helpers.go @@ -105,9 +105,8 @@ func fetchPRRApprovers() ([]string, error) { return nil, fmt.Errorf("unable to read parse aliases content: %v", err) } var result []string - for _, approver := range config.Data["prod-readiness-approvers"] { - result = append(result, approver) - } + result = append(result, config.Data["prod-readiness-approvers"]...) + sort.Strings(result) return result, nil } diff --git a/test/OWNERS b/test/OWNERS new file mode 100644 index 000000000000..3eca1e29d9cb --- /dev/null +++ b/test/OWNERS @@ -0,0 +1,9 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +approvers: + - kep-tools-approvers +reviewers: + - kep-tools-reviewers + +labels: + - area/enhancements diff --git a/cmd/kepval/main_test.go b/test/metadata_test.go similarity index 75% rename from cmd/kepval/main_test.go rename to test/metadata_test.go index 3f871be34485..d0532b5b502d 100644 --- a/cmd/kepval/main_test.go +++ b/test/metadata_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package main +package test import ( "os" @@ -22,8 +22,9 @@ import ( "strings" "testing" - "k8s.io/enhancements/pkg/kepval/keps" - "k8s.io/enhancements/pkg/kepval/prrs" + "github.com/stretchr/testify/require" + + "k8s.io/enhancements/api" ) const ( @@ -32,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 - }, + filepath.Join("..", kepsDir), + walkFn, ) // This indicates a problem walking the filepath, not a validation error. if err != nil { @@ -73,9 +51,9 @@ func TestValidation(t *testing.T) { t.Fatal("must find more than 0 keps") } - kepParser := &keps.Parser{} - prrParser := &prrs.Parser{} - prrsDir := filepath.Join("..", "..", prrsDir) + kepHandler := &api.KEPHandler{} + prrHandler := &api.PRRHandler{} + prrsDir := filepath.Join("..", prrsDir) for _, filename := range files { t.Run(filename, func(t *testing.T) { @@ -83,9 +61,12 @@ 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) + kep, kepParseErr := kepHandler.Parse(kepFile) + require.Nil(t, kepParseErr) + if kep.Error != nil { t.Errorf("%v has an error: %v", filename, kep.Error) } @@ -114,10 +95,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,9 +117,10 @@ func TestValidation(t *testing.T) { case "stable": stagePRRApprover = prr.Stable.Approver } + if len(stageMilestone) > 0 && stageMilestone >= "v1.21" { // PRR approval is needed. - if len(stagePRRApprover) == 0 { + if stagePRRApprover == "" { t.Errorf("PRR approval is required to target milestone %v (stage %v)", stageMilestone, kep.Stage) t.Errorf("For more details about PRR approval see: https://github.com/kubernetes/community/blob/master/sig-architecture/production-readiness.md") t.Errorf("To get PRR approval modify appropriately file %s and have this approved by PRR team", prrFilename) @@ -144,9 +130,41 @@ 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 func ignore(dir, name string) bool { - if dir == "../../keps/NNNN-kep-template" { + if dir == "../keps/NNNN-kep-template" { return true // ignore the template directory because its metadata file does not use a valid sig name }