From b1c7c4d27726095390c229b39db2c469a9d8c7cc Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 29 May 2024 15:05:11 +0200 Subject: [PATCH] more cleanups --- hack/generate-client.go | 6 +- pkg/changelog/generator.go | 52 ++++++++- pkg/changelog/generator_test.go | 32 +----- pkg/changelog/parse.go | 42 ++++--- pkg/changelog/parse_test.go | 105 ++++++++++++++++++ .../testdata/multi-typed-release-note.yaml | 19 ++++ pkg/changelog/types.go | 69 +++++++----- pkg/ranges/ranger.go | 8 +- pkg/render/json.go | 2 +- pkg/render/markdown.go | 40 ++++++- 10 files changed, 290 insertions(+), 85 deletions(-) create mode 100644 pkg/changelog/parse_test.go create mode 100644 pkg/changelog/testdata/multi-typed-release-note.yaml diff --git a/hack/generate-client.go b/hack/generate-client.go index 1bdde2a..595fa5a 100644 --- a/hack/generate-client.go +++ b/hack/generate-client.go @@ -19,8 +19,8 @@ package main import ( "bytes" "go/format" - "io/ioutil" "log" + "os" "path/filepath" "strings" "text/template" @@ -53,7 +53,7 @@ func main() { for _, templateFile := range templates { log.Printf("Rendering %s...", templateFile) - content, err := ioutil.ReadFile(templateFile) + content, err := os.ReadFile(templateFile) if err != nil { log.Fatalf("Failed to read %s -- did you run this from the root directory?: %v", templateFile, err) } @@ -70,7 +70,7 @@ func main() { filename := filepath.Join("pkg/github", strings.TrimSuffix(filepath.Base(templateFile), ".tmpl")) - err = ioutil.WriteFile(filename, source, 0644) + err = os.WriteFile(filename, source, 0644) if err != nil { log.Fatalf("Failed to write %s: %v", filename, err) } diff --git a/pkg/changelog/generator.go b/pkg/changelog/generator.go index 86adfb8..8aabac0 100644 --- a/pkg/changelog/generator.go +++ b/pkg/changelog/generator.go @@ -18,6 +18,8 @@ package changelog import ( "fmt" + "slices" + "strings" "k8c.io/gchl/pkg/types" @@ -82,15 +84,63 @@ func (g *Generator) groupChanges(changes []Change) []ChangeGroup { // List() sorts for us changeTypes := sets.List(sets.KeySet(tempMap)) + // change groups are supposed to be sorted alphabetically, except for some + // well known groups which we want to put in specific spots + // + // 1. New Features + // 2. API Changes + // 3. Deprecations + // ... + // N-2. Miscellaneous + // N-1. Chore + // N. Updates + // + // Everything between 3. and N-2 is sorted alphabetically. + changeTypes = sortSemantically(changeTypes) + result := []ChangeGroup{} for _, changeType := range changeTypes { changes := tempMap[changeType] result = append(result, ChangeGroup{ - Title: changeType.Title(), + Type: changeType, Changes: changes, }) } return result } + +func sortSemantically(changeTypes []ChangeType) []ChangeType { + slices.SortStableFunc(changeTypes, func(a, b ChangeType) int { + for _, check := range []ChangeType{ + ChangeTypeFeature, + ChangeTypeAPIChange, + ChangeTypeDeprecation, + } { + if a == check { + return -1 + } + if b == check { + return 1 + } + } + + for _, check := range []ChangeType{ + ChangeTypeUpdate, + ChangeTypeChore, + ChangeTypeMisc, + } { + if a == check { + return 1 + } + if b == check { + return -1 + } + } + + return strings.Compare(string(a), string(b)) + }) + + return changeTypes +} diff --git a/pkg/changelog/generator_test.go b/pkg/changelog/generator_test.go index ba97332..3c57eed 100644 --- a/pkg/changelog/generator_test.go +++ b/pkg/changelog/generator_test.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubermatic Kubernetes Platform contributors. +Copyright 2024 The Kubermatic Kubernetes Platform contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -26,36 +26,6 @@ import ( "gopkg.in/yaml.v3" ) -// func TestHumanReadableChangeType(t *testing.T) { -// testcases := []struct { -// identifier string -// expected string -// }{ -// { -// identifier: "breaking change", -// expected: "Breaking Change", -// }, -// { -// identifier: "api change", -// expected: "API Change", -// }, -// { -// identifier: "api-change", -// expected: "API Changes", -// }, -// } - -// for _, testcase := range testcases { -// t.Run(testcase.identifier, func(t *testing.T) { -// result := humanReadableChangeType(testcase.identifier) - -// if result != testcase.expected { -// t.Fatalf("Expected %q, got %q.", testcase.expected, result) -// } -// }) -// } -// } - type generateChangesTestcase struct { PR types.PullRequest `yaml:"pr"` Changes []Change `yaml:"changes"` diff --git a/pkg/changelog/parse.go b/pkg/changelog/parse.go index e9ee066..65ea7da 100644 --- a/pkg/changelog/parse.go +++ b/pkg/changelog/parse.go @@ -143,23 +143,22 @@ func splitIntoLines(text string) []string { } func itemToChange(explicitType ChangeType, breaking bool, text string) Change { - text = strings.TrimSuffix(text, ".") - text = inflect.Capitalize(text) - text = harmonizeLinePrefixes(text) - // item is breaking if it's in its text or the entire release-note block // is marked as breaking breaking = breaking || isBreakingChange(text) - if explicitType == "" { - switch { - case isBugfix(text): - explicitType = ChangeTypeBugfix - case isUpdate(text): - explicitType = ChangeTypeUpdate - default: - explicitType = ChangeTypeMisc - } + text = strings.TrimSuffix(text, ".") + text = removeActionRequired(text) + text = inflect.Capitalize(text) + text = harmonizeLinePrefixes(text) + + switch { + case isBugfix(text): + explicitType = ChangeTypeBugfix + case isUpdate(text): + explicitType = ChangeTypeUpdate + case explicitType == "": + explicitType = ChangeTypeMisc } return Change{ @@ -175,15 +174,27 @@ func isBreakingChange(text string) bool { } func isBugfix(releaseNote string) bool { - return strings.HasPrefix(releaseNote, "Fix ") + return strings.HasPrefix(strings.ToLower(releaseNote), "fix ") } -var isUpdateRegex = regexp.MustCompile(`updat(es|ed|ing|e) (.+)( version)? to (.+?)$`) +var isUpdateRegex = regexp.MustCompile(`^updat(es|ed|ing|e) `) func isUpdate(releaseNote string) bool { return isUpdateRegex.MatchString(strings.ToLower(releaseNote)) } +var actionRequiredRegex = regexp.MustCompile(`(?i)^[[*]*action required[\]*:]*`) + +// Remove redundant "ACTION REQUIRED" prefixes: breaking changes are already +// grouped into a dedicated section in the changelog, no need to prefix every +// item with the same yelling. +func removeActionRequired(text string) string { + text = actionRequiredRegex.ReplaceAllLiteralString(text, "") + text = strings.TrimSpace(text) + + return text +} + func harmonizeLinePrefixes(text string) string { replacements := map[string]string{ "Fixes": "Fix", @@ -212,6 +223,7 @@ func harmonizeLinePrefixes(text string) string { "Removes": "Remove", "Removed": "Remove", "Removing": "Remove", + "Resolved": "Resolve", "Deprecates": "Deprecate", "Deprecated": "Deprecate", "Deprecating": "Deprecate", diff --git a/pkg/changelog/parse_test.go b/pkg/changelog/parse_test.go new file mode 100644 index 0000000..13a72f2 --- /dev/null +++ b/pkg/changelog/parse_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2024 The Kubermatic Kubernetes Platform contributors. + +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 changelog + +import ( + "testing" +) + +func TestIsUpdate(t *testing.T) { + testcases := []struct { + text string + expected bool + }{ + { + text: "Update OSM to v1.5.2; fixing cloud-init bootstrapping issues on Ubuntu 22.04 on Azure", + expected: true, + }, + { + text: "updates operating-system-manager to v1.5.1.", + expected: true, + }, + { + text: "update Metering to v1.2.1.", + expected: true, + }, + { + text: "Updated to Go 1.22.2", + expected: true, + }, + { + text: "Update Cilium to 1.14.9 and 1.13.14", + expected: true, + }, + { + text: "updating to Kubernetes 1.29", + expected: true, + }, + { + text: "Update Vertical Pod Autoscaler to 1.0", + expected: true, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.text, func(t *testing.T) { + result := isUpdate(testcase.text) + + if result != testcase.expected { + t.Fatalf("Expected %v, got %v.", testcase.expected, result) + } + }) + } +} + +func TestRemoveActionRequired(t *testing.T) { + testcases := []struct { + input string + expected string + }{ + { + input: "Action required: if you use `velero.restic.deploy: true`...", + expected: "if you use `velero.restic.deploy: true`...", + }, + { + input: "**ACTION REQUIRED**: For velero helm chart upgrade. If running...", + expected: "For velero helm chart upgrade. If running...", + }, + { + input: "Action required: [User-mla] If you had copied `values.yaml...", + expected: "[User-mla] If you had copied `values.yaml...", + }, + { + input: "[ACTION REQUIRED] KubeLB: The prefix for the tenant namespaces created...", + expected: "KubeLB: The prefix for the tenant namespaces created...", + }, + { + input: "[Action Required] The field `ovdcNetwork` in `cluster` and `preset...", + expected: "The field `ovdcNetwork` in `cluster` and `preset...", + }, + } + + for _, testcase := range testcases { + t.Run(testcase.input, func(t *testing.T) { + result := removeActionRequired(testcase.input) + + if result != testcase.expected { + t.Fatalf("Expected %q, got %q.", testcase.expected, result) + } + }) + } +} diff --git a/pkg/changelog/testdata/multi-typed-release-note.yaml b/pkg/changelog/testdata/multi-typed-release-note.yaml new file mode 100644 index 0000000..6edbecd --- /dev/null +++ b/pkg/changelog/testdata/multi-typed-release-note.yaml @@ -0,0 +1,19 @@ +pr: + labels: + - kind/bug + + body: | + hello world + asdasd + + ```release-note + * Update operating-system-manager to v1.5.1. + * [ACTION REQUIRED] The latest Ubuntu 22.04 images ship with cloud-init 24.x package. This package has breaking changes and thus rendered our OSPs as incompatible. It's recommended to refresh your machines with latest provided OSPs to ensure that a system-wide package update, that updates cloud-init to 24.x, doesn't break the machines. + ``` + +changes: + - releaseNote: "The latest Ubuntu 22.04 images ship with cloud-init 24.x package. This package has breaking changes and thus rendered our OSPs as incompatible. It's recommended to refresh your machines with latest provided OSPs to ensure that a system-wide package update, that updates cloud-init to 24.x, doesn't break the machines" + breaking: true + type: bugfix + - releaseNote: "Update operating-system-manager to v1.5.1" + type: update diff --git a/pkg/changelog/types.go b/pkg/changelog/types.go index 2f880ed..448d4e9 100644 --- a/pkg/changelog/types.go +++ b/pkg/changelog/types.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubermatic Kubernetes Platform contributors. +Copyright 2024 The Kubermatic Kubernetes Platform contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,35 +20,46 @@ import ( "strings" "k8c.io/gchl/pkg/types" - - "github.com/go-openapi/inflect" ) type ChangeType string const ( - ChangeTypeAPIChange ChangeType = "api-change" - ChangeTypeUpdate ChangeType = "update" - ChangeTypeBugfix ChangeType = "bugfix" - ChangeTypeFeature ChangeType = "feature" - ChangeTypeMisc ChangeType = "misc" + ChangeTypeAPIChange ChangeType = "api-change" + ChangeTypeBugfix ChangeType = "bugfix" + ChangeTypeCleanup ChangeType = "cleanup" + ChangeTypeDeprecation ChangeType = "deprecation" + ChangeTypeDocumentation ChangeType = "documentation" + ChangeTypeFeature ChangeType = "feature" + ChangeTypeMisc ChangeType = "misc" + ChangeTypeChore ChangeType = "chore" + ChangeTypeRegression ChangeType = "regresssion" + ChangeTypeUpdate ChangeType = "update" ) var knownChangeTypes = map[string]ChangeType{ - "api-change": ChangeTypeAPIChange, "api change": ChangeTypeAPIChange, - "update": ChangeTypeUpdate, - "updates": ChangeTypeUpdate, - "fix": ChangeTypeBugfix, - "fixes": ChangeTypeBugfix, + "api-change": ChangeTypeAPIChange, + "bug": ChangeTypeBugfix, "bugfix": ChangeTypeBugfix, "bugfixes": ChangeTypeBugfix, - "bug": ChangeTypeBugfix, + "chore": ChangeTypeChore, + "cleanup": ChangeTypeCleanup, + "deprecates": ChangeTypeDeprecation, + "deprecation": ChangeTypeDeprecation, + "doc": ChangeTypeDocumentation, + "docs": ChangeTypeDocumentation, + "documentation": ChangeTypeDocumentation, "feature": ChangeTypeFeature, "features": ChangeTypeFeature, + "fix": ChangeTypeBugfix, + "fixes": ChangeTypeBugfix, "misc": ChangeTypeMisc, "miscellaneous": ChangeTypeMisc, "none": ChangeTypeMisc, + "regression": ChangeTypeRegression, + "update": ChangeTypeUpdate, + "updates": ChangeTypeUpdate, } func ParseChangeType(s string) ChangeType { @@ -60,18 +71,6 @@ func ParseChangeType(s string) ChangeType { return ChangeType(s) } -func (t ChangeType) Title() string { - if t == ChangeTypeMisc { - t = "Miscellaneous" - } - - title := strings.ReplaceAll(string(t), "-", " ") - title = inflect.Titleize(title) - title = strings.ReplaceAll(title, "Api", "API") - - return title -} - type Changelog struct { Version string `yaml:"version" json:"version"` RepositoryURL string `yaml:"repository" json:"repository"` @@ -79,8 +78,8 @@ type Changelog struct { } type ChangeGroup struct { - Title string `yaml:"title" json:"title"` - Changes []Change `yaml:"changes" json:"changes"` + Type ChangeType `yaml:"type" json:"type"` + Changes []Change `yaml:"changes" json:"changes"` } type Change struct { @@ -90,3 +89,17 @@ type Change struct { Breaking bool `yaml:"breaking,omitempty"` Text string `yaml:"releaseNote"` } + +func (c *Changelog) BreakingChanges() []Change { + var breaks []Change + + for _, group := range c.ChangeGroups { + for i, change := range group.Changes { + if change.Breaking { + breaks = append(breaks, group.Changes[i]) + } + } + } + + return breaks +} diff --git a/pkg/ranges/ranger.go b/pkg/ranges/ranger.go index f8c9130..b6ab599 100644 --- a/pkg/ranges/ranger.go +++ b/pkg/ranges/ranger.go @@ -229,8 +229,8 @@ func findPreviousReleaseBranch(currentVersion *semver.Version, allRepoRefs types return fmt.Sprintf("release/v%d.%d", prevMajor, prevMinor), nil } -func toLookupTable(commits []types.Commit) sets.String { - result := sets.NewString() +func toLookupTable(commits []types.Commit) sets.Set[string] { + result := sets.New[string]() for _, commit := range commits { result.Insert(commit.Hash) } @@ -242,8 +242,8 @@ func toLookupTable(commits []types.Commit) sets.String { // so that the lookup only contains stable versions. This is because later when we use it, // we do not want to stop at alpha versions, but only on stable versions. The same goes for // the start version (otherwise we would stop immediately on the first commit). -func toTagLookupTable(tags []types.Ref, targetVersion *semver.Version) sets.String { - result := sets.NewString() +func toTagLookupTable(tags []types.Ref, targetVersion *semver.Version) sets.Set[string] { + result := sets.New[string]() for _, tag := range tags { sv, err := semver.NewVersion(tag.Name) if err == nil && sv.Prerelease() == "" && !targetVersion.Equal(sv) { diff --git a/pkg/render/json.go b/pkg/render/json.go index 9dca261..9bf55f9 100644 --- a/pkg/render/json.go +++ b/pkg/render/json.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2024 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. diff --git a/pkg/render/markdown.go b/pkg/render/markdown.go index 3058e81..5692978 100644 --- a/pkg/render/markdown.go +++ b/pkg/render/markdown.go @@ -23,6 +23,8 @@ import ( "text/template" "k8c.io/gchl/pkg/changelog" + + "github.com/go-openapi/inflect" ) type markdown struct{} @@ -35,19 +37,53 @@ var markdownTemplate = ` ## {{ .Version }} **GitHub release: [{{ .Version }}]({{ .RepositoryURL }}/releases/tag/{{ .Version }})** +{{- $breaking := .BreakingChanges }} +{{- if $breaking }} + +### Breaking Changes + +This release contains changes that require additional attention, please read the following items carefully. +{{ range $breaking }} +- {{ .Text }} ([#{{ .Commit.PullRequest.Number }}]({{ prlink .Commit.PullRequest.Number }})) +{{- end }} +{{- end }} {{ range .ChangeGroups }} -### {{ .Title }} +### {{ typename .Type }} {{ range .Changes }} -- {{ .ReleaseNote }} ([#{{ .Commit.PullRequest.Number }}]({{ prlink .Commit.PullRequest.Number }})) +- {{ .Text }} ([#{{ .Commit.PullRequest.Number }}]({{ prlink .Commit.PullRequest.Number }})) {{- end }} {{ end }} ` +var overriddenTypeNames = map[changelog.ChangeType]string{ + changelog.ChangeTypeAPIChange: "API Changes", + changelog.ChangeTypeBugfix: "Bugfixes", + changelog.ChangeTypeCleanup: "Cleanups", + changelog.ChangeTypeDeprecation: "Deprecations", + changelog.ChangeTypeDocumentation: "Documentation", + changelog.ChangeTypeFeature: "New Features", + changelog.ChangeTypeMisc: "Miscellaneous", + changelog.ChangeTypeChore: "Chores", + changelog.ChangeTypeRegression: "Regresssions", + changelog.ChangeTypeUpdate: "Updates", +} + func (m *markdown) Render(log *changelog.Changelog) (string, error) { t := template.New("changelog").Funcs(template.FuncMap{ "prlink": func(number int) string { return fmt.Sprintf("%s/pull/%d", log.RepositoryURL, number) }, + "typename": func(changeType changelog.ChangeType) string { + if known, ok := overriddenTypeNames[changeType]; ok { + return known + } + + title := strings.ReplaceAll(string(changeType), "-", " ") + title = inflect.Titleize(title) + title = strings.ReplaceAll(title, "Api", "API") + + return title + }, }) var err error