Introducing changelog generation tool #264

doggydogworld · 2024-07-01T20:15:42Z

This is a rewrite of the changelog.sh script to Go. This should make it easier to maintain and to align with the companies preference to write tooling in Go. This PR also includes the following:

Improvements to handling of Enterprise changelogs
- No longer prints Enterprise: if there are no Enterprise changelogs.
- No longer generates a summary if changelog is not present in PR body.
- No longer includes links to pull requests since those will be private.

Some notes for implementation:

Introduced a small gh package to wrap the calls to the gh CLI. When looking to implement using the GitHub Web APIs directly I noticed that it would be much simpler with GitHub v4 API than with v3 and since that didn't really fit with our existing v3 library and would over-complicate the PR I figured better to just keep the simple implementation for now.
Introduced a git package for some encapsulation around git

bot/internal/gh/gh.go

bot/tools/changelog/README.md

bot/go.mod

bot/tools/changelog/changelog.go

fheinecke · 2024-07-01T22:05:45Z

bot/tools/changelog/main.go

+func getLastVersion(baseTag, dir string) (string, error) {
+	if baseTag != "" {
+		return baseTag, nil
+	}
+
+	// get root dir of repo
+	topDir, err := git.RunCmd(dir, "rev-parse", "--show-toplevel")
+	if err != nil {
+		return "", trace.Wrap(err)
+	}
+	lastVersion, err := makePrintVersion(topDir)
+	if err != nil {
+		return "", trace.Wrap(err)
+	}
+
+	return lastVersion, nil
+}
+
+// makePrintVersion will run 'make -s print-version'
+func makePrintVersion(dir string) (string, error) {
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+
+	cmd := exec.Command("make", "-s", "print-version")
+	cmd.Dir = dir
+	cmd.Stderr = &stderr
+	cmd.Stdout = &stdout
+
+	err := cmd.Run()
+
+	if err != nil {
+		return strings.TrimSpace(stderr.String()), trace.Wrap(err, "can't get last released version")
+	}
+	out := strings.TrimSpace(stdout.String())
+
+	return "v" + out, nil
+}


IMO it would be better to pass this in as a parameter rather than invoking make. This would allow the tool to be used in other projects without this target.

So this is actually passed in as a flag (--base-tag). This is just the fallback if that flag isn't passed in. I tried to keep the functionality close to the current implementation. But yeah this would fail if the --base-tag flag isn't set and the Makefile doesn't exist or have that target making it not so portable.

For now to be clearer e.g. --base-tag was not set, defaulted to invoking 'make version' but failed. Should be less ambiguous and indicate a way to "fix" it.

doggydogworld · 2024-07-02T04:22:56Z

go.mod

@fheinecke I added the go.mod at the top of the directory to get my code in a working state. I wasn't quite sure what the intention was for the Go module. I did see in the file structure that there were some go.mod files in nested directories. To me that seemed kind of odd because I'd imagine that most of the code would be versioned together (or at least libs and tools). With go.mod setup in different directories we would have to use Go workspaces to manage the dependency between modules.

Oh also the go.sum is currently messed up a bit I'll have to fix it.

I did see in the file structure that there were some go.mod files in nested directories.

I typo'd this in the original PR for that RFD. Each project should have their own go.mod/sum files, though they shouldn't be under the workflows directory. Generally we don't want all the code in this repo versioned together. It's intended to be used as a collection of unrelated projects, so we don't want that direct coupling. This includes libraries, as we would like to avoid needing to update all dependents immediately if a library interface changes.

r0mant · 2024-07-02T17:10:38Z

libs/gh/pull_request.go

+	var prs []ChangelogPR
+	query := fmt.Sprintf("base:%s merged:%s -label:no-changelog", opts.Branch, dateRangeFormat(opts.FromDate, opts.ToDate))
+
+	data, err := RunCmd(dir, "pr", "list", "--search", query, "--limit", "200", "--json", "number,url,title,body")


Since we're moving this to shared-workflows, we should use github package we use in other tools here rather than calling CLI: https://github.com/gravitational/shared-workflows/blob/main/bot/internal/github/github.go

Something to mention is that packages cannot import internal unless they're nested in the same parent directory as internal. I'd imagine that everything under the internal package would be moved up under libs.

Yea it'd need to move to a new module under /libs. Probably just the github package for now though, to avoid too much scope creep.

libs/gh/gh.go

tools/changelog/changelog.go

tools/changelog/main.go

r0mant · 2024-07-02T17:17:09Z

tools/changelog/main.go

