diff --git a/pkg/iac/scanners/helm/parser/option.go b/pkg/iac/scanners/helm/parser/option.go index 0f2eae3cc291..27371fd34fa8 100644 --- a/pkg/iac/scanners/helm/parser/option.go +++ b/pkg/iac/scanners/helm/parser/option.go @@ -13,25 +13,25 @@ type Option func(p *Parser) func OptionWithValuesFile(paths ...string) Option { return func(p *Parser) { - p.valuesFiles = paths + p.valueOpts.ValueFiles = paths } } func OptionWithValues(values ...string) Option { return func(p *Parser) { - p.values = values + p.valueOpts.Values = values } } func OptionWithFileValues(values ...string) Option { return func(p *Parser) { - p.fileValues = values + p.valueOpts.FileValues = values } } func OptionWithStringValues(values ...string) Option { return func(p *Parser) { - p.stringValues = values + p.valueOpts.StringValues = values } } diff --git a/pkg/iac/scanners/helm/parser/parser.go b/pkg/iac/scanners/helm/parser/parser.go index cbb73780c0ef..4e17d3cd9b4b 100644 --- a/pkg/iac/scanners/helm/parser/parser.go +++ b/pkg/iac/scanners/helm/parser/parser.go @@ -1,10 +1,12 @@ package parser import ( - "bytes" + "archive/tar" + "compress/gzip" "context" "errors" "fmt" + "io" "io/fs" "path" "path/filepath" @@ -13,7 +15,7 @@ import ( "strings" "github.com/google/uuid" - "gopkg.in/yaml.v3" + "github.com/samber/lo" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" @@ -28,26 +30,22 @@ import ( var manifestNameRegex = regexp.MustCompile("# Source: [^/]+/(.+)") type Parser struct { - logger *log.Logger - helmClient *action.Install - rootPath string - ChartSource string - filepaths []string - workingFS fs.FS - valuesFiles []string - values []string - fileValues []string - stringValues []string - apiVersions []string - kubeVersion string + logger *log.Logger + helmClient *action.Install + valueOpts ValueOptions + apiVersions []string + kubeVersion string + + vals map[string]any } type ChartFile struct { - TemplateFilePath string - ManifestContent string + ChartPath string + Path string + Content string } -func New(src string, opts ...Option) (*Parser, error) { +func New(opts ...Option) (*Parser, error) { client := action.NewInstall(&action.Configuration{}) client.DryRun = true // don't do anything @@ -55,9 +53,8 @@ func New(src string, opts ...Option) (*Parser, error) { client.ClientOnly = true // don't try to talk to a cluster p := &Parser{ - helmClient: client, - ChartSource: src, - logger: log.WithPrefix("helm parser"), + helmClient: client, + logger: log.WithPrefix("helm parser"), } for _, option := range opts { @@ -77,142 +74,124 @@ func New(src string, opts ...Option) (*Parser, error) { p.helmClient.KubeVersion = kubeVersion } + vals, err := p.valueOpts.MergeValues() + if err != nil { + return nil, err + } + p.vals = vals return p, nil } -func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, target string) error { - p.workingFS = fsys +type Chart struct { + path string + *chart.Chart +} + +func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, root string) ([]ChartFile, error) { - if err := fs.WalkDir(p.workingFS, filepath.ToSlash(target), func(filePath string, entry fs.DirEntry, err error) error { - select { - case <-ctx.Done(): - return ctx.Err() - default: + charts, err := p.collectCharts(fsys, root) + if err != nil { + return nil, err + } + + var files []ChartFile + for _, c := range charts { + chartFiles, err := p.renderChart(c) + if err != nil { + p.logger.Error("Failed to render chart", + log.String("name", c.Name()), log.FilePath(c.path), log.Err(err)) + continue } + files = append(files, chartFiles...) + } + + return files, nil +} + +func (p *Parser) collectCharts(fsys fs.FS, root string) ([]Chart, error) { + var charts []Chart + + walkDirFn := func(filePath string, d fs.DirEntry, err error) error { if err != nil { return err } - if entry.IsDir() { + + if d.IsDir() { return nil } - if detection.IsArchive(filePath) && !isDependencyChartArchive(p.workingFS, filePath) { - tarFS, err := p.addTarToFS(filePath) - if errors.Is(err, errSkipFS) { - // an unpacked Chart already exists - return nil - } else if err != nil { - return fmt.Errorf("failed to add tar %q to FS: %w", filePath, err) + switch { + case strings.HasSuffix(filePath, "Chart.yaml"): + c, err := loadChart(fsys, path.Dir(filePath)) + if err != nil { + p.logger.Error("Failed to load chart", log.FilePath(filePath), log.Err(err)) + return fs.SkipDir } - targetPath := filepath.Dir(filePath) - if targetPath == "" { - targetPath = "." + charts = append(charts, Chart{ + Chart: c, + path: path.Dir(filePath), + }) + return fs.SkipDir + case detection.IsZip(filePath): + c, err := loadArchivedChart(fsys, filePath) + if err != nil { + p.logger.Error("Failed to load chart", log.FilePath(filePath), log.Err(err)) + return nil } - if err := p.ParseFS(ctx, tarFS, targetPath); err != nil { - return fmt.Errorf("parse tar FS error: %w", err) + if c == nil { + return nil } - return nil - } else { - return p.addPaths(filePath) + + charts = append(charts, Chart{ + Chart: c, + path: filePath, + }) } - }); err != nil { - return fmt.Errorf("walk dir error: %w", err) - } - return nil -} + return nil + } -func isDependencyChartArchive(fsys fs.FS, archivePath string) bool { - parent := path.Dir(archivePath) - if !strings.HasSuffix(parent, "charts") { - return false + if err := fs.WalkDir(fsys, root, walkDirFn); err != nil { + return nil, err } - _, err := fs.Stat(fsys, path.Join(parent, "..", "Chart.yaml")) - return err == nil + return charts, nil } -func (p *Parser) addPaths(paths ...string) error { - for _, filePath := range paths { - if _, err := fs.Stat(p.workingFS, filePath); err != nil { - return err - } - - if strings.HasSuffix(filePath, "Chart.yaml") && p.rootPath == "" { - if err := p.extractChartName(filePath); err != nil { - return err - } - p.rootPath = filepath.Dir(filePath) - } - p.filepaths = append(p.filepaths, filePath) - } - return nil +func (p *Parser) resolveReleaseName(c *chart.Chart) { + p.helmClient.ReleaseName = extractChartName(c) } -func (p *Parser) extractChartName(chartPath string) error { - - chrt, err := p.workingFS.Open(chartPath) - if err != nil { - return err - } - defer func() { _ = chrt.Close() }() - - var chartContent map[string]any - if err := yaml.NewDecoder(chrt).Decode(&chartContent); err != nil { - // the chart likely has the name templated and so cannot be parsed as yaml - use a temporary name - if dir := filepath.Dir(chartPath); dir != "" && dir != "." { - p.helmClient.ReleaseName = dir - } else { - p.helmClient.ReleaseName = uuid.NewString() - } - return nil +func extractChartName(c *chart.Chart) string { + if c.Metadata == nil { + return uuid.NewString() } - if name, ok := chartContent["name"]; !ok { - return fmt.Errorf("could not extract the chart name from %s", chartPath) - } else { - p.helmClient.ReleaseName = fmt.Sprintf("%v", name) - } - return nil + return c.Metadata.Name } -func (p *Parser) RenderedChartFiles() ([]ChartFile, error) { - workingChart, err := p.loadChart() - if err != nil { - return nil, err +func (p *Parser) renderChart(c Chart) ([]ChartFile, error) { + if req := c.Metadata.Dependencies; req != nil { + if err := action.CheckDependencies(c.Chart, req); err != nil { + return nil, err + } } - workingRelease, err := p.getRelease(workingChart) + r, err := p.getRelease(c) if err != nil { return nil, err } - var manifests bytes.Buffer - _, _ = fmt.Fprintln(&manifests, strings.TrimSpace(workingRelease.Manifest)) - - splitManifests := releaseutil.SplitManifests(manifests.String()) - manifestsKeys := make([]string, 0, len(splitManifests)) - for k := range splitManifests { - manifestsKeys = append(manifestsKeys, k) - } - return p.getRenderedManifests(manifestsKeys, splitManifests), nil + return getRenderedManifests(c.path, r.Manifest), nil } -func (p *Parser) getRelease(chrt *chart.Chart) (*release.Release, error) { - opts := &ValueOptions{ - ValueFiles: p.valuesFiles, - Values: p.values, - FileValues: p.fileValues, - StringValues: p.stringValues, - } +func (p *Parser) getRelease(c Chart) (*release.Release, error) { + p.resolveReleaseName(c.Chart) + defer func() { p.helmClient.ReleaseName = "" }() - vals, err := opts.MergeValues() - if err != nil { - return nil, err - } - r, err := p.helmClient.RunWithContext(context.Background(), chrt, vals) + r, err := p.helmClient.RunWithContext(context.Background(), c.Chart, p.vals) if err != nil { return nil, err } @@ -223,22 +202,67 @@ func (p *Parser) getRelease(chrt *chart.Chart) (*release.Release, error) { return r, nil } -func (p *Parser) loadChart() (*chart.Chart, error) { +func getRenderedManifests(chartPath, manifest string) []ChartFile { + entries := releaseutil.SplitManifests(strings.TrimSpace(manifest)) + keys := lo.Keys(entries) + + sort.Sort(releaseutil.BySplitManifestsOrder(keys)) + + files := make([]ChartFile, 0, len(keys)) + for _, key := range keys { + entry := entries[key] + submatch := manifestNameRegex.FindStringSubmatch(entry) + if len(submatch) == 0 { + continue + } + files = append(files, ChartFile{ + ChartPath: chartPath, + Path: getManifestPath(entry), + Content: entry, + }) + } + return files +} + +func getManifestPath(manifest string) string { + lines := strings.Split(manifest, "\n") + if len(lines) == 0 { + return "unknown.yaml" + } + parts := strings.SplitN(strings.TrimPrefix(lines[0], "# Source: "), "/", 2) + if len(parts) > 1 { + return parts[1] + } + return parts[0] +} + +func loadChart(fsys fs.FS, root string) (*chart.Chart, error) { var files []*loader.BufferedFile - for _, filePath := range p.filepaths { - b, err := fs.ReadFile(p.workingFS, filePath) + walkFn := func(filePath string, d fs.DirEntry, err error) error { if err != nil { - return nil, err + return err + } + + if d.IsDir() { + return nil + } + + b, err := fs.ReadFile(fsys, filePath) + if err != nil { + return err } - filePath = strings.TrimPrefix(filePath, p.rootPath+"/") filePath = filepath.ToSlash(filePath) - files = append(files, &loader.BufferedFile{ - Name: filePath, - Data: b, - }) + filePath = strings.TrimPrefix(filePath, root+"/") + files = append(files, &loader.BufferedFile{Name: filePath, Data: b}) + + return nil + } + + if err := fs.WalkDir(fsys, root, walkFn); err != nil { + return nil, err } c, err := loader.LoadFiles(files) @@ -246,40 +270,64 @@ func (p *Parser) loadChart() (*chart.Chart, error) { return nil, err } - if req := c.Metadata.Dependencies; req != nil { - if err := action.CheckDependencies(c, req); err != nil { - return nil, err - } + return c, err +} + +func loadArchivedChart(fsys fs.FS, filePath string) (*chart.Chart, error) { + ok, err := archivedChartNextToUnpacked(fsys, filePath) + if err != nil { + return nil, err + } + + // skip if unpacked Chart exists + // we can avoid duplicate results if the user packaged Chart and scans this directory + if ok { + return nil, nil } - return c, nil + f, err := fsys.Open(filePath) + if err != nil { + return nil, err + } + defer f.Close() + return loader.LoadArchive(f) } -func (*Parser) getRenderedManifests(manifestsKeys []string, splitManifests map[string]string) []ChartFile { - sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys)) - var manifestsToRender []ChartFile - for _, manifestKey := range manifestsKeys { - manifest := splitManifests[manifestKey] - submatch := manifestNameRegex.FindStringSubmatch(manifest) - if len(submatch) == 0 { - continue +func archivedChartNextToUnpacked(fsys fs.FS, filePath string) (bool, error) { + f, err := fsys.Open(filePath) + if err != nil { + return false, err + } + defer f.Close() + + gzr, err := gzip.NewReader(f) + if err != nil { + return false, fmt.Errorf("failed to create gzip reader: %w", err) + } + defer gzr.Close() + tr := tar.NewReader(gzr) + + header, err := tr.Next() + if err != nil { + if errors.Is(err, io.EOF) { + return false, nil } - manifestsToRender = append(manifestsToRender, ChartFile{ - TemplateFilePath: getManifestPath(manifest), - ManifestContent: manifest, - }) + return false, fmt.Errorf("failed to get next entry: %w", err) } - return manifestsToRender -} -func getManifestPath(manifest string) string { - lines := strings.Split(manifest, "\n") - if len(lines) == 0 { - return "unknown.yaml" + name := filepath.ToSlash(header.Name) + + // helm package . or helm package + chartPaths := []string{ + path.Join(filePath, "..", "Chart.yaml"), + path.Join(filePath, "..", path.Dir(name), "Chart.yaml"), } - manifestFilePathParts := strings.SplitN(strings.TrimPrefix(lines[0], "# Source: "), "/", 2) - if len(manifestFilePathParts) > 1 { - return manifestFilePathParts[1] + + for _, chartPath := range chartPaths { + _, err := fs.Stat(fsys, chartPath) + if err == nil { + return true, nil + } } - return manifestFilePathParts[0] + return false, nil } diff --git a/pkg/iac/scanners/helm/parser/parser_tar.go b/pkg/iac/scanners/helm/parser/parser_tar.go deleted file mode 100644 index 8e2ba97a81d2..000000000000 --- a/pkg/iac/scanners/helm/parser/parser_tar.go +++ /dev/null @@ -1,176 +0,0 @@ -package parser - -import ( - "archive/tar" - "compress/gzip" - "errors" - "fmt" - "io" - "io/fs" - "os" - "path" - "path/filepath" - - "github.com/liamg/memoryfs" - - "github.com/aquasecurity/trivy/pkg/iac/detection" - "github.com/aquasecurity/trivy/pkg/log" -) - -var errSkipFS = errors.New("skip parse FS") - -func (p *Parser) addTarToFS(archivePath string) (fs.FS, error) { - tarFS := memoryfs.CloneFS(p.workingFS) - - file, err := tarFS.Open(archivePath) - if err != nil { - return nil, fmt.Errorf("failed to open tar: %w", err) - } - defer file.Close() - - var tr *tar.Reader - - if detection.IsZip(archivePath) { - zipped, err := gzip.NewReader(file) - if err != nil { - return nil, fmt.Errorf("failed to create gzip reader: %w", err) - } - defer zipped.Close() - tr = tar.NewReader(zipped) - } else { - tr = tar.NewReader(file) - } - - checkExistedChart := true - symlinks := make(map[string]string) - - for { - header, err := tr.Next() - if err != nil { - if errors.Is(err, io.EOF) { - break - } - return nil, fmt.Errorf("failed to get next entry: %w", err) - } - - name := filepath.ToSlash(header.Name) - - if checkExistedChart { - // Do not add archive files to FS if the chart already exists - // This can happen when the source chart is located next to an archived chart (with the `helm package` command) - // The first level folder in the archive is equal to the Chart name - if _, err := tarFS.Stat(path.Dir(archivePath) + "/" + path.Dir(name)); err == nil { - return nil, errSkipFS - } - checkExistedChart = false - } - - // get the individual path and extract to the current directory - targetPath := path.Join(path.Dir(archivePath), path.Clean(name)) - - link := filepath.ToSlash(header.Linkname) - - switch header.Typeflag { - case tar.TypeDir: - if err := tarFS.MkdirAll(targetPath, os.FileMode(header.Mode)); err != nil && !errors.Is(err, fs.ErrExist) { - return nil, err - } - case tar.TypeReg: - p.logger.Debug("Unpacking tar entry", log.FilePath(targetPath)) - if err := copyFile(tarFS, tr, targetPath); err != nil { - return nil, err - } - case tar.TypeSymlink: - if path.IsAbs(link) { - p.logger.Debug("Symlink is absolute, skipping", log.String("link", link)) - continue - } - - symlinks[targetPath] = path.Join(path.Dir(targetPath), link) // nolint:gosec // virtual file system is used - default: - return nil, fmt.Errorf("header type %q is not supported", header.Typeflag) - } - } - - for target, link := range symlinks { - if err := copySymlink(tarFS, link, target); err != nil { - return nil, fmt.Errorf("copy symlink error: %w", err) - } - } - - if err := tarFS.Remove(archivePath); err != nil { - return nil, fmt.Errorf("remove tar from FS error: %w", err) - } - - return tarFS, nil -} - -func copySymlink(fsys *memoryfs.FS, src, dst string) error { - fi, err := fsys.Stat(src) - if err != nil { - return nil - } - if fi.IsDir() { - if err := copyDir(fsys, src, dst); err != nil { - return fmt.Errorf("copy dir error: %w", err) - } - return nil - } - - if err := copyFileLazy(fsys, src, dst); err != nil { - return fmt.Errorf("copy file error: %w", err) - } - - return nil -} - -func copyFile(fsys *memoryfs.FS, src io.Reader, dst string) error { - if err := fsys.MkdirAll(path.Dir(dst), fs.ModePerm); err != nil && !errors.Is(err, fs.ErrExist) { - return fmt.Errorf("mkdir error: %w", err) - } - - b, err := io.ReadAll(src) - if err != nil { - return fmt.Errorf("read error: %w", err) - } - - if err := fsys.WriteFile(dst, b, fs.ModePerm); err != nil { - return fmt.Errorf("write file error: %w", err) - } - - return nil -} - -func copyDir(fsys *memoryfs.FS, src, dst string) error { - walkFn := func(filePath string, entry fs.DirEntry, err error) error { - if err != nil { - return err - } - - if entry.IsDir() { - return nil - } - - dst := path.Join(dst, filePath[len(src):]) - - if err := copyFileLazy(fsys, filePath, dst); err != nil { - return fmt.Errorf("copy file error: %w", err) - } - return nil - } - - return fs.WalkDir(fsys, src, walkFn) -} - -func copyFileLazy(fsys *memoryfs.FS, src, dst string) error { - if err := fsys.MkdirAll(path.Dir(dst), fs.ModePerm); err != nil && !errors.Is(err, fs.ErrExist) { - return fmt.Errorf("mkdir error: %w", err) - } - return fsys.WriteLazyFile(dst, func() (io.Reader, error) { - f, err := fsys.Open(src) - if err != nil { - return nil, err - } - return f, nil - }, fs.ModePerm) -} diff --git a/pkg/iac/scanners/helm/parser/parser_test.go b/pkg/iac/scanners/helm/parser/parser_test.go index 16a422d78532..71d32c32b3fe 100644 --- a/pkg/iac/scanners/helm/parser/parser_test.go +++ b/pkg/iac/scanners/helm/parser/parser_test.go @@ -1,4 +1,4 @@ -package parser +package parser_test import ( "context" @@ -6,61 +6,64 @@ import ( "path/filepath" "testing" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" -) - -func TestParseFS(t *testing.T) { - t.Run("source chart is located next to an same archived chart", func(t *testing.T) { - p, err := New(".") - require.NoError(t, err) - require.NoError(t, p.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", "chart-and-archived-chart")), ".")) - expectedFiles := []string{ - "my-chart/Chart.yaml", - "my-chart/templates/pod.yaml", - } - assert.Equal(t, expectedFiles, p.filepaths) - }) - - t.Run("archive with symlinks", func(t *testing.T) { - // mkdir -p chart && cd $_ - // touch Chart.yaml - // mkdir -p dir && cp -p Chart.yaml dir/Chart.yaml - // mkdir -p sym-to-file && ln -s ../Chart.yaml sym-to-file/Chart.yaml - // ln -s dir sym-to-dir - // mkdir rec-sym && touch rec-sym/Chart.yaml - // ln -s . ./rec-sym/a - // cd .. && tar -czvf chart.tar.gz chart && rm -rf chart - p, err := New(".") - require.NoError(t, err) + "github.com/aquasecurity/trivy/pkg/iac/scanners/helm/parser" +) - fsys := os.DirFS(filepath.Join("testdata", "archive-with-symlinks")) - require.NoError(t, p.ParseFS(context.TODO(), fsys, "chart.tar.gz")) +func Test_ParseFS(t *testing.T) { - expectedFiles := []string{ - "chart/Chart.yaml", - "chart/dir/Chart.yaml", - "chart/rec-sym/Chart.yaml", - "chart/rec-sym/a/Chart.yaml", - "chart/sym-to-dir/Chart.yaml", - "chart/sym-to-file/Chart.yaml", - } - assert.Equal(t, expectedFiles, p.filepaths) - }) + tests := []struct { + name string + dir string + expected []string + }{ + { + name: "source chart is located next to an same archived chart", + dir: "chart-and-archived-chart", + expected: []string{"templates/pod.yaml"}, + }, + { + name: "archive with symlinks", + // shared-library in "charts" is symlink + // ln -s ../shared-library charts/shared-library + // helm package . + dir: "archive-with-symlinks", + expected: []string{"charts/foo/templates/secret.yaml"}, + }, + { + name: "chart with multiple archived deps", + dir: "multiple-archived-deps", + expected: []string{ + "charts/wordpress-operator/templates/clusterrolebinding.yaml", + "charts/wordpress-operator/templates/service.yaml", + "charts/wordpress-operator/templates/deployment.yaml", + "charts/wordpress-operator/templates/serviceaccount.yaml", + "charts/wordpress-operator/templates/clusterrole.yaml", + "charts/mysql-operator/templates/service_account_operator.yaml", + "charts/mysql-operator/templates/cluster_role_operator.yaml", + "charts/mysql-operator/templates/cluster_role_sidecar.yaml", + "charts/mysql-operator/templates/cluster_role_binding_operator.yaml", + "charts/mysql-operator/templates/service.yaml", + "charts/mysql-operator/templates/deployment.yaml", + "charts/mysql-operator/templates/cluster_kopf_keepering.yaml", + }, + }, + } - t.Run("chart with multiple archived deps", func(t *testing.T) { - p, err := New(".") - require.NoError(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p, err := parser.New() + require.NoError(t, err) - fsys := os.DirFS(filepath.Join("testdata", "multiple-archived-deps")) - require.NoError(t, p.ParseFS(context.TODO(), fsys, ".")) + fsys := os.DirFS(filepath.Join("testdata", tt.dir)) + files, err := p.ParseFS(context.TODO(), fsys, ".") + require.NoError(t, err) - expectedFiles := []string{ - "Chart.yaml", - "charts/common-2.26.0.tgz", - "charts/opentelemetry-collector-0.108.0.tgz", - } - assert.Equal(t, expectedFiles, p.filepaths) - }) + paths := lo.Map(files, func(f parser.ChartFile, _ int) string { return f.Path }) + assert.ElementsMatch(t, tt.expected, paths) + }) + } } diff --git a/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/chart.tar.gz b/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/chart.tar.gz deleted file mode 100644 index a3183710c17f..000000000000 Binary files a/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/chart.tar.gz and /dev/null differ diff --git a/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/example-0.1.0.tgz b/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/example-0.1.0.tgz new file mode 100644 index 000000000000..eb8648e05caf Binary files /dev/null and b/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/example-0.1.0.tgz differ diff --git a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/Chart.yaml b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/Chart.yaml index 82d6c918088f..9dec2ca5065f 100644 --- a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/Chart.yaml +++ b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/Chart.yaml @@ -1,14 +1,14 @@ apiVersion: v2 appVersion: "1.1" description: Test Chart -name: y-chart +name: my-chart version: 1.0.0 -kubeVersion: ">=1.21" dependencies: - - name: common - repository: https://charts.bitnami.com/bitnami - version: 2.26.0 - - name: opentelemetry-collector - version: 0.108.0 - repository: https://open-telemetry.github.io/opentelemetry-helm-charts \ No newline at end of file + - name: mysql-operator + version: 2.2.2 + repository: https://mysql.github.io/mysql-operator + + - name: wordpress-operator + version: 0.12.4 + repository: https://helm-charts.bitpoke.io \ No newline at end of file diff --git a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/common-2.26.0.tgz b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/common-2.26.0.tgz deleted file mode 100644 index 43f85ba36a91..000000000000 Binary files a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/common-2.26.0.tgz and /dev/null differ diff --git a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/mysql-operator-2.2.2.tgz b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/mysql-operator-2.2.2.tgz new file mode 100644 index 000000000000..b80fc3c0425e Binary files /dev/null and b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/mysql-operator-2.2.2.tgz differ diff --git a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/opentelemetry-collector-0.108.0.tgz b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/opentelemetry-collector-0.108.0.tgz deleted file mode 100644 index 0ea88fb48247..000000000000 Binary files a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/opentelemetry-collector-0.108.0.tgz and /dev/null differ diff --git a/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/wordpress-operator-0.12.4.tgz b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/wordpress-operator-0.12.4.tgz new file mode 100644 index 000000000000..4086e9f1a6ba Binary files /dev/null and b/pkg/iac/scanners/helm/parser/testdata/multiple-archived-deps/charts/wordpress-operator-0.12.4.tgz differ diff --git a/pkg/iac/scanners/helm/parser/vals.go b/pkg/iac/scanners/helm/parser/vals.go index f2589b3caec7..53b13d639560 100644 --- a/pkg/iac/scanners/helm/parser/vals.go +++ b/pkg/iac/scanners/helm/parser/vals.go @@ -109,6 +109,7 @@ func readFile(filePath string) ([]byte, error) { } return data.Bytes(), err } else { + // TODO: use fs package return os.ReadFile(filePath) } } diff --git a/pkg/iac/scanners/helm/scanner.go b/pkg/iac/scanners/helm/scanner.go index daf8f3108628..61eea284d2b3 100644 --- a/pkg/iac/scanners/helm/scanner.go +++ b/pkg/iac/scanners/helm/scanner.go @@ -59,86 +59,37 @@ func (s *Scanner) Name() string { return "Helm" } -func (s *Scanner) ScanFS(ctx context.Context, target fs.FS, path string) (scan.Results, error) { +func (s *Scanner) ScanFS(ctx context.Context, fsys fs.FS, root string) (scan.Results, error) { - if err := s.initRegoScanner(target); err != nil { + if err := s.initRegoScanner(fsys); err != nil { return nil, fmt.Errorf("failed to init rego scanner: %w", err) } - var results []scan.Result - if err := fs.WalkDir(target, path, func(path string, d fs.DirEntry, err error) error { - select { - case <-ctx.Done(): - return ctx.Err() - default: - } - - if err != nil { - return err - } - - if d.IsDir() { - return nil - } - - if detection.IsArchive(path) { - if scanResults, err := s.getScanResults(path, ctx, target); err != nil { - return err - } else { - results = append(results, scanResults...) - } - } - - if strings.HasSuffix(path, "Chart.yaml") { - if scanResults, err := s.getScanResults(filepath.Dir(path), ctx, target); err != nil { - return err - } else { - results = append(results, scanResults...) - } - return fs.SkipDir - } - - return nil - }); err != nil { - return nil, err - } - - return results, nil - -} - -func (s *Scanner) getScanResults(path string, ctx context.Context, target fs.FS) (results []scan.Result, err error) { - helmParser, err := parser.New(path, s.parserOptions...) + p, err := parser.New(s.parserOptions...) if err != nil { return nil, err } - if err := helmParser.ParseFS(ctx, target, path); err != nil { + files, err := p.ParseFS(ctx, fsys, root) + if err != nil { return nil, err } - chartFiles, err := helmParser.RenderedChartFiles() - if err != nil { // not valid helm, maybe some other yaml etc., abort - s.logger.Error( - "Failed to render Chart files", - log.FilePath(path), log.Err(err), - ) - return nil, nil - } + var results scan.Results - for _, file := range chartFiles { + for _, file := range files { file := file - s.logger.Debug("Processing rendered chart file", log.FilePath(file.TemplateFilePath)) + s.logger.Debug("Processing rendered chart file", log.FilePath(file.Path)) - manifests, err := kparser.Parse(ctx, strings.NewReader(file.ManifestContent), file.TemplateFilePath) + manifests, err := kparser.Parse(ctx, strings.NewReader(file.Content), file.Path) if err != nil { return nil, fmt.Errorf("unmarshal yaml: %w", err) } for _, manifest := range manifests { fileResults, err := s.regoScanner.ScanInput(ctx, rego.Input{ - Path: file.TemplateFilePath, + Path: file.Path, Contents: manifest, - FS: target, + FS: fsys, }) if err != nil { return nil, fmt.Errorf("scanning error: %w", err) @@ -146,22 +97,25 @@ func (s *Scanner) getScanResults(path string, ctx context.Context, target fs.FS) if len(fileResults) > 0 { renderedFS := memoryfs.New() - if err := renderedFS.MkdirAll(filepath.Dir(file.TemplateFilePath), fs.ModePerm); err != nil { + if err := renderedFS.MkdirAll(filepath.Dir(file.Path), fs.ModePerm); err != nil { return nil, err } - if err := renderedFS.WriteLazyFile(file.TemplateFilePath, func() (io.Reader, error) { - return strings.NewReader(file.ManifestContent), nil + if err := renderedFS.WriteLazyFile(file.Path, func() (io.Reader, error) { + return strings.NewReader(file.Content), nil }, fs.ModePerm); err != nil { return nil, err } - fileResults.SetSourceAndFilesystem(helmParser.ChartSource, renderedFS, detection.IsArchive(helmParser.ChartSource)) + + fileResults.SetSourceAndFilesystem(file.ChartPath, renderedFS, detection.IsArchive(file.ChartPath)) } results = append(results, fileResults...) } } + return results, nil + } func (s *Scanner) initRegoScanner(srcFS fs.FS) error { diff --git a/pkg/iac/scanners/helm/test/option_test.go b/pkg/iac/scanners/helm/test/option_test.go index 05bf96ee01d3..6486655516cc 100644 --- a/pkg/iac/scanners/helm/test/option_test.go +++ b/pkg/iac/scanners/helm/test/option_test.go @@ -39,22 +39,21 @@ func Test_helm_parser_with_options_with_values_file(t *testing.T) { opts = append(opts, parser.OptionWithValuesFile(test.valuesFile)) } - helmParser, err := parser.New(chartName, opts...) + helmParser, err := parser.New(opts...) require.NoError(t, err) - require.NoError(t, helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".")) - manifests, err := helmParser.RenderedChartFiles() + manifests, err := helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") require.NoError(t, err) assert.Len(t, manifests, 3) for _, manifest := range manifests { - expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.TemplateFilePath) + expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.Path) expectedContent, err := os.ReadFile(expectedPath) require.NoError(t, err) cleanExpected := strings.ReplaceAll(string(expectedContent), "\r\n", "\n") - cleanActual := strings.ReplaceAll(manifest.ManifestContent, "\r\n", "\n") + cleanActual := strings.ReplaceAll(manifest.Content, "\r\n", "\n") assert.Equal(t, cleanExpected, cleanActual) } @@ -93,23 +92,22 @@ func Test_helm_parser_with_options_with_set_value(t *testing.T) { opts = append(opts, parser.OptionWithValues(test.values)) } - helmParser, err := parser.New(chartName, opts...) + helmParser, err := parser.New(opts...) require.NoError(t, err) - err = helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") - require.NoError(t, err) - manifests, err := helmParser.RenderedChartFiles() + + manifests, err := helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") require.NoError(t, err) assert.Len(t, manifests, 3) for _, manifest := range manifests { - expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.TemplateFilePath) + expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.Path) expectedContent, err := os.ReadFile(expectedPath) require.NoError(t, err) cleanExpected := strings.ReplaceAll(string(expectedContent), "\r\n", "\n") - cleanActual := strings.ReplaceAll(manifest.ManifestContent, "\r\n", "\n") + cleanActual := strings.ReplaceAll(manifest.Content, "\r\n", "\n") assert.Equal(t, cleanExpected, cleanActual) } @@ -143,23 +141,22 @@ func Test_helm_parser_with_options_with_api_versions(t *testing.T) { opts = append(opts, parser.OptionWithAPIVersions(test.apiVersions...)) } - helmParser, err := parser.New(chartName, opts...) - require.NoError(t, err) - err = helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") + helmParser, err := parser.New(opts...) require.NoError(t, err) - manifests, err := helmParser.RenderedChartFiles() + + manifests, err := helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") require.NoError(t, err) assert.Len(t, manifests, 1) for _, manifest := range manifests { - expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.TemplateFilePath) + expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.Path) expectedContent, err := os.ReadFile(expectedPath) require.NoError(t, err) cleanExpected := strings.TrimSpace(strings.ReplaceAll(string(expectedContent), "\r\n", "\n")) - cleanActual := strings.TrimSpace(strings.ReplaceAll(manifest.ManifestContent, "\r\n", "\n")) + cleanActual := strings.TrimSpace(strings.ReplaceAll(manifest.Content, "\r\n", "\n")) assert.Equal(t, cleanExpected, cleanActual) } @@ -198,26 +195,27 @@ func Test_helm_parser_with_options_with_kube_versions(t *testing.T) { opts = append(opts, parser.OptionWithKubeVersion(test.kubeVersion)) - helmParser, err := parser.New(chartName, opts...) + helmParser, err := parser.New(opts...) if test.expectedError != "" { require.EqualError(t, err, test.expectedError) return } require.NoError(t, err) - require.NoError(t, helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".")) - manifests, err := helmParser.RenderedChartFiles() + + fsys := os.DirFS(filepath.Join("testdata", chartName)) + manifests, err := helmParser.ParseFS(context.TODO(), fsys, ".") require.NoError(t, err) assert.Len(t, manifests, 1) for _, manifest := range manifests { - expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.TemplateFilePath) + expectedPath := filepath.Join("testdata", "expected", "options", chartName, manifest.Path) expectedContent, err := os.ReadFile(expectedPath) require.NoError(t, err) cleanExpected := strings.TrimSpace(strings.ReplaceAll(string(expectedContent), "\r\n", "\n")) - cleanActual := strings.TrimSpace(strings.ReplaceAll(manifest.ManifestContent, "\r\n", "\n")) + cleanActual := strings.TrimSpace(strings.ReplaceAll(manifest.Content, "\r\n", "\n")) assert.Equal(t, cleanExpected, cleanActual) } diff --git a/pkg/iac/scanners/helm/test/parser_test.go b/pkg/iac/scanners/helm/test/parser_test.go index 0116e8b7670b..e8eb09086b5b 100644 --- a/pkg/iac/scanners/helm/test/parser_test.go +++ b/pkg/iac/scanners/helm/test/parser_test.go @@ -33,22 +33,24 @@ func Test_helm_parser(t *testing.T) { for _, test := range tests { t.Run(test.testName, func(t *testing.T) { chartName := test.chartName - helmParser, err := parser.New(chartName) + helmParser, err := parser.New() require.NoError(t, err) - require.NoError(t, helmParser.ParseFS(context.TODO(), os.DirFS("testdata"), chartName)) - manifests, err := helmParser.RenderedChartFiles() + + fsys := os.DirFS("testdata") + manifests, err := helmParser.ParseFS(context.TODO(), fsys, chartName) require.NoError(t, err) assert.Len(t, manifests, 3) for _, manifest := range manifests { - expectedPath := filepath.Join("testdata", "expected", chartName, manifest.TemplateFilePath) + expectedPath := filepath.Join("testdata", "expected", chartName, manifest.Path) expectedContent, err := os.ReadFile(expectedPath) require.NoError(t, err) - got := strings.ReplaceAll(manifest.ManifestContent, "\r\n", "\n") - assert.Equal(t, strings.ReplaceAll(string(expectedContent), "\r\n", "\n"), got) + expected := strings.ReplaceAll(string(expectedContent), "\r\n", "\n") + got := strings.ReplaceAll(manifest.Content, "\r\n", "\n") + assert.Equal(t, expected, got) } }) } @@ -71,9 +73,12 @@ func Test_helm_parser_where_name_non_string(t *testing.T) { t.Logf("Running test: %s", test.testName) - helmParser, err := parser.New(chartName) + helmParser, err := parser.New() + require.NoError(t, err) + + fsys := os.DirFS(filepath.Join("testdata", chartName)) + _, err = helmParser.ParseFS(context.TODO(), fsys, ".") require.NoError(t, err) - require.NoError(t, helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".")) } } @@ -84,16 +89,6 @@ func Test_tar_is_chart(t *testing.T) { archiveFile string isHelmChart bool }{ - { - testName: "standard tarball", - archiveFile: "mysql-8.8.26.tar", - isHelmChart: true, - }, - { - testName: "gzip tarball with tar.gz extension", - archiveFile: "mysql-8.8.26.tar.gz", - isHelmChart: true, - }, { testName: "broken gzip tarball with tar.gz extension", archiveFile: "aws-cluster-autoscaler-bad.tar.gz", @@ -130,11 +125,6 @@ func Test_helm_tarball_parser(t *testing.T) { chartName string archiveFile string }{ - { - testName: "standard tarball", - chartName: "mysql", - archiveFile: "mysql-8.8.26.tar", - }, { testName: "gzip tarball with tar.gz extension", chartName: "mysql", @@ -159,11 +149,10 @@ func Test_helm_tarball_parser(t *testing.T) { testFs := os.DirFS(testTemp) - helmParser, err := parser.New(test.archiveFile) + helmParser, err := parser.New() require.NoError(t, err) - require.NoError(t, helmParser.ParseFS(context.TODO(), testFs, ".")) - manifests, err := helmParser.RenderedChartFiles() + manifests, err := helmParser.ParseFS(context.TODO(), testFs, ".") require.NoError(t, err) assert.Len(t, manifests, 6) @@ -178,18 +167,20 @@ func Test_helm_tarball_parser(t *testing.T) { } for _, manifest := range manifests { - filename := filepath.Base(manifest.TemplateFilePath) + filename := filepath.Base(manifest.Path) assert.Contains(t, oneOf, filename) - if strings.HasSuffix(manifest.TemplateFilePath, "secrets.yaml") { + if strings.HasSuffix(manifest.Path, "secrets.yaml") { continue } - expectedPath := filepath.Join("testdata", "expected", test.chartName, manifest.TemplateFilePath) + expectedPath := filepath.Join("testdata", "expected", test.chartName, manifest.Path) expectedContent, err := os.ReadFile(expectedPath) require.NoError(t, err) - assert.Equal(t, strings.ReplaceAll(string(expectedContent), "\r\n", "\n"), strings.ReplaceAll(manifest.ManifestContent, "\r\n", "\n")) + expected := strings.ReplaceAll(string(expectedContent), "\r\n", "\n") + got := strings.ReplaceAll(manifest.Content, "\r\n", "\n") + assert.Equal(t, expected, got) } } } diff --git a/pkg/iac/scanners/helm/test/scanner_test.go b/pkg/iac/scanners/helm/test/scanner_test.go index ef751ac2a7b8..794c6f8eee61 100644 --- a/pkg/iac/scanners/helm/test/scanner_test.go +++ b/pkg/iac/scanners/helm/test/scanner_test.go @@ -18,11 +18,6 @@ import ( ) func Test_helm_scanner_with_archive(t *testing.T) { - // TODO(simar7): Figure out why this test fails on Winndows only - if runtime.GOOS == "windows" { - t.Skip("skipping test on windows") - } - tests := []struct { testName string chartName string @@ -30,10 +25,10 @@ func Test_helm_scanner_with_archive(t *testing.T) { archiveName string }{ { - testName: "Parsing tarball 'mysql-8.8.26.tar'", + testName: "Parsing tarball 'mysql-8.8.26.tgz'", chartName: "mysql", - path: filepath.Join("testdata", "mysql-8.8.26.tar"), - archiveName: "mysql-8.8.26.tar", + path: filepath.Join("testdata", "mysql-8.8.26.tgz"), + archiveName: "mysql-8.8.26.tgz", }, } @@ -198,10 +193,10 @@ deny[res] { archiveName string }{ { - testName: "Parsing tarball 'mysql-8.8.26.tar'", + testName: "Parsing tarball 'mysql-8.8.26.tgz'", chartName: "mysql", - path: filepath.Join("testdata", "mysql-8.8.26.tar"), - archiveName: "mysql-8.8.26.tar", + path: filepath.Join("testdata", "mysql-8.8.26.tgz"), + archiveName: "mysql-8.8.26.tgz", }, } diff --git a/pkg/iac/scanners/helm/test/testdata/mysql-8.8.26.tar b/pkg/iac/scanners/helm/test/testdata/mysql-8.8.26.tar deleted file mode 100644 index 53cb6802de42..000000000000 Binary files a/pkg/iac/scanners/helm/test/testdata/mysql-8.8.26.tar and /dev/null differ