From 02d31b4f5ae203bc881f3a9650b93703984822db Mon Sep 17 00:00:00 2001
From: Francesco Romani <fromani@redhat.com>
Date: Mon, 9 Dec 2024 13:55:34 +0100
Subject: [PATCH] build: buildinfo: move buildinfo struct in internal api

use the buildinfo.json data for `pkg/version` instead
of injecting the values at link time.
This simplifies the builds and enables better introspection
capabilities.

Signed-off-by: Francesco Romani <fromani@redhat.com>
---
 .gitignore                       |  2 +
 Makefile                         | 68 ++++++++++++++++----------------
 internal/api/buildinfo/types.go  | 27 +++++++++++++
 main.go                          |  5 ++-
 pkg/version/version.go           | 32 +++++++--------
 pkg/version/version_test.go      | 12 ------
 rte/main.go                      |  5 ++-
 tools/buildhelper/buildhelper.go | 10 ++---
 8 files changed, 86 insertions(+), 75 deletions(-)
 create mode 100644 internal/api/buildinfo/types.go

diff --git a/.gitignore b/.gitignore
index e095665ed..c1b7daa07 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,3 +25,5 @@ testbin/*
 *~
 
 coverage.out
+
+pkg/version/_buildinfo.json
diff --git a/Makefile b/Makefile
index ce725e955..199c1d691 100644
--- a/Makefile
+++ b/Makefile
@@ -103,16 +103,20 @@ help: ## Display this help.
 
 ##@ Development
 
-manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
+manifests: controller-gen
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 
-generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
+generate: controller-gen
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
 
-fmt: ## Run go fmt against code.
+generate-source: pkg/version/_buildinfo.json
+
+fmt: 
 	go fmt ./...
 
-vet: ## Run go vet against code.
+# needed otherwise vet complains about trying to embed a missing file.
+# note: this would be missing only at time of vetting, which happens before the build process even starts
+vet:
 	go vet ./...
 
 gosec:
@@ -123,13 +127,13 @@ test: manifests generate fmt vet envtest ## Run tests.
 
 test-unit: test-unit-pkgs test-controllers
 
-test-unit-pkgs:
+test-unit-pkgs: generate-source pkg/
 	go test ./api/... ./pkg/... ./rte/pkg/... ./internal/... ./nrovalidate/validator/...
 
-test-unit-pkgs-cover:
+test-unit-pkgs-cover: generate-source
 	go test -coverprofile=coverage.out ./api/numaresourcesoperator/v1/helper/... ./api/numaresourcesoperator/v1alpha1/helper/... ./pkg/... ./rte/pkg/... ./internal/... ./nrovalidate/validator/...
 
-test-controllers: envtest
+test-controllers: envtest generate-source
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/...
 
 test-e2e: build-e2e-all
@@ -154,21 +158,15 @@ cover-summary:
 
 binary: build-tools
 	LDFLAGS="-s -w"; \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/version.gitcommit=$(shell bin/buildhelper commit)"; \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/version.version=$(shell bin/buildhelper version)"; \
 	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/images.tag=$(VERSION)"; \
 	go build -mod=vendor -o bin/manager -ldflags "$$LDFLAGS" -tags "$$GOTAGS" main.go
 
 binary-rte: build-tools
 	LDFLAGS="-s -w"; \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/version.gitcommit=$(shell bin/buildhelper commit)"; \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/version.version=$(shell bin/buildhelper version)"; \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/images.tag=$(VERSION)"; \
 	go build -mod=vendor -o bin/exporter -ldflags "$$LDFLAGS" -tags "$$GOTAGS" rte/main.go
 
 binary-nrovalidate: build-tools
 	LDFLAGS="-s -w"; \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/version.version=$(shell bin/buildhelper version)"; \
 	go build -mod=vendor -o bin/nrovalidate -ldflags "$$LDFLAGS" -tags "$$GOTAGS" nrovalidate/main.go
 
 binary-numacell: build-tools
@@ -181,30 +179,28 @@ binary-all: goversion \
 	binary-nrovalidate \
 	introspect-data
 
-binary-e2e-rte-local:
+binary-e2e-rte-local: generate-source
 	go test -c -v -o bin/e2e-nrop-rte-local.test ./test/e2e/rte/local
 
-binary-e2e-rte: binary-e2e-rte-local
+binary-e2e-rte: binary-e2e-rte-local generate-source
 	go test -c -v -o bin/e2e-nrop-rte.test ./test/e2e/rte
 
-binary-e2e-install:
+binary-e2e-install: generate-source
 	go test -v -c -o bin/e2e-nrop-install.test ./test/e2e/install && go test -v -c -o bin/e2e-nrop-sched-install.test ./test/e2e/sched/install
 
-binary-e2e-uninstall:
+binary-e2e-uninstall: generate-source
 	go test -v -c -o bin/e2e-nrop-uninstall.test ./test/e2e/uninstall && go test -v -c -o bin/e2e-nrop-sched-uninstall.test ./test/e2e/sched/uninstall
 
-binary-e2e-sched:
+binary-e2e-sched: generate-source
 	go test -c -v -o bin/e2e-nrop-sched.test ./test/e2e/sched
 
-binary-e2e-serial:
-	LDFLAGS+="-X github.com/openshift-kni/numaresources-operator/pkg/version.version=$(shell bin/buildhelper version) "; \
-	LDFLAGS+="-X github.com/openshift-kni/numaresources-operator/pkg/version.gitcommit=$(shell bin/buildhelper commit)"; \
+binary-e2e-serial: generate-source
 	CGO_ENABLED=0 go test -c -v -o bin/e2e-nrop-serial.test -ldflags "$$LDFLAGS" ./test/e2e/serial
 
-binary-e2e-tools:
+binary-e2e-tools: generate-source
 	go test -c -v -o bin/e2e-nrop-tools.test ./test/e2e/tools
 
-binary-e2e-must-gather:
+binary-e2e-must-gather: generate-source
 	go test -c -v -o bin/e2e-nrop-must-gather.test ./test/e2e/must-gather
 
 # backward compatibility
@@ -234,23 +230,23 @@ build-topics:
 build-buildinfo: bin/buildhelper
 	bin/buildhelper inspect > bin/buildinfo.json
 
-build: generate fmt vet binary
+build: generate generate-source fmt vet binary
 
-build-rte: fmt vet binary-rte
+build-rte: generate-source fmt vet binary-rte
 
 build-numacell: fmt vet binary-numacell
 
-build-nrovalidate: fmt vet binary-nrovalidate
+build-nrovalidate: generate-source fmt vet binary-nrovalidate
 
-build-all: generate fmt vet binary binary-rte binary-numacell binary-nrovalidate
+build-all: generate generate-source fmt vet binary binary-rte binary-numacell binary-nrovalidate
 
-build-e2e-rte: fmt vet binary-e2e-rte
+build-e2e-rte: generate-source fmt vet binary-e2e-rte
 
 build-e2e-install: fmt vet binary-e2e-install
 
 build-e2e-uninstall: fmt vet binary-e2e-uninstall
 
-build-e2e-all: fmt vet binary-e2e-all
+build-e2e-all: generate-source fmt vet binary-e2e-all
 
 build-e2e-must-gather: fmt vet binary-e2e-must-gather
 
@@ -266,7 +262,7 @@ build-pause-gcc: bin-dir
 bin-dir:
 	@mkdir -p bin || :
 
-run: manifests generate fmt vet ## Run a controller from your host.
+run: manifests generate generate-source fmt vet ## Run a controller from your host.
 	go run ./main.go
 
 # backward compatibility
@@ -405,10 +401,15 @@ goversion:
 	@go version
 
 .PHONY: build-tools
-build-tools: goversion bin/buildhelper bin/envsubst bin/lsplatform
+build-tools: goversion bin/buildhelper bin/envsubst bin/lsplatform update-buildinfo
 
 .PHONY: build-tools-all
-build-tools-all: goversion bin/buildhelper bin/envsubst bin/lsplatform bin/catkubeletconfmap bin/watchnrtattr bin/mkginkgolabelfilter bin/nrtcacheck
+build-tools-all: goversion bin/buildhelper bin/envsubst bin/lsplatform bin/catkubeletconfmap bin/watchnrtattr bin/mkginkgolabelfilter bin/nrtcacheck update-buildinfo
+
+pkg/version/_buildinfo.json: bin/buildhelper
+	@bin/buildhelper inspect > pkg/version/_buildinfo.json
+
+update-buildinfo: pkg/version/_buildinfo.json
 
 bin/buildhelper:
 	@go build -o bin/buildhelper tools/buildhelper/buildhelper.go
@@ -432,7 +433,6 @@ bin/mkginkgolabelfilter:
 
 bin/nrtcacheck: build-tools
 	LDFLAGS="-s -w" \
-	LDFLAGS+=" -X github.com/openshift-kni/numaresources-operator/pkg/version.version=$(shell bin/buildhelper version)"; \
 	go build -mod=vendor -o bin/nrtcacheck -ldflags "$$LDFLAGS" -tags "$$GOTAGS" tools/nrtcacheck/nrtcacheck.go
 
 verify-generated: bundle generate
@@ -445,7 +445,7 @@ install-git-hooks:
 
 GOLANGCI_LINT_LOCAL_VERSION := $(shell command ${GOLANGCI_LINT} --version 2> /dev/null | awk '{print $$4}')
 .PHONY: golangci-lint
-golangci-lint:
+golangci-lint: pkg/version/_buildinfo.json
 	@if [ ! -x "$(GOLANGCI_LINT)" ]; then\
 		echo "Downloading golangci-lint from https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/$(GOLANGCI_LINT_ARTIFACT_FILE)";\
 		mkdir -p $(BIN_DIR);\
diff --git a/internal/api/buildinfo/types.go b/internal/api/buildinfo/types.go
new file mode 100644
index 000000000..5f48f4946
--- /dev/null
+++ b/internal/api/buildinfo/types.go
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2024 Red Hat, Inc.
+ *
+ * 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 buildinfo
+
+type BuildInfo struct {
+	Branch  string `json:"branch"`
+	Version string `json:"version"`
+	Commit  string `json:"commit"`
+}
+
+func (bi BuildInfo) String() string {
+	return bi.Version + " " + bi.Commit + " (" + bi.Branch + ")"
+}
diff --git a/main.go b/main.go
index b0849bf39..6ff849269 100644
--- a/main.go
+++ b/main.go
@@ -171,8 +171,9 @@ func main() {
 	params.SetDefaults()
 	params.FromFlags()
 
+	bi := version.GetBuildInfo()
 	if params.showVersion {
-		fmt.Printf("%s %s %s %s\n", version.OperatorProgramName(), version.Get(), version.GetGitCommit(), runtime.Version())
+		fmt.Printf("%s %s %s\n", version.OperatorProgramName(), bi.String(), runtime.Version())
 		os.Exit(0)
 	}
 
@@ -189,7 +190,7 @@ func main() {
 	config := textlogger.NewConfig(textlogger.Verbosity(int(klogV)))
 	ctrl.SetLogger(textlogger.NewLogger(config))
 
-	klog.InfoS("starting", "program", version.OperatorProgramName(), "version", version.Get(), "gitcommit", version.GetGitCommit(), "golang", runtime.Version(), "vl", klogV, "auxv", config.Verbosity().String())
+	klog.InfoS("starting", "program", version.OperatorProgramName(), "version", bi.Version, "branch", bi.Branch, "gitcommit", bi.Commit, "golang", runtime.Version(), "vl", klogV, "auxv", config.Verbosity().String())
 
 	ctx := context.Background()
 
diff --git a/pkg/version/version.go b/pkg/version/version.go
index 3ff0bba7e..fc059ef7c 100644
--- a/pkg/version/version.go
+++ b/pkg/version/version.go
@@ -17,36 +17,32 @@
 package version
 
 import (
+	_ "embed"
+	"encoding/json"
 	"os"
 	"path/filepath"
-)
 
-const (
-	undefined string = "undefined"
+	"github.com/openshift-kni/numaresources-operator/internal/api/buildinfo"
 )
 
-// version Must not be const, supposed to be set using ldflags at build time
-var version = undefined
+//go:embed _buildinfo.json
+var buildInfo string
 
-// gitcommit Must not be const, supposed to be set using ldflags at build time
-var gitcommit = undefined
+func GetBuildInfo() buildinfo.BuildInfo {
+	var bi buildinfo.BuildInfo
+	_ = json.Unmarshal([]byte(buildInfo), &bi)
+	return bi
+}
 
 // Get returns the version as a string
 func Get() string {
-	return version
-}
-
-// Undefined returns if version is at it's default value
-func Undefined() bool {
-	return version == undefined
+	bi := GetBuildInfo()
+	return bi.Version
 }
 
 func GetGitCommit() string {
-	return gitcommit
-}
-
-func UndefinedGitCommit() bool {
-	return gitcommit == undefined
+	bi := GetBuildInfo()
+	return bi.Commit
 }
 
 func ProgramName() string {
diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go
index 258ede874..594bb427a 100644
--- a/pkg/version/version_test.go
+++ b/pkg/version/version_test.go
@@ -24,24 +24,12 @@ func TestGet(t *testing.T) {
 	}
 }
 
-func TestUndefined(t *testing.T) {
-	if !Undefined() {
-		t.Errorf("final version should be defined at link stage")
-	}
-}
-
 func TestGetGitCommit(t *testing.T) {
 	if GetGitCommit() == "" {
 		t.Errorf("empty gitcommit")
 	}
 }
 
-func TestUndefinedGitCommit(t *testing.T) {
-	if !UndefinedGitCommit() {
-		t.Errorf("final gitcommit should be defined at link stage")
-	}
-}
-
 func TestProgramName(t *testing.T) {
 	if ProgramName() == "undefined" {
 		t.Errorf("program name should always be defined")
diff --git a/rte/main.go b/rte/main.go
index 5f580ce4c..e484a9abb 100644
--- a/rte/main.go
+++ b/rte/main.go
@@ -45,7 +45,8 @@ const (
 )
 
 func main() {
-	klog.Infof("starting %s %s %s %s\n", version.ExporterProgramName(), version.Get(), version.GetGitCommit(), runtime.Version())
+	bi := version.GetBuildInfo()
+	klog.Infof("starting %s %s %s\n", version.ExporterProgramName(), bi.String(), runtime.Version())
 
 	parsedArgs, err := parseArgs(os.Args[1:]...)
 	if err != nil {
@@ -53,7 +54,7 @@ func main() {
 	}
 
 	if parsedArgs.Version {
-		fmt.Printf("%s %s %s %s\n", version.ExporterProgramName(), version.Get(), version.GetGitCommit(), runtime.Version())
+		fmt.Printf("%s %s %s\n", version.ExporterProgramName(), bi.String(), runtime.Version())
 		os.Exit(0)
 	}
 
diff --git a/tools/buildhelper/buildhelper.go b/tools/buildhelper/buildhelper.go
index d41e41e3e..cb67f8fbf 100644
--- a/tools/buildhelper/buildhelper.go
+++ b/tools/buildhelper/buildhelper.go
@@ -25,6 +25,8 @@ import (
 	"strings"
 
 	"github.com/mdomke/git-semver/version"
+
+	"github.com/openshift-kni/numaresources-operator/internal/api/buildinfo"
 )
 
 const (
@@ -32,12 +34,6 @@ const (
 	releaseBranchPrefix = "release-"
 )
 
-type buildInfo struct {
-	Branch  string `json:"branch"`
-	Version string `json:"version"`
-	Commit  string `json:"commit"`
-}
-
 func getVersion() (string, error) {
 	if ver, ok := os.LookupEnv("NRO_BUILD_VERSION"); ok {
 		return ver, nil
@@ -108,7 +104,7 @@ func showBranch() int {
 }
 
 func inspect() int {
-	var bi buildInfo
+	var bi buildinfo.BuildInfo
 	var err error
 
 	bi.Version, err = getVersion()