Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ops/gmpctl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ SCRIPT_DIR="$(
pwd -P
)"

# Install dependencies in the current context. We switch Go toolchain versions so using go tool might not work.
go install github.com/google/go-containerregistry/cmd/gcrane@latest
go install github.com/mikefarah/yq/v4@latest
go install helm.sh/helm/v3/cmd/helm@latest
go install github.com/google/addlicense@latest

export PATH="$(go env GOPATH)/bin:${PATH}"
Comment on lines +29 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Running go install on every execution of gmpctl.sh is highly inefficient and can significantly slow down the script, especially in offline environments or when dependencies are already installed. Consider checking if the binaries are already available in the PATH or GOPATH/bin before attempting to install them.

Suggested change
# Install dependencies in the current context. We switch Go toolchain versions so using go tool might not work.
go install github.com/google/go-containerregistry/cmd/gcrane@latest
go install github.com/mikefarah/yq/v4@latest
go install helm.sh/helm/v3/cmd/helm@latest
go install github.com/google/addlicense@latest
export PATH="$(go env GOPATH)/bin:${PATH}"
export PATH="$(go env GOPATH)/bin:${PATH}"
for cmd in gcrane yq helm addlicense; do
if ! command -v "$cmd" &>/dev/null; then
case "$cmd" in
gcrane) go install github.com/google/go-containerregistry/cmd/gcrane@latest ;;
yq) go install github.com/mikefarah/yq/v4@latest ;;
helm) go install helm.sh/helm/v3/cmd/helm@latest ;;
addlicense) go install github.com/google/addlicense@latest ;;
esac
fi
done


# NOTE gmpctl expects the gmpctl directory to be present on execution for
# local bash scripts and configuration.
#
Expand Down
4 changes: 3 additions & 1 deletion ops/gmpctl/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ key information and confirmations e.g.
same parameters, and it will continue the previous work or at least yield same results. This is crucial when iterating
on breaking go mod updates for vulnerabilities or fork sync conflicts.

```text mdox-exec="bash ops/gmpctl.sh --help"
```text mdox-exec="bash -c \"bash ops/gmpctl.sh --help 2>&1 | sed -n '/Usage/,$p'\""
Usage: gmpctl [COMMAND] [FLAGS]
-c string
Path to the configuration file. See config.go#Config for the structure. (default ".gmpctl.default.yaml")
Expand All @@ -67,6 +67,8 @@ Usage: gmpctl [COMMAND] [FLAGS]
[vulnfix] Usage of vulnfix:
-b string
Release branch to work on; Project is auto-detected from this
-go-version string
Go minor version to use for docker images.
-pr-branch string
(default: $USER/BRANCH-vulnfix) Upstream branch to push to (user-confirmed first).
-sync-dockerfiles-from
Expand Down
20 changes: 10 additions & 10 deletions ops/gmpctl/cmd_release.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,22 @@ func release() error {
return err
}