+
+// getBranch will return branch if parsed otherwise will attempt to find it
+// Branch should be in the format "branch/v*"
+func getBranch(branch, dir string) (string, error) {


Can these things (get branch, get forked branch, etc.) be moved into a shared reusable "git" library package, similar to what we have for "github"?

At that point it might just be easier to use https://github.com/go-git/go-git and remove the CLI tool dep entirely at the same time. I've used that library on a handful of projects before and it works well

Yep, agreed.

Yup I can do that should be a quick change.

fwiw - Be sure to check the go-git compatibility guide. I had to go with the git lib route for a different project due to a lack of support for certain commands.

doggydogworld · 2024-07-05T21:57:49Z

@r0mant @fheinecke @camscale

Updated based on feedback:

New libs/git module added
- Wraps go-git library and provides method for shelling out to a single git repository (directory).
New libs/github module added
- Includes a new function that uses the v4 GitHub API to search for commits containing changelogs.
- Temporary github.go file included to prepare for a future refactor of the bots internal library.
Updated tools/changelog
- Now requires a path to the local Teleport repository as an argument. This is to remove code that probably wasn't necessary.
- Uses new libs/git and libs/github modules

tools/changelog/changelog.go

libs/git/go.mod

r0mant · 2024-07-05T23:14:37Z

libs/github/go.mod

+	github.com/cli/go-gh/v2 v2.9.0
+	github.com/google/go-github/v37 v37.0.0
+	github.com/gravitational/trace v1.3.1
+	github.com/shurcooL/githubv4 v0.0.0-20240429030203-be2daab69064


Why do we need 3 different libraries for Github? Can we simplify? In our existing bot we already use github.com/google/go-github.

I was using the v4 API for the code but it isn't absolutely necessay so just moved it real quick to use the v3 API. The go-gh library has a useful function I just stumbled upon that will use look for GITHUB_TOKEN, github config file, or gh auth. Useful for running it on developer machines and CI.

I could remove that dependency. I need to update the config to note that GITHUB_TOKEN is needed anyway. Could have it maybe be a requirement to pass in the token.

libs/git/git.go

r0mant

@doggydogworld We've been going back-and-forth on this for quite a bit now, I think we should be wrapping this up as it was supposed to be a just quick port from bash to Go. From my side I'd like us to address 3 things before it's good to go:

Remove go.mod/sum from every package, this is very unnecessary.
Let's go back to just calling git CLI and remove dependency on git library.
Restore PR links in enterprise changes list (I left a comment about this before but it's still unaddressed).

Can you please make these changes and then let's get this ready to merge and start using.

r0mant

Last comment about go.mod, then lgtm.

Also, please don't forget to update make changelog target in teleport repo to use this tool as a follow up.

r0mant · 2024-07-10T16:29:48Z

go.mod

@@ -0,0 +1,27 @@
+module github.com/gravitational/shared-workflows


I don't think this is correct still. We should have a go.mod per tool I feel, not a single one for the entire repo or one for each package.

Oh I think I have a better idea of what is wanted here then. I'm thinking then it should be a module for libs as a whole and a module for each tool.

For example in this change two modules will be created.

github.com/gravitational/shared-workflows/libs github.com/gravitational/shared-workflows/tools/changelog

Tools are versioned separately. Everything under libs will be versioned together.

Btw I implemented the above. Check it out and let me know if it seems right.

r0mant · 2024-07-10T16:30:56Z

go.mod

+	github.com/alecthomas/kingpin/v2 v2.4.0
+	github.com/cli/go-gh/v2 v2.9.0
+	github.com/google/go-github/v37 v37.0.0
+	github.com/gravitational/trace v1.4.0
+	github.com/stretchr/testify v1.9.0
+	golang.org/x/oauth2 v0.21.0


This looks much better as dependency list 👍

tools/changelog/changelog.go

doggydogworld · 2024-07-11T20:07:04Z

Last comment about go.mod, then lgtm.

Also, please don't forget to update make changelog target in teleport repo to use this tool as a follow up.

FYI created a PR for this just to be ready here

fheinecke · 2024-07-15T19:00:40Z

@r0mant I disagree with using a single Go module under libs/. Using a single module adds complexity, rather than reducing it, due to the additional coupling between libraries. This makes it more difficult for tools that depend on this "library module" to update, and removes our ability to defer updates in some cases.

Example 1:

We currently have separate "git" and "github" libraries inside the library module. This changelog generation tool depends on both of these libraries (regardless whether they're one or two modules). It is reasonable to expect that at some point we will introduce at least one breaking change to one of these libraries, for example, changing the signature of a function in the github library. It is also reasonable to expect that we may eventually discover a security issue with one of the libraries, for example, a CLI injection vulnerability in the git library.

When a breaking change is introduced, we may to avoid updating the changelog generation tool for awhile if the breaking change ends up being difficult to handle without providing a lot of value for this tool. If we defer this update, but we later discover a security issue with one of the other libraries, then we will be forced to take on both the security fix work and breaking change migration work at the same time, for all tools affected by both issues. If we use a separate module for each separate library, then we can avoid this extra work.

Example 2:

Say we want to add a new feature to this tool, such as pushing these release notes to S3. We add a new library to wrap S3 API calls, which may be needed for things like handling pagination, retries, etc. If there is a breaking change in elsewhere in the library module, then migrating to accommodate this breaking change will now be required to add the new feature. This adds extra work where there otherwise wouldn't be any.

Example 3 (experienced with Teleport dependencies):

Right now the library module is relatively simple with few dependencies, keeping it small. However, if the module adds a significant number of dependencies (see the above discussion about go-git), and it adds one of the Go reflection footguns (directly or indirectly via a dependency), then the binary size for every tool will explode. Using separate modules negates this issue as only dependencies (and dependencies of dependencies, and so on) that are needed by a given tool will be included.

At absolute bare minimum, this Go library module needs to move to a subdirectory of libs. The libs directory is not exclusively intended for Go code, and we should keep Go code/toolchain files out of it.

Introducing changelog

97a64c0

doggydogworld marked this pull request as ready for review July 1, 2024 20:24

doggydogworld requested a review from a team as a code owner July 1, 2024 20:24

doggydogworld requested review from r0mant, fheinecke and camscale July 1, 2024 20:25

fheinecke reviewed Jul 1, 2024

View reviewed changes

doggydogworld added 2 commits July 1, 2024 17:54

Moving things around

47c7d92

Fixing typo

7d016de

doggydogworld commented Jul 2, 2024

View reviewed changes

Changing licenses on new files

d8046e7

r0mant reviewed Jul 2, 2024

View reviewed changes

doggydogworld added 12 commits July 3, 2024 12:32

Moving to multi-module

2bfafc1

Moved to github SDK

62d5e67

Removing some unecessary code

eb0adee

Requiring dir to be passed in to simplify code

9f091be

Initial refactor to go-git lib

cc60d0d

Removing more git execs for native go git

d604039

Fixing some typos

de6e47e

More refactors for git module

6d34444

Moving another function to git module

7ce3d02

Fixing typo

3d7f69d

Updating an error string

6b00945

Restoring bot module deps

7ffa993

doggydogworld added 2 commits July 5, 2024 17:41

Moving around a file

d3a4a52

Updating docs for git module

f7c537f

r0mant reviewed Jul 5, 2024

View reviewed changes

Using v3 API for search

9acb082

doggydogworld force-pushed the gus/changelog-gen branch from 355a413 to 9acb082 Compare July 6, 2024 00:34

r0mant reviewed Jul 9, 2024

View reviewed changes

doggydogworld added 3 commits July 9, 2024 17:48

Cleaning up multi-module and adding links back to ent cl

6e67e79

Using git CLI

d48641f

Even prettier changelog

207e590

r0mant reviewed Jul 10, 2024

View reviewed changes

doggydogworld added 3 commits July 10, 2024 13:24

Go moduling changelog and libs

a6692fb

Using cl for template name

c94b5c0

Fixing changelog mod

f4a6950

r0mant approved these changes Jul 11, 2024

View reviewed changes

doggydogworld requested review from fheinecke and jimbishopp July 12, 2024 18:29

Updating govulncheck to use latest go version

62ba564

doggydogworld added 2 commits July 15, 2024 14:01

Using more secure version of go-github

2c75aae

New dependencies

86a40ae

doggydogworld merged commit 2712d87 into main Jul 17, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing changelog generation tool #264

Introducing changelog generation tool #264

doggydogworld commented Jul 1, 2024 •

edited

Loading

fheinecke Jul 1, 2024

doggydogworld Jul 2, 2024

doggydogworld Jul 2, 2024

fheinecke Jul 2, 2024

r0mant Jul 2, 2024

doggydogworld Jul 2, 2024

fheinecke Jul 2, 2024 •

edited

Loading

r0mant Jul 2, 2024

fheinecke Jul 2, 2024

r0mant Jul 2, 2024

doggydogworld Jul 2, 2024

jimbishopp Jul 2, 2024

doggydogworld commented Jul 5, 2024 •

edited

Loading

r0mant Jul 5, 2024

doggydogworld Jul 6, 2024

r0mant left a comment

r0mant left a comment

r0mant Jul 10, 2024

doggydogworld Jul 10, 2024

doggydogworld Jul 11, 2024

r0mant Jul 10, 2024

doggydogworld commented Jul 11, 2024

fheinecke commented Jul 15, 2024

		@@ -0,0 +1,27 @@
		module github.com/gravitational/shared-workflows

Introducing changelog generation tool #264

Introducing changelog generation tool #264

Conversation

doggydogworld commented Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fheinecke Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doggydogworld commented Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r0mant left a comment

Choose a reason for hiding this comment

r0mant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doggydogworld commented Jul 11, 2024

fheinecke commented Jul 15, 2024

Example 1:

Example 2:

Example 3 (experienced with Teleport dependencies):

doggydogworld commented Jul 1, 2024 •

edited

Loading

fheinecke Jul 2, 2024 •

edited

Loading

doggydogworld commented Jul 5, 2024 •

edited

Loading