auxdriver: Strict parsing of driver version

Add a driverVersion helper and auxdriver.Version type for parsing driver
version yaml output. The helper read the output of the "driver version",
parses the yaml, and validate that both version and commit are set.

This will make it possible to validate the driver commit hash during the
tests to ensure we test the driver built from the current code.

We log or include in the error message both the driver version and
commit hash, to make it easier to debug issues related to using the
wrong driver.

This changes fixes possible issue if "driver version" command logs
errors or warnings. Previously this could break the code parsing the
version since we combined stdout and stderr. Now we extract stderr from
the command ExitError on errors.

Author: nirs

go.mod

@@ -85,6 +85,7 @@ require (
 	k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
 	libvirt.org/go/libvirt v1.11006.0
 	sigs.k8s.io/sig-storage-lib-external-provisioner/v6 v6.3.0
+	sigs.k8s.io/yaml v1.4.0
 )
 
 require (
@@ -252,7 +253,6 @@ require (
 	sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect
 	sigs.k8s.io/randfill v1.0.0 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect
-	sigs.k8s.io/yaml v1.4.0 // indirect
 )
 
 replace (

pkg/minikube/driver/auxdriver/install.go

@@ -21,7 +21,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"time"
 
@@ -127,22 +126,17 @@ func validateDriver(executable string, minimalVersion semver.Version) (string, e
 		return path, fmt.Errorf("failed to find driver %q: %w", executable, err)
 	}
 
-	cmd := exec.Command(path, "version")
-	output, err := cmd.CombinedOutput()
+	dv, err := driverVersion(path)
 	if err != nil {
-		return path, fmt.Errorf("%s failed: %w: %q", cmd, err, output)
+		return path, err
 	}
 
-	ev := extractDriverVersion(string(output))
-	if len(ev) == 0 {
-		return path, fmt.Errorf("%s: unable to extract version from %q", executable, output)
-	}
+	klog.Infof("%s version is %+v", path, dv)
 
-	actualVersion, err := semver.Make(ev)
+	actualVersion, err := semver.Make(strings.TrimPrefix(dv.Version, "v"))
 	if err != nil {
 		return path, fmt.Errorf("%s: invalid driver version: %w", executable, err)
 	}
-	klog.Infof("%s version is %s", path, actualVersion)
 
 	if actualVersion.LT(minimalVersion) {
 		return path, fmt.Errorf("%s is version %s, want %s or later", executable, actualVersion, minimalVersion)
@@ -151,23 +145,6 @@ func validateDriver(executable string, minimalVersion semver.Version) (string, e
 	return path, nil
 }
 
-// extractDriverVersion extracts the driver version.
-// KVM and Hyperkit drivers support the 'version' command, that display the information as:
-// version: vX.X.X
-// commit: XXXX
-// This method returns the version 'vX.X.X' or empty if the version isn't found.
-func extractDriverVersion(s string) string {
-	versionRegex := regexp.MustCompile(`version:(.*)`)
-	matches := versionRegex.FindStringSubmatch(s)
-
-	if len(matches) != 2 {
-		return ""
-	}
-
-	v := strings.TrimSpace(matches[1])
-	return strings.TrimPrefix(v, "v")
-}
-
 func driverExists(driverName string) bool {
 	_, err := exec.LookPath(driverName)
 	return err == nil

pkg/minikube/driver/auxdriver/install_test.go

@@ -0,0 +1,60 @@
+/*
+Copyright 2020 The Kubernetes Authors All rights reserved.
+
+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 main
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+const version = "v1.2.3"
+const commit = "1af8bdc072232de4b1fec3b6cc0e8337e118bc83"
+
+func main() {
+	if len(os.Args) != 2 {
+		fmt.Fprintf(os.Stderr, "no command specified\n")
+		os.Exit(1)
+	}
+
+	if os.Args[1] != "version" {
+		fmt.Fprintf(os.Stderr, "unknown command %q\n", os.Args[1])
+		os.Exit(1)
+	}
+
+	// We use a single executable to emulate different driver outputs.
+	driverPersonality := strings.TrimSuffix(filepath.Base(os.Args[0]), ".exe")
+
+	switch driverPersonality {
+	case "valid":
+		fmt.Printf("version: %s\n", version)
+		fmt.Printf("commit: %s\n", commit)
+	case "no-version":
+		fmt.Printf("commit: %s\n", commit)
+	case "no-commit":
+		fmt.Printf("version: %s\n", version)
+	case "invalid":
+		fmt.Println("invalid yaml")
+	case "fail":
+		fmt.Fprintf(os.Stderr, "no version for you!\n")
+		os.Exit(1)
+	default:
+		fmt.Fprintf(os.Stderr, "unknown personality %q\n", driverPersonality)
+		os.Exit(1)
+	}
+}

pkg/minikube/driver/auxdriver/testdata/driver.go

@@ -17,16 +17,28 @@ limitations under the License.
 package auxdriver
 
 import (
+	"fmt"
+	"os/exec"
+
 	"github.com/blang/semver/v4"
 	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/minikube/driver"
+	"sigs.k8s.io/yaml"
 )
 
 // minHyperkitVersion is the minimum version of the minikube hyperkit driver compatible with the current minikube code
 var minHyperkitVersion *semver.Version
 
 const minHyperkitVersionStr = "1.11.0"
 
+// Version is auxiliary driver version info.
+type Version struct {
+	// Version is driver version in vX.Y.Z format.
+	Version string `json:"version"`
+	// Commit is the git commit hash the driver was built from.
+	Commit string `json:"commit"`
+}
+
 func init() {
 	v, err := semver.New(minHyperkitVersionStr)
 	if err != nil {
@@ -51,3 +63,34 @@ func minAcceptableDriverVersion(driverName string, mkVer semver.Version) semver.
 		return mkVer
 	}
 }
+
+// driverVersion returns auxiliary driver version.
+func driverVersion(path string) (Version, error) {
+	v := Version{}
+
+	cmd := exec.Command(path, "version")
+	output, err := cmd.Output()
+	if err != nil {
+		var stderr []byte
+		if ee, ok := err.(*exec.ExitError); ok {
+			stderr = ee.Stderr
+		}
+		return v, fmt.Errorf("command %s failed: %v: %s", cmd, err, stderr)
+	}
+
+	if err := yaml.Unmarshal(output, &v); err != nil {
+		return v, fmt.Errorf("invalid driver version: %q: %v", output, err)
+	}
+
+	// Version is required to validate the driver in runtime.
+	if v.Version == "" {
+		return v, fmt.Errorf("version not specified: %+v", v)
+	}
+
+	// Commit is required to validate the driver during tests.
+	if v.Commit == "" {
+		return v, fmt.Errorf("commit not specified: %+v", v)
+	}
+
+	return v, nil
+}

pkg/minikube/driver/auxdriver/version.go

@@ -17,6 +17,10 @@ limitations under the License.
 package auxdriver
 
 import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/blang/semver/v4"
@@ -50,3 +54,80 @@ func semanticVersion(s string) semver.Version {
 	}
 	return *r
 }
+
+func TestDriverVersion(t *testing.T) {
+	t.Run("valid", func(t *testing.T) {
+		path := buildTestDriver(t, "valid")
+		v, err := driverVersion(path)
+		if err != nil {
+			t.Fatalf("failed to get driver version: %s", err)
+		}
+		expected := Version{Version: "v1.2.3", Commit: "1af8bdc072232de4b1fec3b6cc0e8337e118bc83"}
+		if v != expected {
+			t.Errorf("Invalid driver version, got: %v, want: %v", v, expected)
+		}
+	})
+
+	t.Run("no version", func(t *testing.T) {
+		path := buildTestDriver(t, "no-version")
+		if _, err := driverVersion(path); err == nil {
+			t.Fatalf("missing version did not fail")
+		} else {
+			t.Logf("expected error: %v", err)
+		}
+	})
+
+	t.Run("no commit", func(t *testing.T) {
+		path := buildTestDriver(t, "no-commit")
+		if _, err := driverVersion(path); err == nil {
+			t.Fatalf("missing commit did not fail")
+		} else {
+			t.Logf("expected error: %v", err)
+		}
+	})
+
+	t.Run("invalid", func(t *testing.T) {
+		path := buildTestDriver(t, "invalid")
+		if _, err := driverVersion(path); err == nil {
+			t.Fatalf("invalid yaml did not fail")
+		} else {
+			t.Logf("expected error: %v", err)
+		}
+	})
+
+	t.Run("fail", func(t *testing.T) {
+		path := buildTestDriver(t, "fail")
+		if _, err := driverVersion(path); err == nil {
+			t.Fatalf("failing driver did not fail")
+		} else {
+			t.Logf("expected error: %v", err)
+		}
+	})
+
+	t.Run("missing", func(t *testing.T) {
+		path := filepath.Join(t.TempDir(), "driver.exe")
+		if _, err := driverVersion(path); err == nil {
+			t.Fatalf("missing driver did not fail")
+		} else {
+			t.Logf("expected error: %v", err)
+		}
+	})
+}
+
+func buildTestDriver(t *testing.T, name string) string {
+	out := filepath.Join(t.TempDir(), name)
+	if runtime.GOOS == "windows" {
+		out += ".exe"
+	}
+
+	cmd := exec.Command("go", "build", "-o", out, "testdata/driver.go")
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+
+	t.Logf("Building %q", out)
+	if err := cmd.Run(); err != nil {
+		t.Fatal(err)
+	}
+
+	return out
+}