Merge pull request kubernetes-sigs#1513 from DirectXMan12/bug/quote-envtest-tool-path

🐛 Quote path with -p env in envtest-setup

Author: k8s-ci-robot

tools/setup-envtest/env/env.go

@@ -11,6 +11,7 @@ import (
 	"io/fs"
 	"path/filepath"
 	"sort"
+	"strings"
 	"text/tabwriter"
 
 	"github.com/go-logr/logr"
@@ -372,7 +373,14 @@ func (e *Env) PrintInfo(printFmt PrintFormat) {
 	case PrintPath:
 		fmt.Fprint(e.Out, path) // NB(directxman12): no newline -- want the bare path here
 	case PrintEnv:
-		fmt.Fprintf(e.Out, "export KUBEBUILDER_ASSETS=%s\n", path)
+		// quote in case there are spaces, etc in the path
+		// the weird string below works like this:
+		// - you can't escape quotes in shell
+		// - shell strings that are next to each other are concatenated (so "a""b""c" == "abc")
+		// - you can intermix quote styles using the above
+		// - so `'"'"'` --> CLOSE_QUOTE + "'" + OPEN_QUOTE
+		shellQuoted := strings.ReplaceAll(path, "'", `'"'"'`)
+		fmt.Fprintf(e.Out, "export KUBEBUILDER_ASSETS='%s'\n", shellQuoted)
 	default:
 		panic(fmt.Sprintf("unexpected print format %v", printFmt))
 	}

tools/setup-envtest/env/env_suite_test.go

@@ -0,0 +1,31 @@
+package env_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"github.com/go-logr/logr"
+	"github.com/go-logr/zapr"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
+)
+
+var testLog logr.Logger
+
+func zapLogger() logr.Logger {
+	testOut := zapcore.AddSync(GinkgoWriter)
+	enc := zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig())
+	// bleh setting up logging to the ginkgo writer is annoying
+	zapLog := zap.New(zapcore.NewCore(enc, testOut, zap.DebugLevel),
+		zap.ErrorOutput(testOut), zap.Development(), zap.AddStacktrace(zap.WarnLevel))
+	return zapr.NewLogger(zapLog)
+}
+
+func TestEnv(t *testing.T) {
+	testLog = zapLogger()
+
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Env Suite")
+}

tools/setup-envtest/env/env_test.go

@@ -0,0 +1,92 @@
+package env_test
+
+import (
+	"bytes"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/spf13/afero"
+
+	. "sigs.k8s.io/controller-runtime/tools/setup-envtest/env"
+	"sigs.k8s.io/controller-runtime/tools/setup-envtest/store"
+	"sigs.k8s.io/controller-runtime/tools/setup-envtest/versions"
+)
+
+var _ = Describe("Env", func() {
+	// Most of the rest of this is tested e2e via the workflows test,
+	// but there's a few things that are easier to test here.  Eventually
+	// we should maybe move some of the tests here.
+	var (
+		env       *Env
+		outBuffer *bytes.Buffer
+	)
+	BeforeEach(func() {
+		outBuffer = new(bytes.Buffer)
+		env = &Env{
+			Out: outBuffer,
+			Log: testLog,
+
+			Store: &store.Store{
+				// use spaces and quotes to test our quote escaping below
+				Root: afero.NewBasePathFs(afero.NewMemMapFs(), "/kb's test store"),
+			},
+
+			// shouldn't use these, but just in case
+			NoDownload: true,
+			FS:         afero.Afero{Fs: afero.NewMemMapFs()},
+		}
+
+		env.Version.MakeConcrete(versions.Concrete{
+			Major: 1, Minor: 21, Patch: 3,
+		})
+		env.Platform.Platform = versions.Platform{
+			OS: "linux", Arch: "amd64",
+		}
+	})
+
+	Describe("printing", func() {
+		It("should use a manual path if one is present", func() {
+			By("using a manual path")
+			Expect(env.PathMatches("/otherstore/1.21.4-linux-amd64")).To(BeTrue())
+
+			By("checking that that path is printed properly")
+			env.PrintInfo(PrintPath)
+			Expect(outBuffer.String()).To(Equal("/otherstore/1.21.4-linux-amd64"))
+		})
+
+		Context("as human-readable info", func() {
+			BeforeEach(func() {
+				env.PrintInfo(PrintOverview)
+			})
+
+			It("should contain the version", func() {
+				Expect(outBuffer.String()).To(ContainSubstring("/kb's test store/k8s/1.21.3-linux-amd64"))
+			})
+			It("should contain the path", func() {
+				Expect(outBuffer.String()).To(ContainSubstring("1.21.3"))
+			})
+			It("should contain the platform", func() {
+				Expect(outBuffer.String()).To(ContainSubstring("linux/amd64"))
+			})
+
+		})
+		Context("as just a path", func() {
+			It("should print out just the path", func() {
+				env.PrintInfo(PrintPath)
+				Expect(outBuffer.String()).To(Equal(`/kb's test store/k8s/1.21.3-linux-amd64`))
+			})
+		})
+
+		Context("as env vars", func() {
+			BeforeEach(func() {
+				env.PrintInfo(PrintEnv)
+			})
+			It("should set KUBEBUILDER_ASSETS", func() {
+				Expect(outBuffer.String()).To(HavePrefix("export KUBEBUILDER_ASSETS="))
+			})
+			It("should quote the return path, escaping quotes to deal with spaces, etc", func() {
+				Expect(outBuffer.String()).To(HaveSuffix(`='/kb'"'"'s test store/k8s/1.21.3-linux-amd64'` + "\n"))
+			})
+		})
+	})
+})