feat: Ignore lib directory while processing hooks (#621)

Signed-off-by: Evsyukov Denis <[email protected]>

.github/workflows/lint.yaml

@@ -36,7 +36,7 @@ jobs:
 
       - name: Run golangci-lint
         run: |
-          curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b . v1.54.2
+          curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b . v1.59.1
           ./golangci-lint run --sort-results --build-tags integration,test
 
 

docs/src/HOOKS.md

@@ -9,7 +9,7 @@ The hook receives the data and returns the result via files. Paths to files are
 At startup Shell-operator initializes the hooks:
 
 - The recursive search for hook files is performed in the hooks directory. You can specify it with `--hooks-dir` command-line argument or with the `SHELL_OPERATOR_HOOKS_DIR` environment variable (the default path is `/hooks`).
-  - Every executable file found in the path is considered a hook.
+  - Every executable file found in the path is considered a hook (please note, `lib` subdirectory ignored).
 - Found hooks are sorted alphabetically according to the directories’ and hooks’ names. Then they are executed with the `--config` flag to get bindings to events in YAML or JSON format.
 - If hook's configuration is successful, the working queue named "main" is filled with `onStartup` hooks.
 - Then, the "main" queue is filled with `kubernetes` hooks with `Synchronization` [binding context](#binding-context) type, so that each hook receives all existing objects described in hook's configuration.

pkg/hook/hook_manager.go

@@ -103,6 +103,10 @@ func (hm *hookManager) Init() error {
 	hm.hooksInOrder = make(map[BindingType][]*Hook)
 	hm.hooksByName = make(map[string]*Hook)
 
+	if err := utils_file.RecursiveCheckLibDirectory(hm.workingDir); err != nil {
+		log.Errorf("failed to check lib directory %s: %v", hm.workingDir, err)
+	}
+
 	hooksRelativePaths, err := utils_file.RecursiveGetExecutablePaths(hm.workingDir)
 	if err != nil {
 		return err

pkg/utils/file/dir.go

@@ -17,7 +17,7 @@ func RequireExistingDirectory(inDir string) (dir string, err error) {
 	if err != nil {
 		return "", fmt.Errorf("get absolute path: %v", err)
 	}
-	if exists, _ := DirExists(dir); !exists {
+	if exists := DirExists(dir); !exists {
 		return "", fmt.Errorf("path '%s' not exist", dir)
 	}
 
@@ -40,7 +40,7 @@ func EnsureTempDirectory(inDir string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("get absolute path: %v", err)
 	}
-	if exists, _ := DirExists(dir); !exists {
+	if exists := DirExists(dir); !exists {
 		err := os.Mkdir(dir, os.FileMode(0o777))
 		if err != nil {
 			return "", fmt.Errorf("create tmp dir '%s': %s", dir, err)
@@ -49,11 +49,13 @@ func EnsureTempDirectory(inDir string) (string, error) {
 	return dir, nil
 }
 
-func DirExists(path string) (bool, error) {
+// DirExists checking for directory existence
+// return bool value
+func DirExists(path string) bool {
 	fileInfo, err := os.Stat(path)
 	if err != nil && os.IsNotExist(err) {
-		return false, nil
+		return false
 	}
 
-	return fileInfo.IsDir(), nil
+	return fileInfo.IsDir()
 }

pkg/utils/file/file.go

@@ -34,8 +34,8 @@ func RecursiveGetExecutablePaths(dir string) ([]string, error) {
 		}
 
 		if f.IsDir() {
-			// Skip hidden directories inside initial directory
-			if strings.HasPrefix(f.Name(), ".") {
+			// Skip hidden and lib directories inside initial directory
+			if strings.HasPrefix(f.Name(), ".") || f.Name() == "lib" {
 				return filepath.SkipDir
 			}
 
@@ -55,6 +55,7 @@ func RecursiveGetExecutablePaths(dir string) ([]string, error) {
 
 		if !IsFileExecutable(f) {
 			log.Warnf("File '%s' is skipped: no executable permissions, chmod +x is required to run this hook", path)
+
 			return nil
 		}
 
@@ -67,3 +68,52 @@ func RecursiveGetExecutablePaths(dir string) ([]string, error) {
 
 	return paths, nil
 }
+
+// RecursiveCheckLibDirectory finds recursively all executable files
+// inside a lib directory. And will log warning with these files.
+func RecursiveCheckLibDirectory(dir string) error {
+	dir = filepath.Join(dir, "lib")
+	if exist := DirExists(dir); !exist {
+		return nil
+	}
+
+	err := filepath.Walk(dir, func(path string, f os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if f.IsDir() {
+			// Skip hidden directory inside initial directory
+			if strings.HasPrefix(f.Name(), ".") {
+				return filepath.SkipDir
+			}
+
+			return nil
+		}
+
+		// ignore hidden files
+		if strings.HasPrefix(f.Name(), ".") {
+			return nil
+		}
+
+		// ignore .yaml, .json, .txt, .md files
+		switch filepath.Ext(f.Name()) {
+		case ".yaml", ".json", ".md", ".txt":
+			return nil
+		}
+
+		if IsFileExecutable(f) {
+			log.Warnf("File '%s' has executable permissions and is located in the ignored 'lib' directory", strings.TrimPrefix(path, dir))
+
+			return nil
+		}
+
+		return nil
+	})
+
+	if err != nil {
+		return err
+	}
+
+	return nil
+}

pkg/utils/file/file_test.go

@@ -0,0 +1,143 @@
+package utils
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	log "github.com/sirupsen/logrus"
+)
+
+func prepareTestDirTree() (string, error) {
+	tmpDir, err := os.MkdirTemp("", "")
+	if err != nil {
+		return "", fmt.Errorf("error creating temp directory: %v\n", err)
+	}
+
+	if err = os.MkdirAll(filepath.Join(tmpDir, "aa"), 0755); err != nil {
+		os.RemoveAll(tmpDir)
+		return "", err
+	}
+
+	if err = os.MkdirAll(filepath.Join(tmpDir, "lib"), 0755); err != nil {
+		os.RemoveAll(tmpDir)
+		return "", err
+	}
+
+	if err = createExecutableFile(filepath.Join(tmpDir, "aa/exec.py")); err != nil {
+		os.RemoveAll(tmpDir)
+		return "", err
+	}
+
+	if err = createExecutableFile(filepath.Join(tmpDir, "check.py")); err != nil {
+		os.RemoveAll(tmpDir)
+		return "", err
+	}
+
+	if err = createExecutableFile(filepath.Join(filepath.Join(tmpDir, "lib"), "lib.py")); err != nil {
+		os.RemoveAll(tmpDir)
+		return "", err
+	}
+
+	return tmpDir, nil
+}
+
+func createExecutableFile(file string) error {
+	if _, err := os.Create(file); err != nil {
+		return err
+	}
+	os.Chmod(file, 0777)
+
+	return nil
+}
+
+func TestRecursiveGetExecutablePaths(t *testing.T) {
+	dir, err := prepareTestDirTree()
+	if err != nil {
+		t.Fatalf("error creating temp directory: %v\n", err)
+	}
+	type args struct {
+		dir string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []string
+		wantErr bool
+	}{
+		{
+			name: "get executable files",
+			args: args{
+				dir: dir,
+			},
+			want:    []string{"aa/exec.py", "check.py"},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := RecursiveGetExecutablePaths(tt.args.dir)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("RecursiveGetExecutablePaths() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			for i := range got {
+				if !strings.HasSuffix(got[i], tt.want[i]) {
+					t.Errorf("RecursiveGetExecutablePaths() got = %v, want %v", got, tt.want)
+				}
+			}
+		})
+	}
+
+	os.RemoveAll(dir)
+}
+
+func TestRecursiveCheckLibDirectory(t *testing.T) {
+	dir, err := prepareTestDirTree()
+	if err != nil {
+		t.Fatalf("error creating temp directory: %v\n", err)
+	}
+
+	type args struct {
+		dir string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "check lib directory",
+			args: args{
+				dir: dir,
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		var buf bytes.Buffer
+		log.SetOutput(&buf)
+
+		formatter := new(log.TextFormatter)
+		formatter.DisableColors = true
+		formatter.DisableTimestamp = true
+		log.SetFormatter(formatter)
+
+		t.Run(tt.name, func(t *testing.T) {
+			if err := RecursiveCheckLibDirectory(tt.args.dir); (err != nil) != tt.wantErr {
+				t.Errorf("RecursiveCheckLibDirectory() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			if strings.Compare(
+				strings.TrimSpace(buf.String()),
+				`level=warning msg="File '/lib.py' has executable permissions and is located in the ignored 'lib' directory"`) != 0 {
+				t.Errorf("RecursiveCheckLibDirectory() error, got `%v`", buf.String())
+			}
+		})
+	}
+
+	os.RemoveAll(dir)
+}

test/utils/directory.go

@@ -33,7 +33,7 @@ func ChooseExistedDirectoryPath(paths ...string) (path string, err error) {
 		return
 	}
 
-	if exists, _ := utils_file.DirExists(path); !exists {
+	if exists := utils_file.DirExists(path); !exists {
 		return "", fmt.Errorf("no working dir")
 	}