From 5f2eff7dfa22f3c74816c17b1d2e6e0be3a93d39 Mon Sep 17 00:00:00 2001 From: Nahshon Unna-Tsameret Date: Sun, 24 Jan 2021 15:43:24 +0200 Subject: [PATCH] Add option to read the operator namespace from an envirnment variable, or from a non-standard SA file. While developing an operator, it is impossible to run from a local environment variable if the code is trying to access the namespace file. For example, calling the `conditions.NewCondition` function will return an error when the operator is running in a development environment. This PR add to options to override the hard-coded path to the namespace file with another location: 1. by setting the new `OPERATOR_NAMESPACE` environment variable to the required namespace. 2. by setting the new `SA_FILE_PATH` environment variable to the local path of the namespace file. If both new environment variables are set, the namespace will be taken from the `OPERATOR_NAMESPACE` environment variable. Fixes #50 Signed-off-by: Nahshon Unna-Tsameret --- internal/utils/testfiles/namespace | 1 + internal/utils/testfiles/namespaceWithSpaces | 3 + internal/utils/utils.go | 26 +++- internal/utils/utils_test.go | 121 ++++++++++++++++++- 4 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 internal/utils/testfiles/namespace create mode 100644 internal/utils/testfiles/namespaceWithSpaces diff --git a/internal/utils/testfiles/namespace b/internal/utils/testfiles/namespace new file mode 100644 index 0000000..634fc70 --- /dev/null +++ b/internal/utils/testfiles/namespace @@ -0,0 +1 @@ +testnamespace \ No newline at end of file diff --git a/internal/utils/testfiles/namespaceWithSpaces b/internal/utils/testfiles/namespaceWithSpaces new file mode 100644 index 0000000..010e363 --- /dev/null +++ b/internal/utils/testfiles/namespaceWithSpaces @@ -0,0 +1,3 @@ + testnamespace + + diff --git a/internal/utils/utils.go b/internal/utils/utils.go index ec1b4de..c4e20e4 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -21,17 +21,41 @@ import ( "strings" ) +const ( + // SAFileDefaultLocation default location of the service account namespace file + SAFileDefaultLocation = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" + + // SAFileLocationEnv is the name of the environment variable that holds the service + // account file location file. It is not set by default, but setting it allows operator + // developers set different file location, because the default path may not be accessible + // on the development environment. If not set, the default path will be used. + SAFileLocationEnv = "SA_FILE_PATH" + + // OperatorNamespaceEnv the name of the environm,ent variable that holds the namespace. + // If set, the GetOperatorNamespace method returns its value. If not, the method read the + // service account file. + OperatorNamespaceEnv = "OPERATOR_NAMESPACE" +) + // ErrNoNamespace indicates that a namespace could not be found for the current // environment var ErrNoNamespace = fmt.Errorf("namespace not found for current environment") var readSAFile = func() ([]byte, error) { - return ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") + saFileLocation, found := os.LookupEnv(SAFileLocationEnv) + if !found { + saFileLocation = SAFileDefaultLocation + } + return ioutil.ReadFile(saFileLocation) } // GetOperatorNamespace returns the namespace the operator should be running in from // the associated service account secret. var GetOperatorNamespace = func() (string, error) { + if ns := strings.TrimSpace(os.Getenv(OperatorNamespaceEnv)); ns != "" { + return ns, nil + } + nsBytes, err := readSAFile() if err != nil { if os.IsNotExist(err) { diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 04c5aae..42b05a0 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -16,6 +16,7 @@ package utils import ( "os" + "strings" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -23,6 +24,11 @@ import ( var _ = Describe("Helpers test", func() { Describe("GetOperatorNamespace", func() { + var origReadSAFile = readSAFile + AfterEach(func() { + readSAFile = origReadSAFile + }) + const testNamespace = "testnamespace" It("should return error when namespace not found", func() { readSAFile = func() ([]byte, error) { return nil, os.ErrNotExist @@ -33,24 +39,129 @@ var _ = Describe("Helpers test", func() { }) It("should return namespace", func() { readSAFile = func() ([]byte, error) { - return []byte("testnamespace"), nil + return []byte(testNamespace), nil } // test namespace, err := GetOperatorNamespace() Expect(err).Should(BeNil()) - Expect(namespace).To(Equal("testnamespace")) + Expect(namespace).To(Equal(testNamespace)) }) It("should trim whitespace from namespace", func() { readSAFile = func() ([]byte, error) { - return []byte(" testnamespace "), nil + return []byte(" " + testNamespace + " "), nil } // test namespace, err := GetOperatorNamespace() - Expect(err).Should(BeNil()) - Expect(namespace).To(Equal("testnamespace")) + Expect(err).ShouldNot(HaveOccurred()) + Expect(namespace).To(Equal(testNamespace)) + }) + Context("read namespace from environment variable", func() { + var originalVal string + JustBeforeEach(func() { + originalVal = os.Getenv(OperatorNamespaceEnv) + }) + JustAfterEach(func() { + err := os.Setenv(OperatorNamespaceEnv, originalVal) + Expect(err).ShouldNot(HaveOccurred()) + }) + It("should return the env var value, if set", func() { + err := os.Setenv(OperatorNamespaceEnv, testNamespace) + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).ShouldNot(HaveOccurred()) + Expect(namespace).To(Equal(testNamespace)) + }) + It("should trim spaces from the namespace", func() { + err := os.Setenv(OperatorNamespaceEnv, " "+testNamespace+" ") + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).ShouldNot(HaveOccurred()) + Expect(namespace).To(Equal(testNamespace)) + }) + It("should return the namespace from a file if not the env var is not set", func() { + readSAFile = func() ([]byte, error) { + return []byte("namespace-from-file"), nil + } + err := os.Unsetenv(OperatorNamespaceEnv) + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).Should(BeNil()) + Expect(namespace).To(Equal("namespace-from-file")) + + }) + It("should return the namespace from a file if not the env var is only spaces", func() { + readSAFile = func() ([]byte, error) { + return []byte("namespace-from-file"), nil + } + err := os.Setenv(OperatorNamespaceEnv, " ") + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).Should(BeNil()) + Expect(namespace).To(Equal("namespace-from-file")) + }) + }) + Context("read namespace from non standard location", func() { + var originalVal string + JustBeforeEach(func() { + originalVal = os.Getenv(SAFileLocationEnv) + }) + JustAfterEach(func() { + err := os.Setenv(SAFileLocationEnv, originalVal) + Expect(err).ShouldNot(HaveOccurred()) + }) + It("should return the env var value, if set", func() { + err := os.Setenv(SAFileLocationEnv, getTestFilesDir()+"namespace") + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).ShouldNot(HaveOccurred()) + Expect(namespace).To(Equal(testNamespace)) + }) + It("should trim spaces from the namespace", func() { + err := os.Setenv(SAFileLocationEnv, getTestFilesDir()+"namespaceWithSpaces") + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).ShouldNot(HaveOccurred()) + + Expect(namespace).To(Equal(testNamespace)) + }) + It("should return error if the file is not exists", func() { + err := os.Setenv(SAFileLocationEnv, getTestFilesDir()+"notExists") + Expect(err).ShouldNot(HaveOccurred()) + + namespace, err := GetOperatorNamespace() + Expect(err).Should(HaveOccurred()) + Expect(err).Should(Equal(ErrNoNamespace)) + + Expect(namespace).Should(BeEmpty()) + }) }) }) }) + +// return the path to the test files directory +func getTestFilesDir() string { + const ( + packageUnderTestPath = "internal/utils" + testFileDir = "/testfiles/" + ) + + wd, err := os.Getwd() + ExpectWithOffset(1, err).ShouldNot(HaveOccurred()) + + // if running form internal/utils/ + if strings.HasSuffix(wd, packageUnderTestPath) { + return wd + testFileDir + } + + // if running from repository root + return packageUnderTestPath + testFileDir +}