From 89a23770633b916e6991637c6d07cb2ae3aaa34a Mon Sep 17 00:00:00 2001 From: ffranr Date: Thu, 21 Dec 2023 12:37:18 +0000 Subject: [PATCH 1/8] version: separate status from pre-release The modifications in this commit streamline version.go for easier parsing with bash, setting the stage for enhancements in a subsequent commit. --- version.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/version.go b/version.go index d7714f433..60361a33a 100644 --- a/version.go +++ b/version.go @@ -47,9 +47,21 @@ const ( // AppPatch defines the application patch for this binary. AppPatch uint = 2 - // AppPreRelease MUST only contain characters from semanticAlphabet - // per the semantic versioning spec. - AppPreRelease = "alpha" + // AppStatus defines the release status of this binary (e.g. beta). + AppStatus = "alpha" + + // AppPreRelease defines the pre-release version of this binary. + // It MUST only contain characters from the semantic versioning spec. + AppPreRelease = "" + + // GitTagIncludeStatus indicates whether the status should be included + // in the git tag name. + // + // Including the app version status in the git tag may be problematic + // for golang projects when importing them as dependencies. We therefore + // include this flag to allow toggling the status on and off in a + // standardised way across our projects. + GitTagIncludeStatus = false // defaultAgentName is the default name of the software that is added as // the first part of the user agent string. @@ -104,9 +116,19 @@ func UserAgent(initiator string) string { } func init() { + // Assert that AppStatus is valid according to the semantic versioning + // guidelines for pre-release version and build metadata strings. In + // particular, it MUST only contain characters in semanticAlphabet. + for _, r := range AppStatus { + if !strings.ContainsRune(semanticAlphabet, r) { + panic(fmt.Errorf("rune: %v is not in the semantic "+ + "alphabet", r)) + } + } + // Assert that AppPreRelease is valid according to the semantic // versioning guidelines for pre-release version and build metadata - // strings. In particular it MUST only contain characters in + // strings. In particular, it MUST only contain characters in // semanticAlphabet. for _, r := range AppPreRelease { if !strings.ContainsRune(semanticAlphabet, r) { @@ -162,12 +184,28 @@ func semanticVersion() string { // Start with the major, minor, and patch versions. version := fmt.Sprintf("%d.%d.%d", AppMajor, AppMinor, AppPatch) - // Append pre-release version if there is one. The hyphen called for - // by the semantic versioning spec is automatically appended and should - // not be contained in the pre-release string. The pre-release version + // If defined, we will now sanitise the release status string. The + // hyphen called for by the semantic versioning spec is automatically + // appended and should not be contained in the status string. The status // is not appended if it contains invalid characters. + appStatus := normalizeVerString(AppStatus, semanticAlphabet) + + // If defined, we will now sanitise the pre-release version string. The + // hyphen called for by the semantic versioning spec is automatically + // appended and should not be contained in the pre-release string. + // The pre-release version is not appended if it contains invalid + // characters. preRelease := normalizeVerString(AppPreRelease, semanticAlphabet) - if preRelease != "" { + + // Append any status and pre-release strings to the version string. + switch { + case appStatus != "" && preRelease != "": + version = fmt.Sprintf( + "%s-%s.%s", version, appStatus, preRelease, + ) + case appStatus != "": + version = fmt.Sprintf("%s-%s", version, appStatus) + case preRelease != "": version = fmt.Sprintf("%s-%s", version, preRelease) } From 3da789efc243ccb4d55623402f5e9de880dc91ba Mon Sep 17 00:00:00 2001 From: ffranr Date: Thu, 11 Jan 2024 15:32:16 +0000 Subject: [PATCH 2/8] version: simplify version semantic alphabet This commit removes the characters `.`, `-`, and ` ` from the semantic alphabet. --- version.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/version.go b/version.go index 60361a33a..ca1b878fc 100644 --- a/version.go +++ b/version.go @@ -31,9 +31,9 @@ var ( GoVersion string ) -// semanticAlphabet is the set of characters that are permitted for use in an -// AppPreRelease. -const semanticAlphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-." +// semanticAlphabet is the set of characters that are permitted for use in +// AppStatus and AppPreRelease. +const semanticAlphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" // These constants define the application version and follow the semantic // versioning 2.0.0 spec (http://semver.org/). @@ -78,8 +78,10 @@ var agentName = defaultAgentName // the software tapd is bundled in (for example LiT). This function panics if // the agent name contains characters outside of the allowed semantic alphabet. func SetAgentName(newAgentName string) { + agentNameAlphabet := semanticAlphabet + "-. " + for _, r := range newAgentName { - if !strings.ContainsRune(semanticAlphabet, r) { + if !strings.ContainsRune(agentNameAlphabet, r) { panic(fmt.Errorf("rune: %v is not in the semantic "+ "alphabet", r)) } @@ -93,7 +95,7 @@ func SetAgentName(newAgentName string) { func UserAgent(initiator string) string { // We'll only allow "safe" characters in the initiator portion of the // user agent string and spaces only if surrounded by other characters. - initiatorAlphabet := semanticAlphabet + ". " + initiatorAlphabet := semanticAlphabet + "-. " cleanInitiator := normalizeVerString( strings.TrimSpace(initiator), initiatorAlphabet, ) From 183b11a124d127828303d7953de616aa9dc935dd Mon Sep 17 00:00:00 2001 From: ffranr Date: Tue, 5 Dec 2023 15:21:26 +0000 Subject: [PATCH 3/8] makefile: add command release-tag for generating a release git tag This commit introduces a makefile command that generates a git release tag by parsing the version string from version.go via the newly created script get-git-tag-name.sh. --- Makefile | 14 +++++++ scripts/get-git-tag-name.sh | 77 +++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100755 scripts/get-git-tag-name.sh diff --git a/Makefile b/Makefile index b80964f03..f3d0501f1 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,9 @@ GOACC_BIN := $(GO_BIN)/go-acc GOIMPORTS_BIN := $(GO_BIN)/gosimports MIGRATE_BIN := $(GO_BIN)/migrate +# VERSION_GO_FILE is the golang file which defines the current project version. +VERSION_GO_FILE := "version.go" + COMMIT := $(shell git describe --tags --dirty) GOBUILD := GOEXPERIMENT=loopvar GO111MODULE=on go build -v @@ -115,6 +118,17 @@ release: $(VERSION_CHECK) ./scripts/release.sh build-release "$(VERSION_TAG)" "$(BUILD_SYSTEM)" "$(RELEASE_TAGS)" "$(RELEASE_LDFLAGS)" +release-tag: + @$(call print, "Adding release tag.") + + tag=$$(./scripts/get-git-tag-name.sh ${VERSION_GO_FILE}); \ + exit_status=$$?; \ + if [ $$exit_status -ne 0 ]; then \ + echo "Script encountered an error with exit status $$exit_status."; \ + fi; \ + echo "Adding git tag: $$tag"; \ + git tag -as -m "Tag generated using command \`make release-tag\`." "$$tag"; + docker-release: @$(call print, "Building release helper docker image.") if [ "$(tag)" = "" ]; then echo "Must specify tag=!"; exit 1; fi diff --git a/scripts/get-git-tag-name.sh b/scripts/get-git-tag-name.sh new file mode 100755 index 000000000..b31849412 --- /dev/null +++ b/scripts/get-git-tag-name.sh @@ -0,0 +1,77 @@ +#!/bin/bash + +# This script derives a git tag name from the version fields found in a given Go +# file. It also checks if the derived git tag name is a valid SemVer compliant +# version string. + +# get_git_tag_name reads the version fields from the given file and then +# constructs and returns a git tag name. +get_git_tag_name() { + local file_path="$1" + + # Check if the file exists + if [ ! -f "$file_path" ]; then + echo "Error: File not found at $file_path" >&2 + exit 1 + fi + + # Read and parse the version fields. We interpret these fields using regex + # matching which effectively serves as a basic sanity check. + local app_major + app_major=$(grep -oP 'AppMajor\s*uint\s*=\s*\K\d+' "$file_path") + + local app_minor + app_minor=$(grep -oP 'AppMinor\s*uint\s*=\s*\K\d+' "$file_path") + + local app_patch + app_patch=$(grep -oP 'AppPatch\s*uint\s*=\s*\K\d+' "$file_path") + + local app_status + app_status=$(grep -oP 'AppStatus\s*=\s*"\K([a-z]*)' "$file_path") + + local app_pre_release + app_pre_release=$(grep -oP 'AppPreRelease\s*=\s*"\K([a-z0-9]*)' "$file_path") + + # Parse the GitTagIncludeStatus field. + local git_tag_include_status + git_tag_include_status=false + + if grep -q 'GitTagIncludeStatus = true' "$file_path"; then + git_tag_include_status=true + elif grep -q 'GitTagIncludeStatus = false' "$file_path"; then + git_tag_include_status=false + else + echo "Error: GitTagIncludeStatus is not present in the Go version file." + exit 1 + fi + + # Construct the git tag name with conditional inclusion of app_status and + # app_pre_release. + tag_name="v${app_major}.${app_minor}.${app_patch}" + + # Append app_status if git_tag_include_status is true and app_status if + # specified. + if [ "$git_tag_include_status" = true ] && [ -n "$app_status" ]; then + tag_name+="-${app_status}" + + # Append app_pre_release if specified. + if [ -n "$app_pre_release" ]; then + tag_name+=".${app_pre_release}" + fi + else + # If the app_status field is not specified, then append + # app_pre_release (if specified) using a dash prefix. + if [ -n "$app_pre_release" ]; then + tag_name+="-${app_pre_release}" + fi + fi + + echo "$tag_name" +} + +file_path="$1" +echo "Reading version fields from file: $file_path" >&2 +tag_name=$(get_git_tag_name "$file_path") +echo "Derived git tag name: $tag_name" >&2 + +echo "$tag_name" From 5d1fd00fa4ae6bf3c7c7bfb24ddbf1ebf29dea9e Mon Sep 17 00:00:00 2001 From: ffranr Date: Tue, 5 Dec 2023 12:32:35 +0000 Subject: [PATCH 4/8] .github/workflows: validate release tag version string This commit adds a check to this project's GitHub release workflow to ensure that the release version string is SemVer compliant. It also indirectly ensures that the version string matches the tag that would have been created with the `make release-tag` command. --- .github/workflows/release.yaml | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index e89a700c7..161741a97 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -22,14 +22,26 @@ jobs: with: fetch-depth: 0 + - name: Set env + run: echo "RELEASE_VERSION=${GITHUB_REF#refs/*/}" >> $GITHUB_ENV + + - name: Validate release tag ${{ env.RELEASE_VERSION }} + run: | + expected_tag=$(./scripts/get-git-tag-name.sh version.go) + actual_tag=${{ env.RELEASE_VERSION }} + + if [ "$actual_tag" = "$expected_tag" ]; then + echo "Git tag release string is as expected." + else + echo "Error: Versions are not equal. Actual: $actual_tag, Expected: $expected_tag" + exit 1 + fi + - name: setup go ${{ env.GO_VERSION }} uses: actions/setup-go@v2 with: go-version: '${{ env.GO_VERSION }}' - - name: Set env - run: echo "RELEASE_VERSION=${GITHUB_REF#refs/*/}" >> $GITHUB_ENV - - name: build release for all architectures run: SKIP_VERSION_CHECK=1 make release tag=${{ env.RELEASE_VERSION }} From 6114235840660d683a3a7b1fa7e8eb3e32632122 Mon Sep 17 00:00:00 2001 From: ffranr Date: Thu, 21 Dec 2023 13:38:55 +0000 Subject: [PATCH 5/8] .github/workflows: enforce version check when building release binaries --- .github/workflows/release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 161741a97..bfad6e346 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -43,7 +43,7 @@ jobs: go-version: '${{ env.GO_VERSION }}' - name: build release for all architectures - run: SKIP_VERSION_CHECK=1 make release tag=${{ env.RELEASE_VERSION }} + run: make release tag=${{ env.RELEASE_VERSION }} - name: Create Release uses: lightninglabs/gh-actions/action-gh-release@2021.01.25.00 From c82d0792828fd0a568979c7933f9861882d3f82f Mon Sep 17 00:00:00 2001 From: ffranr Date: Wed, 24 Jan 2024 14:50:59 +0000 Subject: [PATCH 6/8] version: narrow alphabet of acceptable version string field characters This commit narrows the field of acceptable characters to 0-9 and a-z. The alphabet is compliant with SemVer but also stricter. --- version.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/version.go b/version.go index ca1b878fc..e561bef62 100644 --- a/version.go +++ b/version.go @@ -31,9 +31,9 @@ var ( GoVersion string ) -// semanticAlphabet is the set of characters that are permitted for use in -// AppStatus and AppPreRelease. -const semanticAlphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" +// versionFieldsAlphabet is the set of characters that are permitted for use in +// a version string field. +const versionFieldsAlphabet = "0123456789abcdefghijklmnopqrstuvwxyz" // These constants define the application version and follow the semantic // versioning 2.0.0 spec (http://semver.org/). @@ -78,7 +78,7 @@ var agentName = defaultAgentName // the software tapd is bundled in (for example LiT). This function panics if // the agent name contains characters outside of the allowed semantic alphabet. func SetAgentName(newAgentName string) { - agentNameAlphabet := semanticAlphabet + "-. " + agentNameAlphabet := versionFieldsAlphabet + "-. " for _, r := range newAgentName { if !strings.ContainsRune(agentNameAlphabet, r) { @@ -95,7 +95,7 @@ func SetAgentName(newAgentName string) { func UserAgent(initiator string) string { // We'll only allow "safe" characters in the initiator portion of the // user agent string and spaces only if surrounded by other characters. - initiatorAlphabet := semanticAlphabet + "-. " + initiatorAlphabet := versionFieldsAlphabet + "-. " cleanInitiator := normalizeVerString( strings.TrimSpace(initiator), initiatorAlphabet, ) @@ -120,9 +120,9 @@ func UserAgent(initiator string) string { func init() { // Assert that AppStatus is valid according to the semantic versioning // guidelines for pre-release version and build metadata strings. In - // particular, it MUST only contain characters in semanticAlphabet. + // particular, it MUST only contain characters in versionFieldsAlphabet. for _, r := range AppStatus { - if !strings.ContainsRune(semanticAlphabet, r) { + if !strings.ContainsRune(versionFieldsAlphabet, r) { panic(fmt.Errorf("rune: %v is not in the semantic "+ "alphabet", r)) } @@ -131,9 +131,9 @@ func init() { // Assert that AppPreRelease is valid according to the semantic // versioning guidelines for pre-release version and build metadata // strings. In particular, it MUST only contain characters in - // semanticAlphabet. + // versionFieldsAlphabet. for _, r := range AppPreRelease { - if !strings.ContainsRune(semanticAlphabet, r) { + if !strings.ContainsRune(versionFieldsAlphabet, r) { panic(fmt.Errorf("rune: %v is not in the semantic "+ "alphabet", r)) } @@ -190,14 +190,14 @@ func semanticVersion() string { // hyphen called for by the semantic versioning spec is automatically // appended and should not be contained in the status string. The status // is not appended if it contains invalid characters. - appStatus := normalizeVerString(AppStatus, semanticAlphabet) + appStatus := normalizeVerString(AppStatus, versionFieldsAlphabet) // If defined, we will now sanitise the pre-release version string. The // hyphen called for by the semantic versioning spec is automatically // appended and should not be contained in the pre-release string. // The pre-release version is not appended if it contains invalid // characters. - preRelease := normalizeVerString(AppPreRelease, semanticAlphabet) + preRelease := normalizeVerString(AppPreRelease, versionFieldsAlphabet) // Append any status and pre-release strings to the version string. switch { From ee0b36a5c9ad3f4b328471134ee44c209fdfe2b8 Mon Sep 17 00:00:00 2001 From: ffranr Date: Wed, 24 Jan 2024 16:09:06 +0000 Subject: [PATCH 7/8] scripts: `make release` checks git tag using script get-git-tag-name.sh This commit modifies the release build script such that it checks the validity of the checked-out git tag by cross-referencing with the output of the `get-git-tag-name.sh` script. --- make/release_flags.mk | 2 +- scripts/release.sh | 34 ++++++++++------------------------ 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/make/release_flags.mk b/make/release_flags.mk index 6af613434..6c4b41a3a 100644 --- a/make/release_flags.mk +++ b/make/release_flags.mk @@ -26,7 +26,7 @@ RELEASE_TAGS = monitoring # from the current date. ifneq ($(tag),) VERSION_TAG = $(tag) -VERSION_CHECK = ./scripts/release.sh check-tag "$(VERSION_TAG)" +VERSION_CHECK = ./scripts/release.sh check-tag "$(VERSION_TAG)" "$(VERSION_GO_FILE)" endif # By default we will build all systems. But with the 'sys' tag, a specific diff --git a/scripts/release.sh b/scripts/release.sh index 152b0fdcf..3d28a9542 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -80,9 +80,10 @@ function red() { # check_tag_correct makes sure the given git tag is checked out and the git tree # is not dirty. -# arguments: +# arguments: function check_tag_correct() { local tag=$1 + local version_file_path=$2 # For automated builds we can skip this check as they will only be triggered # on tags. @@ -102,31 +103,16 @@ function check_tag_correct() { echo "Tag $tag checked out. Git commit: $commit_hash" fi - # Build tapd to extract version. - go build ${PKG}/cmd/tapd + # Ensure that the git tag matches the version string derived from the version + # file. + local expected_tag + expected_tag=$(./scripts/get-git-tag-name.sh "$version_file_path") - # Extract version command output. - tapd_version_output=$(./tapd --version) - - # Use a regex to isolate the version string. - if [[ $tapd_version_output =~ $TAPD_VERSION_REGEX ]]; then - # Prepend 'v' to match git tag naming scheme. - tapd_version="v${BASH_REMATCH[1]}" - green "version: $tapd_version" - - # If the tapd reported version contains a suffix, remove it, so we can match - # the tag properly. - # shellcheck disable=SC2001 - tapd_version=$(echo "$tapd_version" | sed -e 's/-\(alpha\|beta\)\(\.rc[0-9]\+\)\?//g') - - # Match git tag with tapd version. - if [[ $tag != "${tapd_version}" ]]; then - red "tapd version $tapd_version does not match tag $tag" - exit 1 - fi - else - red "malformed tapd version output" + if [[ $tag != "$expected_tag" ]]; then + red "Error: tag $tag does not match git tag version string derived from $version_file_path" exit 1 + else + green "tag $tag matches git tag version string derived from $version_file_path" fi } From 4d6aa3a5da86f68ef5c3b79fad0b72c3fb206efb Mon Sep 17 00:00:00 2001 From: ffranr Date: Wed, 24 Jan 2024 16:12:25 +0000 Subject: [PATCH 8/8] make: cluster version flags together for clarity --- make/release_flags.mk | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/make/release_flags.mk b/make/release_flags.mk index 6c4b41a3a..45defe716 100644 --- a/make/release_flags.mk +++ b/make/release_flags.mk @@ -1,6 +1,13 @@ +# One can either specify a git tag as the version suffix or one that is +# generated from the current date. VERSION_TAG = $(shell date +%Y%m%d)-01 VERSION_CHECK = @$(call print, "Building master with date version tag") +ifneq ($(tag),) +VERSION_TAG = $(tag) +VERSION_CHECK = ./scripts/release.sh check-tag "$(VERSION_TAG)" "$(VERSION_GO_FILE)" +endif + DOCKER_RELEASE_HELPER = docker run \ -it \ --rm \ @@ -22,13 +29,6 @@ windows-amd64 RELEASE_TAGS = monitoring -# One can either specify a git tag as the version suffix or one is generated -# from the current date. -ifneq ($(tag),) -VERSION_TAG = $(tag) -VERSION_CHECK = ./scripts/release.sh check-tag "$(VERSION_TAG)" "$(VERSION_GO_FILE)" -endif - # By default we will build all systems. But with the 'sys' tag, a specific # system can be specified. This is useful to release for a subset of # systems/architectures.