From d759a8a6c9a1c88f8bd6f75d4a2137c6104c5cfd Mon Sep 17 00:00:00 2001 From: Hakan Baba Date: Mon, 1 May 2017 15:21:04 -0700 Subject: [PATCH 1/4] Add line comment support for blacklist whitelist files. Includes tests and documentaiton too. --- README.md | 8 +++++ applylist/factory.go | 25 ++++++++++++++- applylist/factory_test.go | 67 +++++++++++++++++++++++++++++++++++++++ sysutil/filesystem.go | 3 +- 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3febbe3e..c957cfce 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,14 @@ We suggest running kube-applier as a Deployment (see [demo/](https://github.com/ set to `/git/repo`, and the file to be whitelisted is `/git/repo/apps/app1.json`, the line in the whiltelist file should be `apps/app1.json`). + +--- +**NOTE** +The blacklist and whiltelist files support line comments. +A single line gets ignored if the first non-blank character is # in that line. + +--- + * `POLL_INTERVAL_SECONDS` - (int) Number of seconds to wait between each check for new commits to the repo (default is 5). Set to 0 to disable the wait period. * `FULL_RUN_INTERVAL_SECONDS` - (int) Number of seconds between automatic full runs (default is 300, or 5 minutes). Set to 0 to disable the wait period. * `DIFF_URL_FORMAT` - (string) If specified, allows the status page to display a link to the source code referencing the diff for a specific commit. `DIFF_URL_FORMAT` should be a URL for a hosted remote repo that supports linking to a commit hash. Replace the commit hash portion with "%s" so it can be filled in by kube-applier (e.g. `https://github.com/kubernetes/kubernetes/commit/%s`). diff --git a/applylist/factory.go b/applylist/factory.go index 0c353538..41c3b520 100644 --- a/applylist/factory.go +++ b/applylist/factory.go @@ -36,6 +36,23 @@ func (f *Factory) Create() ([]string, []string, []string, error) { return applyList, blacklist, whitelist, nil } +// purgeCommentsFromList iterates over the list contents and deletes comment +// lines. A comment is a line whose first non-space character is # +func (f *Factory) purgeCommentsFromList(rawList []string) ([]string, error) { + + // http://stackoverflow.com/a/20551116/5771861 + i := 0 + for _, l := range rawList { + // # is the comment line + if len(l) > 0 && string(l[0]) != "#" { + rawList[i] = l + i++ + } + } + rv := rawList[:i] + return rv, nil +} + // createFilelist reads lines from the given file, converts the relative // paths to full paths, and returns a sorted list of full paths. func (f *Factory) createFileList(listFilePath string) ([]string, error) { @@ -46,7 +63,13 @@ func (f *Factory) createFileList(listFilePath string) ([]string, error) { if err != nil { return nil, err } - list := prependToEachPath(f.RepoPath, rawList) + + filteredList, err := f.purgeCommentsFromList(rawList) + if err != nil { + return nil, err + } + + list := prependToEachPath(f.RepoPath, filteredList) sort.Strings(list) return list, nil } diff --git a/applylist/factory_test.go b/applylist/factory_test.go index c36d039b..49949120 100644 --- a/applylist/factory_test.go +++ b/applylist/factory_test.go @@ -18,6 +18,63 @@ type testCase struct { expectedErr error } +// TestPurgeComments verifies the comment processing and purging from the +// whitelist and blacklist specifications. +func TestPurgeComments(t *testing.T) { + + var testData = []struct { + rawList []string + expectedReturn []string + }{ + // No comment + { + []string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + []string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + }, + // First line is commented + { + []string{"#/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + []string{"/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + }, + // Last line is commented + { + []string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "# /repo/c.json"}, + []string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c"}, + }, + // Empty line + { + []string{"/repo/a/b.json", "", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + []string{"/repo/a/b.json", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + }, + // Comment line only containing the comment character. + { + []string{"/repo/a/b.json", "#", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + []string{"/repo/a/b.json", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, + }, + // Empty file + { + []string{}, + []string{}, + }, + // File with only comment lines. + { + []string{"# some comment "}, + []string{}, + }, + } + + assert := assert.New(t) + mockCtrl := gomock.NewController(t) + fs := sysutil.NewMockFileSystemInterface(mockCtrl) + f := &Factory{"", "", "", fs} + for _, td := range testData { + + rv, err := f.purgeCommentsFromList(td.rawList) + assert.Equal(rv, td.expectedReturn) + assert.Equal(err, nil) + } +} + func TestFactoryCreate(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -128,6 +185,16 @@ func TestFactoryCreate(t *testing.T) { ) tc = testCase{"/repo", "/blacklist", "/whitelist", fs, []string{"/repo/c.json"}, []string{"/repo/a/b/c.yaml"}, nil} createAndAssert(t, tc) + + // Both whitelist and blacklist contain the same file and other comments. + gomock.InOrder( + fs.EXPECT().ReadLines("/blacklist").Times(1).Return([]string{"a/b/c.yaml", "# c.json"}, nil), + fs.EXPECT().ReadLines("/whitelist").Times(1).Return([]string{"a/b/c.yaml", "c.json", "# a/b/c.yaml"}, nil), + fs.EXPECT().ListAllFiles("/repo").Times(1).Return([]string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, nil), + ) + tc = testCase{"/repo", "/blacklist", "/whitelist", fs, []string{"/repo/c.json"}, []string{"/repo/a/b/c.yaml"}, nil} + createAndAssert(t, tc) + } func createAndAssert(t *testing.T, tc testCase) { diff --git a/sysutil/filesystem.go b/sysutil/filesystem.go index f07e92dc..ffe403c4 100644 --- a/sysutil/filesystem.go +++ b/sysutil/filesystem.go @@ -6,6 +6,7 @@ import ( "log" "os" "path/filepath" + "strings" "time" ) @@ -29,7 +30,7 @@ func (fs *FileSystem) ReadLines(filePath string) ([]string, error) { var result []string s := bufio.NewScanner(f) for s.Scan() { - result = append(result, s.Text()) + result = append(result, strings.TrimSpace(s.Text())) } if err := s.Err(); err != nil { return nil, fmt.Errorf("Error reading the file at %v: %v", filePath, err) From 6a27ce855123531400d6fea507e9e2904853de43 Mon Sep 17 00:00:00 2001 From: Hakan Baba Date: Fri, 28 Apr 2017 15:02:12 -0700 Subject: [PATCH 2/4] Do not write to /etc. A dir owned but root Instead we create a tmmp file and log about it. --- kube/client.go | 16 +++++++++------- main.go | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/kube/client.go b/kube/client.go index 8bcfff60..fceb1bba 100644 --- a/kube/client.go +++ b/kube/client.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/box/kube-applier/sysutil" "io/ioutil" - "os" + "log" "os/exec" "regexp" "strconv" @@ -17,9 +17,6 @@ const ( // Location of the kubeconfig template file within the container - see ADD command in Dockerfile kubeconfigTemplatePath = "/templates/kubeconfig" - - // Location of the written kubeconfig file within the container - kubeconfigFilePath = "/etc/kubeconfig" ) // ClientInterface allows for mocking out the functionality of Client when testing the full process of an apply run. @@ -32,6 +29,8 @@ type ClientInterface interface { // The Server field enables discovery of the API server when kube-proxy is not configured (see README.md for more information). type Client struct { Server string + // Location of the written kubeconfig file within the container + kubeconfigFilePath string } // Configure writes the kubeconfig file to be used for authenticating kubectl commands. @@ -41,7 +40,10 @@ func (c *Client) Configure() error { return nil } - f, err := os.Create(kubeconfigFilePath) + f, err := ioutil.TempFile("", "kubeConfig") + c.kubeconfigFilePath = f.Name() + log.Printf("Using kubeConfig file:", c.kubeconfigFilePath) + if err != nil { return fmt.Errorf("Error creating kubeconfig file: %v", err) } @@ -74,7 +76,7 @@ func (c *Client) Configure() error { func (c *Client) CheckVersion() error { args := []string{"kubectl", "version"} if c.Server != "" { - args = append(args, fmt.Sprintf("--kubeconfig=%s", kubeconfigFilePath)) + args = append(args, fmt.Sprintf("--kubeconfig=%s", c.kubeconfigFilePath)) } stdout, err := exec.Command(args[0], args[1:]...).CombinedOutput() output := strings.TrimSuffix(string(stdout), "\n") @@ -125,7 +127,7 @@ func isCompatible(clientMajor, clientMinor, serverMajor, serverMinor string) err func (c *Client) Apply(path string) (string, string, error) { args := []string{"kubectl", "apply", "-f", path} if c.Server != "" { - args = append(args, fmt.Sprintf("--kubeconfig=%s", kubeconfigFilePath)) + args = append(args, fmt.Sprintf("--kubeconfig=%s", c.kubeconfigFilePath)) } cmd := strings.Join(args, " ") stdout, err := exec.Command(args[0], args[1:]...).CombinedOutput() diff --git a/main.go b/main.go index fed16cff..b82f2116 100644 --- a/main.go +++ b/main.go @@ -52,7 +52,7 @@ func main() { log.Fatal(err) } - kubeClient := &kube.Client{server} + kubeClient := &kube.Client{Server: server} kubeClient.Configure() batchApplier := &run.BatchApplier{kubeClient, metrics} From 6e65cbcfc944ee6b605d69f894fb50faf3fae433 Mon Sep 17 00:00:00 2001 From: Hakan Baba Date: Fri, 28 Apr 2017 16:47:19 -0700 Subject: [PATCH 3/4] Use a temp directory for schema cache dir. The default path uses the home directory. Which may or may not exist for the current user. --- kube/client.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/kube/client.go b/kube/client.go index fceb1bba..39330ec7 100644 --- a/kube/client.go +++ b/kube/client.go @@ -31,6 +31,9 @@ type Client struct { Server string // Location of the written kubeconfig file within the container kubeconfigFilePath string + + // A directory with write permissions for kubectl to load store its schema cache + schemaCacheDir string } // Configure writes the kubeconfig file to be used for authenticating kubectl commands. @@ -41,13 +44,20 @@ func (c *Client) Configure() error { } f, err := ioutil.TempFile("", "kubeConfig") + if err != nil { + return fmt.Errorf("Error creating kubeconfig file: %v", err) + } + defer f.Close() + c.kubeconfigFilePath = f.Name() log.Printf("Using kubeConfig file:", c.kubeconfigFilePath) + scd, err := ioutil.TempDir("", "kubectl-schema-cache-dir") if err != nil { - return fmt.Errorf("Error creating kubeconfig file: %v", err) + log.Printf("Error creating kubectl schema cache dir: %v", err) + } else { + c.schemaCacheDir = scd } - defer f.Close() token, err := ioutil.ReadFile(tokenPath) if err != nil { @@ -125,7 +135,7 @@ func isCompatible(clientMajor, clientMinor, serverMajor, serverMinor string) err // Apply attempts to "kubectl apply" the file located at path. // It returns the full apply command and its output. func (c *Client) Apply(path string) (string, string, error) { - args := []string{"kubectl", "apply", "-f", path} + args := []string{"kubectl", "apply", "--schema-cache-dir", c.schemaCacheDir, "-f", path} if c.Server != "" { args = append(args, fmt.Sprintf("--kubeconfig=%s", c.kubeconfigFilePath)) } From 2fede9911ddcacbfe73ad13fefd3f00632f8cfec Mon Sep 17 00:00:00 2001 From: Hakan Baba Date: Wed, 3 May 2017 13:01:23 -0700 Subject: [PATCH 4/4] Return one value from purgeCommentsFromList function. --- applylist/factory.go | 9 +++------ applylist/factory_test.go | 3 +-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/applylist/factory.go b/applylist/factory.go index 41c3b520..756bb2a7 100644 --- a/applylist/factory.go +++ b/applylist/factory.go @@ -38,7 +38,7 @@ func (f *Factory) Create() ([]string, []string, []string, error) { // purgeCommentsFromList iterates over the list contents and deletes comment // lines. A comment is a line whose first non-space character is # -func (f *Factory) purgeCommentsFromList(rawList []string) ([]string, error) { +func (f *Factory) purgeCommentsFromList(rawList []string) []string { // http://stackoverflow.com/a/20551116/5771861 i := 0 @@ -50,7 +50,7 @@ func (f *Factory) purgeCommentsFromList(rawList []string) ([]string, error) { } } rv := rawList[:i] - return rv, nil + return rv } // createFilelist reads lines from the given file, converts the relative @@ -64,10 +64,7 @@ func (f *Factory) createFileList(listFilePath string) ([]string, error) { return nil, err } - filteredList, err := f.purgeCommentsFromList(rawList) - if err != nil { - return nil, err - } + filteredList := f.purgeCommentsFromList(rawList) list := prependToEachPath(f.RepoPath, filteredList) sort.Strings(list) diff --git a/applylist/factory_test.go b/applylist/factory_test.go index 49949120..00ae41b8 100644 --- a/applylist/factory_test.go +++ b/applylist/factory_test.go @@ -69,9 +69,8 @@ func TestPurgeComments(t *testing.T) { f := &Factory{"", "", "", fs} for _, td := range testData { - rv, err := f.purgeCommentsFromList(td.rawList) + rv := f.purgeCommentsFromList(td.rawList) assert.Equal(rv, td.expectedReturn) - assert.Equal(err, nil) } }