Skip to content

Commit

Permalink
Semver version validation (gravitational#30480)
Browse files Browse the repository at this point in the history
Tagging a release with an invalid semver string (especially an almost-but-
not-quite-valid semver string) can cause an incorrect environment to be
selected when publishing packages during promotion, with the attendant
risk of polluting production artefact repositories.

This change attempts to force the use of correctly-formed semver strings by

 - validating the semver string on application (i.e. as part of make version)
 - validating that the GITTAG value used to trigger a build is valid semver, and
 - as a harm reduction measure, changing the semver check tool to interpret
   an invalid semver string as if it had pre-release or build metadata.
   This is a backstop, just in case a malformed tag sneaks through outside of
   our automation.

Changelog: none
  • Loading branch information
tcsc authored Aug 16, 2023
1 parent c731b81 commit a39c84b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 7 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,7 @@ $(VERSRC): Makefile
update-tag: TAG_REMOTE ?= origin
update-tag:
@test $(VERSION)
cd build.assets/tooling && go run ./cmd/check -check valid -tag $(GITTAG)
git tag $(GITTAG)
git tag api/$(GITTAG)
(cd e && git tag $(GITTAG) && git push origin $(GITTAG))
Expand Down
32 changes: 28 additions & 4 deletions build.assets/tooling/cmd/check/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,16 @@ func main() {
switch check {
case "latest":
err = checkLatest(ctx, tag, github.NewGitHub())

case "prerelease":
err = checkPrerelease(tag)
// Assert that the supplied tag is a valid semver version with no
// prerelease or build-metadata components
err = checkIsBareRelease(tag)

case "valid":
// Assert that the supplied tag is a valid semver version string.
err = checkValidSemver(tag)

default:
log.Fatalf("invalid check: %v", check)
}
Expand All @@ -58,7 +66,7 @@ func main() {

func parseFlags() (string, string, error) {
tag := flag.String("tag", "", "tag to validate")
check := flag.String("check", "", "check to run [latest, prerelease]")
check := flag.String("check", "", "check to run [latest, prerelease, valid]")
flag.Parse()

if *tag == "" {
Expand All @@ -68,7 +76,7 @@ func parseFlags() (string, string, error) {
return "", "", trace.BadParameter("check missing")
}
switch *check {
case "latest", "prerelease":
case "latest", "prerelease", "valid":
default:
return "", "", trace.BadParameter("invalid check: %v", *check)
}
Expand Down Expand Up @@ -108,7 +116,23 @@ func checkLatest(ctx context.Context, tag string, gh github.GitHub) error {
return nil
}

func checkPrerelease(tag string) error {
// checkValidSemver returns an error if the supplied string is not a valid
// Semver identifier
func checkValidSemver(tag string) error {
if !semver.IsValid(tag) {
return trace.BadParameter("version is invalid semver: %v", tag)
}
return nil
}

// checkIsBareRelease returns nil if the supplied tag is a valid semver
// version string without pre-release or build-metadata components. Returns
// an error if any of these conditions is not met.
func checkIsBareRelease(tag string) error {
if err := checkValidSemver(tag); err != nil {
return trace.Wrap(err)
}

if semver.Prerelease(tag) != "" { // https://semver.org/#spec-item-9
return trace.BadParameter("version is pre-release: %v", tag)
}
Expand Down
9 changes: 7 additions & 2 deletions build.assets/tooling/cmd/check/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/stretchr/testify/require"
)

func TestCheckPrerelease(t *testing.T) {
func TestCheckIsBareRelease(t *testing.T) {
tests := []struct {
desc string
tag string
Expand All @@ -50,10 +50,15 @@ func TestCheckPrerelease(t *testing.T) {
tag: "v8.0.1",
wantErr: require.NoError,
},
{
desc: "fail-leading-zero",
tag: "v13.3.3-tcsc.02",
wantErr: require.Error,
},
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
test.wantErr(t, checkPrerelease(test.tag))
test.wantErr(t, checkIsBareRelease(test.tag))
})
}

Expand Down
6 changes: 5 additions & 1 deletion version.mk
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func init() { Gitref = \"$(GITREF)\" }\n"
# setver updates version.go and gitref.go with VERSION and GITREF vars
#
.PHONY:setver
setver: helm-version tsh-version
setver: validate-semver helm-version tsh-version
@printf $(VERSION_GO) | gofmt > version.go
@printf $(API_VERSION_GO) | gofmt > ./api/version.go
@printf $(UPDATER_VERSION_GO) | gofmt > ./integrations/kube-agent-updater/version.go
Expand All @@ -56,3 +56,7 @@ PLIST_FILES := $(abspath $(TSH_APP_PLISTS))
.PHONY:tsh-version
tsh-version:
cd build.assets/tooling && go run ./cmd/update-plist-version $(VERSION) $(PLIST_FILES)

.PHONY:validate-semver
validate-semver:
cd build.assets/tooling && go run ./cmd/check -check valid -tag v$(VERSION)

0 comments on commit a39c84b

Please sign in to comment.