mustCreateOrRecreateTag(dir, tag)
if !mustIsRemoteUpToDate(dir, branch) {
if confirmf("About to git push state from %q to \"origin/%v\" for %q tag; are you sure?", dir, branch, tag) {
// We are in detached state, so use the HEAD reference.
mustPush(dir, fmt.Sprintf("HEAD:%v", branch))
if confirmf("About to create a signed tag %q and git push state (HEAD:%v) and tag %q from %q to \"origin\"; are you sure?", tag, branch, tag, dir) {
mustPush(dir, fmt.Sprintf("HEAD:%v", branch), tag)
} else {
return errors.New("aborting")
}
}

// TODO(bwplotka): Check if tag exists.
mustCreateSignedTag(dir, tag)
if confirmf("About to git push %q tag from %q to \"origin/%v\"; are you sure?", tag, dir, branch) {
mustPush(dir, tag)
} else {
return errors.New("aborting")
// Retagging only. This can happen if someone wants to continue the script.
if confirmf("About to create a signed tag %q and push it from %q to \"origin\"; are you sure?", tag, dir) {
mustPush(dir, tag)
} else {
return errors.New("aborting")
}
}
Comment on lines +86 to 100

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Calling mustCreateOrRecreateTag before the confirmation prompt creates or recreates the signed tag locally (which may prompt the user for their GPG passphrase) even if they ultimately choose to abort the release. Additionally, the confirmation prompt says "About to create a signed tag..." when the tag has already been created. Restructuring the flow to prompt the user first avoids these side-effects and misleading messages.

Suggested change
mustCreateOrRecreateTag(dir, tag)
if !mustIsRemoteUpToDate(dir, branch) {
if confirmf("About to git push state from %q to \"origin/%v\" for %q tag; are you sure?", dir, branch, tag) {
// We are in detached state, so use the HEAD reference.
mustPush(dir, fmt.Sprintf("HEAD:%v", branch))
if confirmf("About to create a signed tag %q and git push state (HEAD:%v) and tag %q from %q to \"origin\"; are you sure?", tag, branch, tag, dir) {
mustPush(dir, fmt.Sprintf("HEAD:%v", branch), tag)
} else {
return errors.New("aborting")
}
}
// TODO(bwplotka): Check if tag exists.
mustCreateSignedTag(dir, tag)
if confirmf("About to git push %q tag from %q to \"origin/%v\"; are you sure?", tag, dir, branch) {
mustPush(dir, tag)
} else {
return errors.New("aborting")
// Retagging only. This can happen if someone wants to continue the script.
if confirmf("About to create a signed tag %q and push it from %q to \"origin\"; are you sure?", tag, dir) {
mustPush(dir, tag)
} else {
return errors.New("aborting")
}
}
if !mustIsRemoteUpToDate(dir, branch) {
if confirmf("About to create a signed tag %q and git push state (HEAD:%v) and tag %q from %q to \"origin\"; are you sure?", tag, branch, tag, dir) {
mustCreateOrRecreateTag(dir, tag)
mustPush(dir, fmt.Sprintf("HEAD:%v", branch), tag)
} else {
return errors.New("aborting")
}
} else {
// Retagging only. This can happen if someone wants to continue the script.
if confirmf("About to create a signed tag %q and push it from %q to \"origin\"; are you sure?", tag, dir) {
mustCreateOrRecreateTag(dir, tag)
mustPush(dir, tag)
} else {
return errors.New("aborting")
}
}


if confirmf("Do you want to remove the %v worktree (recommended)?", dir) {
proj.RemoveWorkDir(cfg.Directory, dir)
}
Expand Down
176 changes: 175 additions & 1 deletion ops/gmpctl/cmd_vulnfix.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ import (
"flag"
"fmt"
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
)

var (
vulnfixFlags = flag.NewFlagSet("vulnfix", flag.ExitOnError)
vulnfixBranch = vulnfixFlags.String("b", "", "Release branch to work on; Project is auto-detected from this")
vulnfixPRBranch = vulnfixFlags.String("pr-branch", "", "(default: $USER/BRANCH-vulnfix) Upstream branch to push to (user-confirmed first).")
vulnfixSyncDockerfilesFrom = vulnfixFlags.Bool("sync-dockerfiles-from", false, "Optional branch name to sync Dockerfiles from. Useful when things changed.")
vulnfixGoVersion = vulnfixFlags.String("go-version", "", "Go minor version to use for docker images.")
)

// Attempt a minimal dependency upgrade to solve fixable vulnerabilities.
Expand Down Expand Up @@ -74,10 +79,22 @@ func vulnfix() error {
// Refresh.
mustFetchAll(dir)

goVersion := *vulnfixGoVersion
if goVersion == "" {
goVersion, err = detectGoMinorVersion(dir)
if err != nil {
return fmt.Errorf("could not detect Go version from Dockerfile: %v", err)
}
}
logf("Using Go version: %s", goVersion)

opts := []string{
fmt.Sprintf("DIR=%v", dir),
fmt.Sprintf("BRANCH=%v", branch),
fmt.Sprintf("PROJECT=%v", proj.Name),
fmt.Sprintf("GO_VERSION=%v", goVersion),
// We use the local toolchain to avoid downloading remote toolchains during vulnfix.
fmt.Sprintf("GOTOOLCHAIN=local"),
}
if *vulnfixSyncDockerfilesFrom {
opts = append(opts, "SYNC_DOCKERFILES_FROM=true")
Expand All @@ -88,7 +105,13 @@ func vulnfix() error {
return err
}

// TODO: Warn of unstaged files at this point.
if proj.Name != "prometheus-engine" {
if err := fixOtelSchemaConflict(dir); err != nil {
return err
}
}

// TODO: Warn of any unstaged files at this point.

// Commit if anything is staged.
msg := fmt.Sprintf("google patch[deps]: fix %v vulnerabilities", branch)
Expand All @@ -110,6 +133,7 @@ func vulnfix() error {
// We are in detached state, so be explicit what to push and from where, by recreating the local prBranch.
mustRecreateBranch(dir, prBranch)
mustForcePush(dir, prBranch)
mustEnsurePullRequest(dir, branch, prBranch, msg, "Updating Go and image vulnerabilities using"+wrapCode("./gmpctl.sh vulnfix"))
} else {
return errors.New("aborting")
}
Expand All @@ -119,3 +143,153 @@ func vulnfix() error {
}
return nil
}

func detectGoMinorVersion(dir string) (string, error) {
var dockerfiles []string
err := filepath.WalkDir(dir, func(path string, info os.DirEntry, err error) error {
if err != nil {
return err
}
if info.IsDir() {
name := info.Name()
if name == "third_party" || name == "ui" || name == "vendor" || name == "node_modules" || name == ".git" {
return filepath.SkipDir
}
return nil
}
if strings.HasPrefix(info.Name(), "Dockerfile") {
dockerfiles = append(dockerfiles, path)
}
return nil
})
if err != nil {
return "", err
}
if len(dockerfiles) == 0 {
return "", fmt.Errorf("no Dockerfile found in %s", dir)
}

re := regexp.MustCompile(`(?:google-go\.pkg\.dev/golang|golang):([0-9]+\.[0-9]+)`)

for _, df := range dockerfiles {
content, err := os.ReadFile(df)
if err != nil {
continue
}
matches := re.FindSubmatch(content)
if len(matches) > 1 {
return string(matches[1]), nil
}
}
return "", fmt.Errorf("could not find golang image in any Dockerfile under %s", dir)
}

func wrapCode(s string) string {
return "\n```\n" + s + "\n```\n"
}

// It's a common occurrence that schema import goes off-sync with the go module, fix it.
func fixOtelSchemaConflict(dir string) error {
targetVersion, err := detectSchemaVersion(dir)
if err != nil {
return err
}
if targetVersion == "" {
return nil
}
return replaceOtelImports(dir, targetVersion)
}

// TODO(bwplotka): AI figured some way, but there's likely a better way to tell?
func detectSchemaVersion(dir string) (string, error) {
tmpFile := filepath.Join(dir, "gmpctl_tmp_schema.go")
tmpCode := `package main

import (
"fmt"
"go.opentelemetry.io/otel/sdk/resource"
)

func main() {
r := resource.Default()
fmt.Print(r.SchemaURL())
}
`
if err := os.WriteFile(tmpFile, []byte(tmpCode), 0o644); err != nil {
return "", fmt.Errorf("failed to write temp file: %w", err)
}
defer os.Remove(tmpFile)
Comment on lines +205 to +221

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a hardcoded temporary filename gmpctl_tmp_schema.go inside the project directory can lead to accidental file overwrites or deletion if a file with that name already exists. Using os.CreateTemp with a pattern ensures a unique filename is used.

	tmpFile, err := os.CreateTemp(dir, "gmpctl_tmp_schema_*.go")
	if err != nil {
		return "", fmt.Errorf("failed to create temp file: %w", err)
	}
	tmpPath := tmpFile.Name()
	defer os.Remove(tmpPath)
	defer tmpFile.Close()

	tmpCode := `package main

import (
	"fmt"
	"go.opentelemetry.io/otel/sdk/resource"
)

func main() {
	r := resource.Default()
	fmt.Print(r.SchemaURL())
}
`
	if _, err := tmpFile.Write([]byte(tmpCode)); err != nil {
		return "", fmt.Errorf("failed to write temp file: %w", err)
	}
	_ = tmpFile.Close()


cmd := exec.Command("go", "run", "gmpctl_tmp_schema.go")
cmd.Dir = dir
cmd.Stderr = os.Stderr
out, err := cmd.Output()
Comment on lines +223 to +226

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting cmd.Stderr = os.Stderr will print noisy compilation errors directly to the console if the project does not have OpenTelemetry dependencies or fails to compile. Since the error is explicitly caught and logged as a warning, it is better to let cmd.Output() capture the stderr automatically (by leaving cmd.Stderr as nil) and only log it if an error occurs.

Suggested change
cmd := exec.Command("go", "run", "gmpctl_tmp_schema.go")
cmd.Dir = dir
cmd.Stderr = os.Stderr
out, err := cmd.Output()
cmd := exec.Command("go", "run", filepath.Base(tmpPath))
cmd.Dir = dir
out, err := cmd.Output()

if err != nil {
// If it fails to run, it might be because otel/sdk is not in dependencies,
// or some other issue. We log and ignore to not block the whole pipeline if it's not relevant.
logf("Warning: failed to run temp schema detector: %v", err)
return "", nil
}

schemaURL := string(out)
if schemaURL == "" {
logf("No schema URL detected from SDK resource")
return "", nil
}

reVersion := regexp.MustCompile(`([0-9]+\.[0-9]+\.[0-9]+)$`)
matches := reVersion.FindStringSubmatch(schemaURL)
if len(matches) < 2 {
logf("Could not parse version from schema URL: %s", schemaURL)
return "", nil
}
return "v" + matches[1], nil
}

func replaceOtelImports(dir string, targetVersion string) error {
logf("Detected target OpenTelemetry schema version: %s", targetVersion)

reImport := regexp.MustCompile(`"go\.opentelemetry\.io/otel/semconv/(v1\.[0-9]+\.[0-9]+)"`)
reSchemaURLUse := regexp.MustCompile(`\.SchemaURL\b`)

if err := filepath.WalkDir(dir, func(path string, info os.DirEntry, err error) error {
if err != nil {
return err
}
if info.IsDir() {
name := info.Name()
if name == "vendor" || name == "third_party" || name == ".git" {
return filepath.SkipDir
}
return nil
}
if !strings.HasSuffix(info.Name(), ".go") {
return nil
}

content, err := os.ReadFile(path)
if err != nil {
return err
}

if !reImport.Match(content) || !reSchemaURLUse.Match(content) {
return nil
}

newContent := reImport.ReplaceAllFunc(content, func(match []byte) []byte {
return []byte(fmt.Sprintf(`"go.opentelemetry.io/otel/semconv/%s"`, targetVersion))
})

if string(newContent) != string(content) {
logf("Updating OTEL semconv imports to %s in %s", targetVersion, path)
if err := os.WriteFile(path, newContent, 0o644); err != nil {
return fmt.Errorf("failed to write file %s: %w", path, err)
}
}
return nil
}); err != nil {
return err
}
mustAddAll(dir)
return nil
}
68 changes: 61 additions & 7 deletions ops/gmpctl/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,68 @@ func mustFetchAll(dir string) {
}
}

func getTTY(dir string) string {
out, err := runCommand(&cmdOpts{Dir: dir, HideOutputs: true}, "tty")
if err != nil {
return ""
}
return strings.TrimSpace(out)
}

func mustCreateSignedTag(dir, tag string) {
logf("Creating a signed tag %v...", tag)

// explicit TTY is often needed on Macs.
// Ensure TTY is set for GPG signing.
var envs []string
if tty := getTTY(dir); tty != "" {
envs = append(envs, "GPG_TTY="+tty)
}

// TODO(bwplotka): Consider adding v0.x second tag for Prometheus fork (similar to how v0.300 Prometheus releases are structured).
// This is to have a little bit cleaner prometheus-engine go.mod version against the fork.
if _, err := runCommand(
&cmdOpts{Dir: dir},
"bash", "-c",
fmt.Sprintf("GPG_TTY=$(tty) git tag -s %v -m %v", tag, tag),
&cmdOpts{Dir: dir, Envs: envs},
"git", "tag", "-s", tag, "-m", tag,
); err != nil {
panicf(err.Error())
}
}

func mustCreateOrRecreateTag(dir, tag string) {
// Check if the tag exists on origin, and if so, finish
_, errLs := runCommand(&cmdOpts{Dir: dir, HideOutputs: true}, "git", "ls-remote", "--exit-code", "--tags", "origin", "refs/tags/"+tag)
if errLs == nil {
panicf("This tag %v was already pushed, chose a different one!", tag)
}

// Check if the tag already exists locally.
tagCommit, err := runCommand(&cmdOpts{Dir: dir, HideOutputs: true}, "git", "rev-parse", "--verify", "-q", "refs/tags/"+tag+"^{commit}")
if err == nil {
// Tag exists locally!
headCommit, err := runCommand(&cmdOpts{Dir: dir, HideOutputs: true}, "git", "rev-parse", "HEAD")
if err != nil {
panicf("failed to get HEAD commit: %v", err)
}

tagCommit = strings.TrimSpace(tagCommit)
headCommit = strings.TrimSpace(headCommit)

if tagCommit == headCommit {
logf("Tag %q already exists locally and points to HEAD (%s). Skipping recreation.", tag, headCommit)
return
}
logf("Tag %q exists locally but points to commit %s, while HEAD is at %s. Re-tagging...", tag, tagCommit, headCommit)

// Delete the local tag.
if _, err := runCommand(&cmdOpts{Dir: dir}, "git", "tag", "-d", tag); err != nil {
panicf("failed to delete local tag %s: %v", tag, err)
}
}

// Create the signed tag.
mustCreateSignedTag(dir, tag)
}

// mustIsRemoteUpToDate returns true if HEAD points to the same commit as
// the origin branch
func mustIsRemoteUpToDate(dir, branch string) bool {
Expand All @@ -84,9 +131,10 @@ func mustIsRemoteUpToDate(dir, branch string) bool {
return strings.TrimSpace(localHead) == strings.TrimSpace(remoteHead)
}

func mustPush(dir, what string) {
logf("Pushing %v...", what)
if _, err := runCommand(&cmdOpts{Dir: dir}, "git", "push", "origin", what); err != nil {
func mustPush(dir string, what ...string) {
logf("Pushing %v...", strings.Join(what, " "))
args := append([]string{"git", "push", "origin"}, what...)
if _, err := runCommand(&cmdOpts{Dir: dir}, args...); err != nil {
panicf("failed to push: %v", err)
}
}
Comment on lines +134 to 140

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the signature of runCommand is func runCommand(opts *cmdOpts, name string, args ...string), passing args... directly where args contains the command name ("git") will fail to compile because Go does not allow a slice to satisfy both a regular parameter and a variadic parameter in a single call. Passing the executable name "git" explicitly as the second argument ensures compilation succeeds regardless of whether runCommand takes (opts, name, args...) or (opts, cmdAndArgs...).

Suggested change
func mustPush(dir string, what ...string) {
logf("Pushing %v...", strings.Join(what, " "))
args := append([]string{"git", "push", "origin"}, what...)
if _, err := runCommand(&cmdOpts{Dir: dir}, args...); err != nil {
panicf("failed to push: %v", err)
}
}
func mustPush(dir string, what ...string) {
logf("Pushing %v...", strings.Join(what, " "))
args := append([]string{"push", "origin"}, what...)
if _, err := runCommand(&cmdOpts{Dir: dir}, "git", args...); err != nil {
panicf("failed to push: %v", err)
}
}

Expand All @@ -106,6 +154,12 @@ func mustRecreateBranch(dir, branch string) {
}
}

func mustAddAll(dir string) {
if _, err := runCommand(&cmdOpts{Dir: dir}, "git", "add", "--all"); err != nil {
panicf("failed to git add all: %v", err)
}
}

func checkoutBranch(dir, branchName string) {
if _, err := runCommand(&cmdOpts{Dir: dir}, "git", "checkout", branchName); err != nil {
panicf(err.Error())
Expand Down
Loading
Loading