From 2bffa4ecd8b260918da85dbdc890cc1db4c67e19 Mon Sep 17 00:00:00 2001 From: lucianpy Date: Fri, 28 Oct 2022 14:34:24 +0300 Subject: [PATCH 1/6] Add changelog_lint cmd --- cmd/changelog_lint.go | 68 ++++++++++++++++++++++ internal/changelog/linter.go | 106 +++++++++++++++++++++++++++++++++++ main.go | 1 + 3 files changed, 175 insertions(+) create mode 100644 cmd/changelog_lint.go create mode 100644 internal/changelog/linter.go diff --git a/cmd/changelog_lint.go b/cmd/changelog_lint.go new file mode 100644 index 0000000..82ddd9e --- /dev/null +++ b/cmd/changelog_lint.go @@ -0,0 +1,68 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package cmd + +import ( + "fmt" + "log" + + "github.com/elastic/elastic-agent-changelog-tool/internal/changelog" + "github.com/spf13/afero" + "github.com/spf13/cobra" + "github.com/spf13/viper" +) + +func ChangelogLintCmd(fs afero.Fs) *cobra.Command { + + lintCmd := &cobra.Command{ + Use: "changelog_lint", + Short: "Lint the consolidated changelog", + Args: func(cmd *cobra.Command, args []string) error { + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + + // src := viper.GetString("fragment_location") + dest := viper.GetString("changelog_destination") + + version, err := cmd.Flags().GetString("version") + if err != nil { + return fmt.Errorf("error parsing flag 'version': %w", err) + } + + relaxed, err := cmd.Flags().GetBool("relaxed") + if err != nil { + return fmt.Errorf("error parsing flag 'relaxed': %w", err) + } + + linter := changelog.NewLinter(fs) + errs := linter.Lint(dest, version) + + for _, err := range errs { + log.Println(err) + } + + if !relaxed && len(errs) > 0 { + log.Fatal("Linting failed.") + } + + log.Println("Linting done.") + + return nil + }, + } + + lintCmd.Flags().VisitAll(viperOverrides(lintCmd)) + + lintCmd.Flags().String("version", "", "The version of the consolidated changelog being created") + lintCmd.Flags().Bool("relaxed", false, "Relaxed mode will only log erros, without terminating execution") + err := lintCmd.MarkFlagRequired("version") + if err != nil { + // NOTE: the only case this error appear is when the flag is not defined + log.Fatal(err) + } + + return lintCmd +} diff --git a/internal/changelog/linter.go b/internal/changelog/linter.go new file mode 100644 index 0000000..a7f57ba --- /dev/null +++ b/internal/changelog/linter.go @@ -0,0 +1,106 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package changelog + +import ( + "fmt" + + "github.com/spf13/afero" + "github.com/spf13/viper" +) + +func NewLinter(fs afero.Fs) Linter { + return newLinter(fs) +} + +type Linter struct { + fs afero.Fs + validators map[string]func(entry Entry) error + errors []error +} + +func newLinter(fs afero.Fs) Linter { + return Linter{ + fs: fs, + validators: map[string]func(entry Entry) error{ + "multiplePRIDs": func(entry Entry) error { + if len(entry.LinkedPR) > 1 { + return fmt.Errorf("changelog entry: %s has multiple PR ids", entry.File.Name) + } + + return nil + }, + "noPRIDs": func(entry Entry) error { + if len(entry.LinkedPR) == 0 { + return fmt.Errorf("changelog entry: %s has no PR id", entry.File.Name) + } + + return nil + }, + "noIssueID": func(entry Entry) error { + if len(entry.LinkedIssue) == 0 { + return fmt.Errorf("changelog entry: %s has no issue id", entry.File.Name) + } + + return nil + }, + // multiple issues check? + "validComponent": func(entry Entry) error { + configComponents := viper.GetStringSlice("components") + + switch len(configComponents) { + case 0: + return nil + case 1: + c := configComponents[0] + + if c != entry.Component && len(entry.Component) > 0 { + return fmt.Errorf("Component [%s] not found in config", entry.Component) + } + default: + var match string + + if entry.Component == "" { + return fmt.Errorf("Component cannot be assumed, choose it from config values: %s", entry.File.Name) + } + + match = "" + for _, c := range configComponents { + if entry.Component != c { + continue + } + match = entry.Component + } + + if match == "" { + return fmt.Errorf("Component [%s] not found in config", entry.Component) + } + } + + return nil + }, + }, + } +} + +type LinterErrors []error + +func (l Linter) Lint(dest, version string) []error { + c, err := FromFile(l.fs, fmt.Sprintf("./%s/%s.yaml", dest, version)) + if err != nil { + return []error{fmt.Errorf("error loading changelog from file: %w", err)} + } + + for _, entry := range c.Entries { + for _, validator := range l.validators { + err := validator(entry) + if err != nil { + l.errors = append(l.errors, err) + } + } + } + + return l.errors +} diff --git a/main.go b/main.go index 694d6ff..99c2122 100644 --- a/main.go +++ b/main.go @@ -25,6 +25,7 @@ func main() { rootCmd.AddCommand(cmd.PrHasFragmentCommand(appFs)) rootCmd.AddCommand(cmd.RenderCmd(appFs)) rootCmd.AddCommand(cmd.VersionCmd()) + rootCmd.AddCommand(cmd.ChangelogLintCmd(appFs)) err := rootCmd.Execute() if err != nil { From 6422a8b70c4c6713a8415e05856f0291c409fcb9 Mon Sep 17 00:00:00 2001 From: lucianpy Date: Fri, 28 Oct 2022 14:35:17 +0300 Subject: [PATCH 2/6] Add fragment --- ...666956886-Add-changelong-lint-command.yaml | 31 +++++++++++++++++++ cmd/changelog_lint.go | 2 -- 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 changelog/fragments/1666956886-Add-changelong-lint-command.yaml diff --git a/changelog/fragments/1666956886-Add-changelong-lint-command.yaml b/changelog/fragments/1666956886-Add-changelong-lint-command.yaml new file mode 100644 index 0000000..6b4fbbf --- /dev/null +++ b/changelog/fragments/1666956886-Add-changelong-lint-command.yaml @@ -0,0 +1,31 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: feature + +# Change summary; a 80ish characters long description of the change. +summary: Add changelong lint command to validate the fields + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +#description: + +# Affected component; a word indicating the component this changeset affects. +component: + +# PR number; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +#pr: 1234 + +# Issue number; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +#issue: 1234 diff --git a/cmd/changelog_lint.go b/cmd/changelog_lint.go index 82ddd9e..77f4b00 100644 --- a/cmd/changelog_lint.go +++ b/cmd/changelog_lint.go @@ -23,8 +23,6 @@ func ChangelogLintCmd(fs afero.Fs) *cobra.Command { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - - // src := viper.GetString("fragment_location") dest := viper.GetString("changelog_destination") version, err := cmd.Flags().GetString("version") From 3aee0d18b9955f06f15b9dabc6e70f44fbe3fdc5 Mon Sep 17 00:00:00 2001 From: lucianpy Date: Fri, 28 Oct 2022 14:48:59 +0300 Subject: [PATCH 3/6] Update flag description --- cmd/changelog_lint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/changelog_lint.go b/cmd/changelog_lint.go index 77f4b00..dad8e8d 100644 --- a/cmd/changelog_lint.go +++ b/cmd/changelog_lint.go @@ -54,7 +54,7 @@ func ChangelogLintCmd(fs afero.Fs) *cobra.Command { lintCmd.Flags().VisitAll(viperOverrides(lintCmd)) - lintCmd.Flags().String("version", "", "The version of the consolidated changelog being created") + lintCmd.Flags().String("version", "", "The version of the consolidated changelog subject to linting") lintCmd.Flags().Bool("relaxed", false, "Relaxed mode will only log erros, without terminating execution") err := lintCmd.MarkFlagRequired("version") if err != nil { From 2146959fe7a45f8685e8bd6b4d4550f81f259d79 Mon Sep 17 00:00:00 2001 From: Edoardo Tenani Date: Mon, 31 Oct 2022 17:41:29 +0100 Subject: [PATCH 4/6] extract validators --- internal/changelog/linter.go | 138 +++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 63 deletions(-) diff --git a/internal/changelog/linter.go b/internal/changelog/linter.go index a7f57ba..2337caf 100644 --- a/internal/changelog/linter.go +++ b/internal/changelog/linter.go @@ -16,72 +16,15 @@ func NewLinter(fs afero.Fs) Linter { } type Linter struct { - fs afero.Fs - validators map[string]func(entry Entry) error - errors []error + fs afero.Fs + entryValidators entryValidators + errors []error } func newLinter(fs afero.Fs) Linter { return Linter{ - fs: fs, - validators: map[string]func(entry Entry) error{ - "multiplePRIDs": func(entry Entry) error { - if len(entry.LinkedPR) > 1 { - return fmt.Errorf("changelog entry: %s has multiple PR ids", entry.File.Name) - } - - return nil - }, - "noPRIDs": func(entry Entry) error { - if len(entry.LinkedPR) == 0 { - return fmt.Errorf("changelog entry: %s has no PR id", entry.File.Name) - } - - return nil - }, - "noIssueID": func(entry Entry) error { - if len(entry.LinkedIssue) == 0 { - return fmt.Errorf("changelog entry: %s has no issue id", entry.File.Name) - } - - return nil - }, - // multiple issues check? - "validComponent": func(entry Entry) error { - configComponents := viper.GetStringSlice("components") - - switch len(configComponents) { - case 0: - return nil - case 1: - c := configComponents[0] - - if c != entry.Component && len(entry.Component) > 0 { - return fmt.Errorf("Component [%s] not found in config", entry.Component) - } - default: - var match string - - if entry.Component == "" { - return fmt.Errorf("Component cannot be assumed, choose it from config values: %s", entry.File.Name) - } - - match = "" - for _, c := range configComponents { - if entry.Component != c { - continue - } - match = entry.Component - } - - if match == "" { - return fmt.Errorf("Component [%s] not found in config", entry.Component) - } - } - - return nil - }, - }, + fs: fs, + entryValidators: defaultEntryValidators, } } @@ -94,7 +37,7 @@ func (l Linter) Lint(dest, version string) []error { } for _, entry := range c.Entries { - for _, validator := range l.validators { + for _, validator := range l.entryValidators { err := validator(entry) if err != nil { l.errors = append(l.errors, err) @@ -104,3 +47,72 @@ func (l Linter) Lint(dest, version string) []error { return l.errors } + +type entryValidationFn func(Entry) error +type entryValidators map[string]entryValidationFn + +var defaultEntryValidators = entryValidators{ + "pr_multipleids": validator_PRMultipleIDs, + "pr_noids": validator_PRnoIDs, + "issue_noids": validator_IssueNoIDs, + "component_valid": validator_componentValid(viper.GetStringSlice("components")), +} + +func validator_PRMultipleIDs(entry Entry) error { + if len(entry.LinkedPR) > 1 { + return fmt.Errorf("changelog entry: %s has multiple PR ids", entry.File.Name) + } + + return nil +} + +func validator_PRnoIDs(entry Entry) error { + if len(entry.LinkedPR) == 0 { + return fmt.Errorf("changelog entry: %s has no PR id", entry.File.Name) + } + + return nil +} + +func validator_IssueNoIDs(entry Entry) error { + if len(entry.LinkedIssue) == 0 { + return fmt.Errorf("changelog entry: %s has no issue id", entry.File.Name) + } + + return nil +} + +func validator_componentValid(configComponents []string) entryValidationFn { + return func(entry Entry) error { + switch len(configComponents) { + case 0: + return nil + case 1: + c := configComponents[0] + + if c != entry.Component && len(entry.Component) > 0 { + return fmt.Errorf("Component [%s] not found in config", entry.Component) + } + default: + var match string + + if entry.Component == "" { + return fmt.Errorf("Component cannot be assumed, choose it from config values: %s", entry.File.Name) + } + + match = "" + for _, c := range configComponents { + if entry.Component != c { + continue + } + match = entry.Component + } + + if match == "" { + return fmt.Errorf("Component [%s] not found in config", entry.Component) + } + } + + return nil + } +} From b32f2c0768c0b3739d12f5b9a0630f427cda3cec Mon Sep 17 00:00:00 2001 From: lucianpy Date: Wed, 2 Nov 2022 12:47:53 +0200 Subject: [PATCH 5/6] Add linter tests --- internal/changelog/linter.go | 6 +- internal/changelog/linter_test.go | 97 +++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 internal/changelog/linter_test.go diff --git a/internal/changelog/linter.go b/internal/changelog/linter.go index 2337caf..42ef146 100644 --- a/internal/changelog/linter.go +++ b/internal/changelog/linter.go @@ -91,13 +91,13 @@ func validator_componentValid(configComponents []string) entryValidationFn { c := configComponents[0] if c != entry.Component && len(entry.Component) > 0 { - return fmt.Errorf("Component [%s] not found in config", entry.Component) + return fmt.Errorf("changelog entry: %s -> component [%s] not found in config: %s", entry.File.Name, entry.Component, configComponents) } default: var match string if entry.Component == "" { - return fmt.Errorf("Component cannot be assumed, choose it from config values: %s", entry.File.Name) + return fmt.Errorf("changelog entry: %s -> component cannot be assumed, choose it from config: %s", entry.File.Name, configComponents) } match = "" @@ -109,7 +109,7 @@ func validator_componentValid(configComponents []string) entryValidationFn { } if match == "" { - return fmt.Errorf("Component [%s] not found in config", entry.Component) + return fmt.Errorf("changelog entry: %s -> component [%s] not found in config: %s", entry.File.Name, entry.Component, configComponents) } } diff --git a/internal/changelog/linter_test.go b/internal/changelog/linter_test.go new file mode 100644 index 0000000..d75a327 --- /dev/null +++ b/internal/changelog/linter_test.go @@ -0,0 +1,97 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package changelog + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidators(t *testing.T) { + testcases := []struct { + name string + entry Entry + validatorFunc func(entry Entry) error + expectedErr error + }{ + // PRMultipleIDs + { + "pr multiple ids: 1 id", + Entry{ + LinkedPR: []string{"1"}, + }, + validator_PRMultipleIDs, + nil, + }, + { + "pr multiple ids: multiple ids", + Entry{ + LinkedPR: []string{"1", "2"}, + }, + validator_PRMultipleIDs, + fmt.Errorf("changelog entry: %s has multiple PR ids", ""), + }, + // PRnoIDs + { + "pr multiple ids: error", + Entry{ + LinkedPR: []string{}, + }, + validator_PRnoIDs, + fmt.Errorf("changelog entry: %s has no PR id", ""), + }, + // IssueNoIDs + { + "issue no ids: error", + Entry{ + LinkedIssue: []string{}, + }, + validator_IssueNoIDs, + fmt.Errorf("changelog entry: %s has no issue id", ""), + }, + // ComponentValid + { + "component valid: beats", + Entry{ + Component: "beats", + }, + validator_componentValid([]string{"beats"}), + nil, + }, + { + "component valid: not found in config", + Entry{ + Component: "agent", + }, + validator_componentValid([]string{"beats"}), + fmt.Errorf("changelog entry: %s -> component [%s] not found in config: [%s]", "", "agent", "beats"), + }, + { + "component valid: no component", + Entry{ + Component: "", + }, + validator_componentValid([]string{"beats", "agent"}), + fmt.Errorf("changelog entry: %s -> component cannot be assumed, choose it from config: %s", "", []string{"beats", "agent"}), + }, + { + "component valid: invalid component", + Entry{ + Component: "invalid_component", + }, + validator_componentValid([]string{"beats"}), + fmt.Errorf("changelog entry: %s -> component [%s] not found in config: [%s]", "", "invalid_component", "beats"), + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.validatorFunc(tc.entry) + require.Equal(t, err, tc.expectedErr) + }) + } +} From df98606959706c5c8fa6e33370bef4e9eafc4cc5 Mon Sep 17 00:00:00 2001 From: lucianpy Date: Thu, 3 Nov 2022 11:23:25 +0200 Subject: [PATCH 6/6] Refactor tests --- internal/changelog/linter_test.go | 65 ++++++++++++++++++++++++++++--- main.go | 2 +- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/internal/changelog/linter_test.go b/internal/changelog/linter_test.go index d75a327..a32fd30 100644 --- a/internal/changelog/linter_test.go +++ b/internal/changelog/linter_test.go @@ -11,14 +11,13 @@ import ( "github.com/stretchr/testify/require" ) -func TestValidators(t *testing.T) { +func TestPRMultipleIDs(t *testing.T) { testcases := []struct { name string entry Entry validatorFunc func(entry Entry) error expectedErr error }{ - // PRMultipleIDs { "pr multiple ids: 1 id", Entry{ @@ -35,7 +34,23 @@ func TestValidators(t *testing.T) { validator_PRMultipleIDs, fmt.Errorf("changelog entry: %s has multiple PR ids", ""), }, - // PRnoIDs + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.validatorFunc(tc.entry) + require.Equal(t, err, tc.expectedErr) + }) + } +} + +func TestPRnoIDs(t *testing.T) { + testcases := []struct { + name string + entry Entry + validatorFunc func(entry Entry) error + expectedErr error + }{ { "pr multiple ids: error", Entry{ @@ -44,7 +59,23 @@ func TestValidators(t *testing.T) { validator_PRnoIDs, fmt.Errorf("changelog entry: %s has no PR id", ""), }, - // IssueNoIDs + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.validatorFunc(tc.entry) + require.Equal(t, err, tc.expectedErr) + }) + } +} + +func TestIssueNoIDs(t *testing.T) { + testcases := []struct { + name string + entry Entry + validatorFunc func(entry Entry) error + expectedErr error + }{ { "issue no ids: error", Entry{ @@ -53,7 +84,31 @@ func TestValidators(t *testing.T) { validator_IssueNoIDs, fmt.Errorf("changelog entry: %s has no issue id", ""), }, - // ComponentValid + { + "component valid: invalid component", + Entry{ + Component: "invalid_component", + }, + validator_componentValid([]string{"beats"}), + fmt.Errorf("changelog entry: %s -> component [%s] not found in config: [%s]", "", "invalid_component", "beats"), + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.validatorFunc(tc.entry) + require.Equal(t, err, tc.expectedErr) + }) + } +} + +func TestComponentValid(t *testing.T) { + testcases := []struct { + name string + entry Entry + validatorFunc func(entry Entry) error + expectedErr error + }{ { "component valid: beats", Entry{ diff --git a/main.go b/main.go index 99c2122..f4ec5f0 100644 --- a/main.go +++ b/main.go @@ -19,13 +19,13 @@ func main() { rootCmd := cmd.RootCmd() rootCmd.AddCommand(cmd.BuildCmd(appFs)) + rootCmd.AddCommand(cmd.ChangelogLintCmd(appFs)) rootCmd.AddCommand(cmd.CleanupCmd(appFs)) rootCmd.AddCommand(cmd.FindPRCommand(appFs)) rootCmd.AddCommand(cmd.NewCmd()) rootCmd.AddCommand(cmd.PrHasFragmentCommand(appFs)) rootCmd.AddCommand(cmd.RenderCmd(appFs)) rootCmd.AddCommand(cmd.VersionCmd()) - rootCmd.AddCommand(cmd.ChangelogLintCmd(appFs)) err := rootCmd.Execute() if err != nil {