From bfc6236e7ed29e1487619c90a90b2030cf35c485 Mon Sep 17 00:00:00 2001 From: Diego Alvarez Date: Mon, 18 Dec 2023 17:08:25 -0500 Subject: [PATCH 1/3] Add more linters and fix the new issues - bump used go to 1.21 --- .golangci.yaml | 47 ++++++++++++++++++++++++++++++++++++++++--- Makefile | 5 +---- go.mod | 2 +- go.sum | 8 ++++++++ hashing.go | 12 +++++------ pkg/helm/helm.go | 17 +++++++++------- pkg/helm/helm_test.go | 2 +- 7 files changed, 71 insertions(+), 22 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index d0dc07b..7f15387 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -2,25 +2,66 @@ run: allow-parallel-runners: true timeout: 5m - go: '1.20' + go: "1.21" + tests: true linters: + enable-all: false + disable-all: true enable: - - errcheck + - depguard - errorlint + - errcheck - exportloopref + - gochecknoglobals - gocritic + - gocyclo - gofmt - goimports + - gomodguard - gosec + - gosimple - govet + - ineffassign + - lll - misspell + - nakedret + - nolintlint - revive - staticcheck - tenv + - typecheck - unconvert - - unused - unparam + - unused + +linters-settings: + gocritic: + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + enabled-tags: + - diagnostic + - opinionated + - style + disabled-checks: + - paramTypeCombine + - unnamedResult + - whyNoLint + gomodguard: + blocked: + modules: + - github.com/pkg/errors: + recommendations: + - errors + - fmt + lll: + line-length: 150 + depguard: + rules: + all: + deny: + - pkg: io/ioutil + desc: "io/ioutil package has been deprecated" issues: # Excluding configuration per-path, per-linter, per-text and per-source diff --git a/Makefile b/Makefile index f500f61..070c85b 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,5 @@ test: - go test ./... - -test-verbose: - go test -v ./... + go test -v -cover -short ./... benchmark: go test -bench=. diff --git a/go.mod b/go.mod index 03ce950..f4a1d5a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/1debit/mani-diffy -go 1.20 +go 1.21 require ( github.com/argoproj/argo-cd/v2 v2.6.15 diff --git a/go.sum b/go.sum index 7e3b9b4..fe08398 100644 --- a/go.sum +++ b/go.sum @@ -91,7 +91,9 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a h1:HbKu58rmZpUGpz5+4FfNmIU+FmZg2P3Xaj2v2bfNWmk= +github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc= github.com/alicebob/miniredis/v2 v2.23.1 h1:jR6wZggBxwWygeXcdNyguCOCIjPsZyNUNlAkTx2fu0U= +github.com/alicebob/miniredis/v2 v2.23.1/go.mod h1:84TWKZlxYkfgMucPBf5SOQBYJceZeQRFIaQgNMiCX6Q= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= @@ -263,6 +265,7 @@ github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.5.1 h1:mZcQUHVQUQWoPXXtuf9yuEXKudkV2sx1E06UadKWpgI= +github.com/fsnotify/fsnotify v1.5.1/go.mod h1:T3375wBYaZdLLcVNkcVbzGHY7f1l/uK5T5Ai1i3InKU= github.com/fvbommel/sortorder v1.0.1 h1:dSnXLt4mJYH25uDDGa3biZNQsozaUWDSWeKJ0qqFfzE= github.com/fvbommel/sortorder v1.0.1/go.mod h1:uk88iVf1ovNn1iLfgUVU2F9o5eO30ui720w+kxuqRs0= github.com/getkin/kin-openapi v0.76.0/go.mod h1:660oXbgy5JFMKreazJaQTw7o+X00qeSyhcnluiMv+Xg= @@ -638,13 +641,16 @@ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108 github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= +github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/ginkgo/v2 v2.1.4 h1:GNapqRSid3zijZ9H77KrgVG4/8KqiyRsxcSxe+7ApXY= +github.com/onsi/ginkgo/v2 v2.1.4/go.mod h1:um6tUpWM/cxCK3/FK8BXqEiUMUwRgSM4JXG47RKZmLU= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.15.0/go.mod h1:cIuvLEne0aoVhAgh/O6ac0Op8WWw9H6eYCriF+tEHG0= github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= +github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= @@ -833,6 +839,7 @@ github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1 github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/yuin/gopher-lua v0.0.0-20220504180219-658193537a64 h1:5mLPGnFdSsevFRFc9q3yYbBkB6tsm4aCwwQV/j1JQAQ= +github.com/yuin/gopher-lua v0.0.0-20220504180219-658193537a64/go.mod h1:GBR0iDaNXjAgGg9zfCvksxSRnQx76gclCIb7kdAd1Pw= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.6/go.mod h1:qXsaaIqmgQH0T+OPdb99Bf+PKfBBQVAdyD6TY9G8XM4= @@ -1403,6 +1410,7 @@ gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24 gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/square/go-jose.v2 v2.6.0 h1:NGk74WTnPKBNUhNzQX7PYcTLUjoq7mzKk2OKbvwk2iI= +gopkg.in/square/go-jose.v2 v2.6.0/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/warnings.v0 v0.1.1/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI= diff --git a/hashing.go b/hashing.go index 65cee43..c6a2ebf 100644 --- a/hashing.go +++ b/hashing.go @@ -80,7 +80,7 @@ func (s *JSONHashStore) Save() error { return err } - return os.WriteFile(s.path, b, 0644) + return os.WriteFile(s.path, b, 0o644) } type ChartHash struct { @@ -114,24 +114,24 @@ func (s *SumFileStore) Add(name, hash string) error { return nil } - return os.WriteFile(s.filepath(name), data, 0664) + return os.WriteFile(s.filepath(name), data, 0o664) } func (s *SumFileStore) Get(name string) (string, error) { - filepath := s.filepath(name) + fpath := s.filepath(name) - yfile, err := os.ReadFile(filepath) + yfile, err := os.ReadFile(fpath) if err != nil { if errors.Is(err, os.ErrNotExist) { // This is fine to do since there are cases where there won't be a hash. e.g. root return "", nil } - return "", fmt.Errorf("error reading file hash from %s error: %w", filepath, err) + return "", fmt.Errorf("error reading file hash from %s error: %w", fpath, err) } ch := ChartHash{} err2 := yaml.Unmarshal(yfile, &ch) if err2 != nil { - return "", fmt.Errorf("error unmarshaling hash %s error: %w", filepath, err2) + return "", fmt.Errorf("error unmarshaling hash %s error: %w", fpath, err2) } return ch.Hash, nil } diff --git a/pkg/helm/helm.go b/pkg/helm/helm.go index 7cf6b99..0b748e7 100644 --- a/pkg/helm/helm.go +++ b/pkg/helm/helm.go @@ -103,14 +103,14 @@ func installDependencies(chartDirectory string) error { func template(helmInfo *v1alpha1.Application, skipRenderKey string, ignoreValueFile string) ([]byte, error) { chartPath := strings.Split(helmInfo.Spec.Source.Path, "/") - chart := fmt.Sprint("../" + chartPath[len(chartPath)-1]) + chart := "../" + chartPath[len(chartPath)-1] setValues, fileValues := buildParams(helmInfo, ignoreValueFile) tmpFile := "" if helmInfo.Spec.Source.Helm.Values != "" { dataFile, err := createTempFile(helmInfo.Spec.Source.Helm.Values) - defer os.Remove(dataFile) + defer removeFile(dataFile) if err != nil { log.Println(err) } @@ -166,10 +166,16 @@ func writeToFile(manifest []byte, location string) error { "manifest.yaml", ), manifest, - 0664, + 0o664, ) } +func removeFile(filePath string) { + if err := os.Remove(filePath); err != nil { + log.Printf("Failed to remove file %s: %v", filePath, err) + } +} + func EmptyManifest(manifest string) (bool, error) { fileInfo, err := os.Stat(manifest) if err != nil { @@ -208,7 +214,7 @@ func GenerateHash(crd *v1alpha1.Application, ignoreValueFile string) (string, er if crd.Spec.Source.Helm != nil && len(crd.Spec.Source.Helm.ValueFiles) > 0 { oHash := sha256.New() overrideFiles := crd.Spec.Source.Helm.ValueFiles - matchDots := regexp.MustCompile(`\.\.\/`) + matchDots := regexp.MustCompile(`\.\./`) for i := 0; i < len(overrideFiles); i++ { if ignoreValueFile == "" || !strings.Contains(overrideFiles[i], ignoreValueFile) { trimmedFilename := matchDots.ReplaceAllString(overrideFiles[i], "") @@ -248,7 +254,6 @@ func generalHashFunction(dirFilepath string) ([]byte, error) { } fmt.Fprintf(hash, "%x %s\n", m[path], path) } - // log.Printf("FINAL HASH: %v\n", hex.EncodeToString(hash.Sum(nil))) return hash.Sum(nil), nil } @@ -406,7 +411,6 @@ func Read(inputCRD string) ([]*v1alpha1.Application, error) { crdSpecs := make([]*v1alpha1.Application, 0) yamlFile, err := os.ReadFile(inputCRD) if err != nil { - // log.Fatalf("Error reading crd: %s %v", inputCRD, err) return crdSpecs, fmt.Errorf("error reading crd: %s %w", inputCRD, err) } @@ -417,7 +421,6 @@ func Read(inputCRD string) ([]*v1alpha1.Application, error) { if errors.Is(err, io.EOF) { break } - // panic(fmt.Errorf("document decode failed: %w", err)) return crdSpecs, fmt.Errorf("document decode failed: %w", err) } crdSpecs = append(crdSpecs, &app) diff --git a/pkg/helm/helm_test.go b/pkg/helm/helm_test.go index 9d3d6d1..bd579e6 100644 --- a/pkg/helm/helm_test.go +++ b/pkg/helm/helm_test.go @@ -122,7 +122,7 @@ metadata: } }) - defer os.Remove(fileName) + defer removeFile(fileName) t.Run("Verify the file is cleaned up", func(t *testing.T) { _, err = os.Stat(fileName) From 8ab03806f97b5135927f443788349b687e191094 Mon Sep 17 00:00:00 2001 From: Diego Alvarez Date: Mon, 18 Dec 2023 17:09:17 -0500 Subject: [PATCH 2/3] Refactor walk due to its complexity. --- main.go | 140 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 78 insertions(+), 62 deletions(-) diff --git a/main.go b/main.go index 80a998a..10cf60e 100644 --- a/main.go +++ b/main.go @@ -42,6 +42,8 @@ type Walker struct { ignoreSuffix string } +type HashStores map[string]func(string, string) (HashStore, error) + // Walk walks a directory tree looking for Argo applications and renders them func (w *Walker) Walk(inputPath, outputPath string, maxDepth int, hashes HashStore) error { visited := make(map[string]bool) @@ -85,73 +87,87 @@ func pruneUnvisited(visited map[string]bool, outputPath string) error { } func (w *Walker) walk(inputPath, outputPath string, depth, maxDepth int, visited map[string]bool, hashes HashStore) error { - if maxDepth != InfiniteDepth { - // If we've reached the max depth, stop walking - if depth > maxDepth { - return nil - } + if maxDepth != InfiniteDepth && depth > maxDepth { + return nil } log.Println("Dropping into", inputPath) - - fi, err := os.ReadDir(inputPath) + files, err := os.ReadDir(inputPath) if err != nil { return err } - for _, file := range fi { + + for _, file := range files { if !strings.Contains(file.Name(), ".yaml") { continue } - - crds, err := helm.Read(filepath.Join(inputPath, file.Name())) - if err != nil { + if err := w.processFile(file, inputPath, outputPath, depth, maxDepth, visited, hashes); err != nil { return err } - for _, crd := range crds { - if crd.Kind != "Application" { - continue - } - - if strings.HasSuffix(crd.ObjectMeta.Name, w.ignoreSuffix) { - continue - } - - path := filepath.Join(outputPath, crd.ObjectMeta.Name) - visited[path] = true - - hash, err := hashes.Get(crd.ObjectMeta.Name) - // COMPARE HASHES HERE. STEP INTO RENDER IF NO MATCH - if err != nil { - return err - } - hashGenerated, err := w.GenerateHash(crd) - if err != nil { - return err - } - - emptyManifest, err := helm.EmptyManifest(filepath.Join(path, "manifest.yaml")) - if err != nil { - return err - } - if hashGenerated != hash || emptyManifest { - log.Printf("No match detected. Render: %s\n", crd.ObjectMeta.Name) - if err := w.Render(crd, path); err != nil { - return err - } - - if err := hashes.Add(crd.ObjectMeta.Name, hashGenerated); err != nil { - return err - } - } - - if err := w.walk(path, outputPath, depth+1, maxDepth, visited, hashes); err != nil { - return err - } + } + + return nil +} + +func (w *Walker) processFile(file os.DirEntry, inputPath, outputPath string, depth, maxDepth int, visited map[string]bool, hashes HashStore) error { + apps, err := helm.Read(filepath.Join(inputPath, file.Name())) + if err != nil { + return err + } + + for _, app := range apps { + if err := w.processApps(app, outputPath, depth, maxDepth, visited, hashes); err != nil { + return err } } + return nil } +func (w *Walker) processApps(app *v1alpha1.Application, outputPath string, depth, maxDepth int, visited map[string]bool, hashes HashStore) error { + if app.Kind != "Application" || strings.HasSuffix(app.ObjectMeta.Name, w.ignoreSuffix) { + return nil + } + + path := filepath.Join(outputPath, app.ObjectMeta.Name) + visited[path] = true + + emptyManifest, err := helm.EmptyManifest(filepath.Join(path, "manifest.yaml")) + if err != nil { + return err + } + + existingHash, appHash, err := w.generateHashes(app, hashes) + if err != nil { + return err + } + + if appHash != existingHash || emptyManifest { + if err := w.renderAndUpdateHashes(app, path, appHash, hashes); err != nil { + return err + } + } + + return w.walk(path, outputPath, depth+1, maxDepth, visited, hashes) +} + +func (w *Walker) generateHashes(app *v1alpha1.Application, hashes HashStore) (string, string, error) { + existingHash, err := hashes.Get(app.ObjectMeta.Name) + if err != nil { + return "", "", err + } + generatedHash, err := w.GenerateHash(app) + return existingHash, generatedHash, err +} + +func (w *Walker) renderAndUpdateHashes(app *v1alpha1.Application, path, generatedHash string, hashes HashStore) error { + log.Printf("No match detected. Render: %s\n", app.ObjectMeta.Name) + if err := w.Render(app, path); err != nil { + return err + } + return hashes.Add(app.ObjectMeta.Name, generatedHash) +} + func (w *Walker) Render(application *v1alpha1.Application, output string) error { log.Println("Render", application.ObjectMeta.Name) @@ -218,7 +234,16 @@ func main() { log.Fatal(err) } - h, err := getHashStore(*hashStore, *hashStrategy, *renderDir) + hashStores := HashStores{ + "sumfile": func(outputPath, hashStrategy string) (HashStore, error) { //nolint:unparam + return NewSumFileStore(outputPath, hashStrategy), nil + }, + "json": func(outputPath, hashStrategy string) (HashStore, error) { + return NewJSONHashStore(filepath.Join(outputPath, "hashes.json"), hashStrategy) + }, + } + + h, err := getHashStore(hashStores, *hashStore, *hashStrategy, *renderDir) if err != nil { log.Fatal(err) } @@ -244,16 +269,7 @@ func main() { log.Printf("mani-diffy took %v to run", time.Since(start)) } -var hashStores = map[string]func(string, string) (HashStore, error){ - "sumfile": func(outputPath, hashStrategy string) (HashStore, error) { //nolint:unparam - return NewSumFileStore(outputPath, hashStrategy), nil - }, - "json": func(outputPath, hashStrategy string) (HashStore, error) { - return NewJSONHashStore(filepath.Join(outputPath, "hashes.json"), hashStrategy) - }, -} - -func getHashStore(hashStore, hashStrategy, outputPath string) (HashStore, error) { +func getHashStore(hashStores HashStores, hashStore, hashStrategy, outputPath string) (HashStore, error) { if fn, ok := hashStores[hashStore]; ok { return fn(outputPath, hashStrategy) } From edb0f67d5ee8d7285ffb75300e557f3e7f5310bc Mon Sep 17 00:00:00 2001 From: Diego Alvarez Date: Mon, 18 Dec 2023 17:16:43 -0500 Subject: [PATCH 3/3] add coverage task --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 070c85b..f8ec08d 100644 --- a/Makefile +++ b/Makefile @@ -12,3 +12,6 @@ lint: build-binaries: go build -o mani-diffy + +coverage: + go test -v -coverprofile=coverage.out -short ./... && go tool cover -html=coverage.out