Fix version command to show correct information (istio#14754)

* Fix version command to show correct information

A recent change made the ldflags not get passed properly leading to the
version command giving "unknown". This change fixes this issue, and adds
a tests to ensure similar changes are caught in the future.

During this it was discovered some of our binaries don't include the
version command, so those were added to sdsclient and mixgen.

* Fix lint and codecov

* Fix lint

Author: howardjohn

Makefile

@@ -370,6 +370,11 @@ ${ISTIO_OUT}/sdsclient:
 # Build will rebuild the go binaries.
 build: depend $(PILOT_GO_BINS_SHORT) mixc mixs mixgen node_agent node_agent_k8s istio_ca istioctl galley sdsclient
 
+.PHONY: version-test
+# Do not run istioctl since is different (connects to kubernetes)
+version-test:
+	go test ./tests/version/... -v --base-dir ${ISTIO_OUT} --binaries="$(PILOT_GO_BINS_SHORT) mixc mixs mixgen node_agent node_agent_k8s istio_ca galley sdsclient"
+
 # The following are convenience aliases for most of the go targets
 # The first block is for aliases that are the same as the actual binary,
 # while the ones that follow need slight adjustments to their names.

bin/gobuild.sh

@@ -36,7 +36,7 @@ GOBINARY=${GOBINARY:-go}
 GOPKG="$GOPATH/pkg"
 BUILDINFO=${BUILDINFO:-""}
 STATIC=${STATIC:-1}
-LDFLAGS=${LDFLAGS:-extldflags -static}
+LDFLAGS=${LDFLAGS:--extldflags -static}
 GOBUILDFLAGS=${GOBUILDFLAGS:-""}
 # Split GOBUILDFLAGS by spaces into an array called GOBUILDFLAGS_ARRAY.
 IFS=' ' read -r -a GOBUILDFLAGS_ARRAY <<< "$GOBUILDFLAGS"

codecov.skip

@@ -18,6 +18,7 @@ istio.io/istio/samples
 istio.io/istio/security/tests/integration
 istio.io/istio/tests/codecov
 istio.io/istio/tests/e2e
+istio.io/istio/tests/version
 istio.io/istio/tests/integration2/examples
 istio.io/istio/tests/integration2/qualification
 istio.io/istio/tests/integration_old

mixer/tools/mixgen/main.go

@@ -19,11 +19,14 @@ import (
 
 	"istio.io/istio/mixer/cmd/shared"
 	"istio.io/istio/mixer/tools/mixgen/cmd"
+	"istio.io/pkg/version"
 )
 
 func main() {
 	rootCmd := cmd.GetRootCmd(os.Args[1:], shared.Printf, shared.Fatalf)
 
+	rootCmd.AddCommand(version.CobraCommand())
+
 	if err := rootCmd.Execute(); err != nil {
 		os.Exit(-1)
 	}

prow/istio-unit-tests.sh

@@ -34,4 +34,4 @@ cd "${ROOT}"
 # Integration/e2e tests in the other scripts are run against GKE or real clusters.
 JUNIT_UNIT_TEST_XML="${ARTIFACTS_DIR}/junit_unit-tests.xml" \
 T="-v" \
-make build localTestEnv test
+make build localTestEnv test version-test

security/tools/sdsclient/main.go

@@ -20,6 +20,8 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"istio.io/pkg/version"
+
 	"istio.io/istio/pkg/cmd"
 	"istio.io/istio/security/pkg/testing/sdsc"
 	"istio.io/pkg/env"
@@ -48,6 +50,10 @@ var (
 	}
 )
 
+func init() {
+	rootCmd.AddCommand(version.CobraCommand())
+}
+
 func main() {
 	if err := rootCmd.Execute(); err != nil {
 		log.Fatalf("failed to start the sdsclient, error %v", err)

tests/version/version_test.go

@@ -0,0 +1,79 @@
+// Copyright 2019 Istio Authors
+//
+// 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 version
+
+import (
+	"encoding/json"
+	"flag"
+	"os/exec"
+	"path"
+	"strings"
+	"testing"
+
+	"istio.io/pkg/version"
+)
+
+var (
+	binaries   *string
+	releasedir *string
+)
+
+func init() {
+	releasedir = flag.String("base-dir", "", "directory for binaries")
+	binaries = flag.String("binaries", "", "space separated binaries to test")
+	flag.Parse()
+
+}
+
+func TestVersion(t *testing.T) {
+	binariesToTest := strings.Split(*binaries, " ")
+	if len(binariesToTest) == 0 {
+		t.Fatal("No binaries to test. Pass the --binaries flag.")
+	}
+	for _, b := range binariesToTest {
+		cmd := path.Join(*releasedir, b)
+		t.Run(b, func(t *testing.T) {
+			out, err := exec.Command(cmd, "version", "-ojson").Output()
+			if err != nil {
+				t.Fatalf("Version failed with error: %v. Output: %v", err, string(out))
+			}
+
+			var resp version.Version
+			if err := json.Unmarshal(out, &resp); err != nil {
+				t.Fatalf("Failed to marshal to json: %v. Output: %v", err, string(out))
+			}
+
+			verInfo := resp.ClientVersion
+
+			validateField(t, "Version", verInfo.Version)
+			validateField(t, "GitRevision", verInfo.GitRevision)
+			validateField(t, "User", verInfo.User)
+			validateField(t, "Host", verInfo.Host)
+			validateField(t, "GolangVersion", verInfo.GolangVersion)
+			validateField(t, "DockerHub", verInfo.DockerHub)
+			validateField(t, "BuildStatus", verInfo.BuildStatus)
+			validateField(t, "GitTag", verInfo.GitTag)
+		})
+	}
+}
+
+// TODO we may be able to do more validation of fields here, but because it changes based on the environment this is not easy
+// For now ensuring the fields even get populated is most important.
+func validateField(t *testing.T, field, s string) {
+	t.Helper()
+	if s == "unknown" || s == "" {
+		t.Errorf("Field %v was invalid. Got: %v.", field, s)
+	}
+}