Skip to content

Commit

Permalink
fix: ignore templates in Chart dependencies based on ignore paths (#667)
Browse files Browse the repository at this point in the history
  • Loading branch information
Trojan295 authored Jan 18, 2024
1 parent 8094592 commit e0225f4
Show file tree
Hide file tree
Showing 12 changed files with 255 additions and 16 deletions.
12 changes: 11 additions & 1 deletion pkg/command/lint/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"

"golang.stackrox.io/kube-linter/pkg/diagnostic"
"golang.stackrox.io/kube-linter/pkg/pathutil"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"golang.stackrox.io/kube-linter/internal/flagutil"
Expand Down Expand Up @@ -86,7 +87,16 @@ func Command() *cobra.Command {
return err
}

lintCtxs, err := lintcontext.CreateContexts(ignorePaths, args...)
absArgs := []string{}
for _, arg := range args {
absArg, err := pathutil.GetAbsolutPath(arg)
if err != nil {
return err
}
absArgs = append(absArgs, absArg)
}

lintCtxs, err := lintcontext.CreateContexts(ignorePaths, absArgs...)
if err != nil {
return err
}
Expand Down
15 changes: 11 additions & 4 deletions pkg/lintcontext/create_contexts.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ fileOrDirsLoop:
if !info.IsDir() {
if strings.HasSuffix(strings.ToLower(currentPath), ".tgz") {
ctx := newCtx(options)
ctx.loadObjectsFromTgzHelmChart(currentPath)
if err := ctx.loadObjectsFromTgzHelmChart(currentPath, ignorePaths); err != nil {
return errors.Wrapf(err, "loading helm chart %s", currentPath)
}

contextsByDir[currentPath] = ctx
return nil
}
Expand All @@ -118,7 +121,9 @@ fileOrDirsLoop:
}
ctx := newCtx(options)
contextsByDir[currentPath] = ctx
ctx.loadObjectsFromHelmChart(currentPath)
if err := ctx.loadObjectsFromHelmChart(currentPath, ignorePaths); err != nil {
return errors.Wrap(err, "loading helm chart")
}
return filepath.SkipDir
}
return nil
Expand All @@ -142,9 +147,11 @@ fileOrDirsLoop:
// CreateContextsFromHelmArchive creates a context from TGZ reader of Helm Chart.
// Note: although this function is not used in CLI, it is exposed from kube-linter library and therefore should stay.
// See https://github.com/stackrox/kube-linter/pull/173
func CreateContextsFromHelmArchive(fileName string, tgzReader io.Reader) ([]LintContext, error) {
func CreateContextsFromHelmArchive(ignorePaths []string, fileName string, tgzReader io.Reader) ([]LintContext, error) {
ctx := newCtx(Options{})
ctx.readObjectsFromTgzHelmChart(fileName, tgzReader)
if err := ctx.readObjectsFromTgzHelmChart(fileName, tgzReader, ignorePaths); err != nil {
return nil, err
}

return []LintContext{ctx}, nil
}
30 changes: 29 additions & 1 deletion pkg/lintcontext/create_contexts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,33 @@ func TestCreateContextsWithIgnorePaths(t *testing.T) {
checkEmptyLintContext(t, contexts)
}

func TestIgnoreSubchartManifests(t *testing.T) {
ignorePaths := []string{
"../../tests/testdata/mychart/charts/**",
}
dir := "../../tests/testdata/mychart"

lintCtxs, err := CreateContexts(ignorePaths, dir)
require.NoError(t, err)
lintCtx := lintCtxs[0]
objects := lintCtx.Objects()

actualPaths := make([]string, 0, len(objects))
for _, obj := range objects {
actualPaths = append(actualPaths, obj.Metadata.FilePath)
}

expectedPaths := []string{
"../../tests/testdata/mychart/templates/serviceaccount.yaml",
"../../tests/testdata/mychart/templates/service.yaml",
"../../tests/testdata/mychart/templates/hpa.yaml",
"../../tests/testdata/mychart/templates/deployment.yaml",
"../../tests/testdata/mychart/templates/tests/test-connection.yaml",
}

assert.ElementsMatch(t, expectedPaths, actualPaths)
}

func TestCreateContextsObjectPaths(t *testing.T) {
bools := []bool{false, true}

Expand Down Expand Up @@ -117,7 +144,7 @@ func createContextsAndVerifyPaths(t *testing.T, useTarball, useAbsolutePath, ren
require.NoError(t, file.Close())
}()

lintCtxs, err = CreateContextsFromHelmArchive(mockPath, file)
lintCtxs, err = CreateContextsFromHelmArchive(testIgnorePaths, mockPath, file)
} else {
lintCtxs, err = CreateContexts(testIgnorePaths, testPath)
}
Expand Down Expand Up @@ -165,6 +192,7 @@ func checkObjectPaths(t *testing.T, objects []Object, expectedPrefix string) {
path.Join(expectedPrefix, "templates/service.yaml"),
path.Join(expectedPrefix, "templates/serviceaccount.yaml"),
path.Join(expectedPrefix, "templates/tests/test-connection.yaml"),
path.Join(expectedPrefix, "charts/subchart/templates/deployment.yaml"),
}
assert.ElementsMatchf(t, expectedPaths, actualPaths, "expected and actual template paths don't match")
}
35 changes: 25 additions & 10 deletions pkg/lintcontext/parse_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"strings"

"github.com/bmatcuk/doublestar/v4"
y "github.com/ghodss/yaml"
ocsAppsV1 "github.com/openshift/api/apps/v1"
ocpSecV1 "github.com/openshift/api/security/v1"
Expand Down Expand Up @@ -122,23 +123,24 @@ func (l *lintContextImpl) renderValues(chrt *chart.Chart, values map[string]inte
return rendered, nil
}

func (l *lintContextImpl) loadObjectsFromHelmChart(dir string) {
func (l *lintContextImpl) loadObjectsFromHelmChart(dir string, ignorePaths []string) error {
renderedFiles, err := l.renderHelmChart(dir)
if err != nil {
l.addInvalidObjects(InvalidObject{Metadata: ObjectMetadata{FilePath: dir}, LoadErr: err})
return
return nil
}

// Paths returned by helm include redundant directory in front, therefore we strip it out.
l.loadHelmRenderedTemplates(dir, normalizeDirectoryPaths(renderedFiles))
return l.loadHelmRenderedTemplates(dir, normalizeDirectoryPaths(renderedFiles), ignorePaths)
}

func (l *lintContextImpl) loadObjectsFromTgzHelmChart(tgzFile string) {
func (l *lintContextImpl) loadObjectsFromTgzHelmChart(tgzFile string, ignorePaths []string) error {
renderedFiles, err := l.renderTgzHelmChart(tgzFile)
if err != nil {
l.addInvalidObjects(InvalidObject{Metadata: ObjectMetadata{FilePath: tgzFile}, LoadErr: err})
return
return nil
}
l.loadHelmRenderedTemplates(tgzFile, renderedFiles)
return l.loadHelmRenderedTemplates(tgzFile, renderedFiles, ignorePaths)
}

func (l *lintContextImpl) renderTgzHelmChart(tgzFile string) (map[string]string, error) {
Expand Down Expand Up @@ -251,19 +253,30 @@ func (l *lintContextImpl) renderTgzHelmChartReader(fileName string, tgzReader io
return l.renderChart(fileName, chrt)
}

func (l *lintContextImpl) readObjectsFromTgzHelmChart(fileName string, tgzReader io.Reader) {
func (l *lintContextImpl) readObjectsFromTgzHelmChart(fileName string, tgzReader io.Reader, ignoredPaths []string) error {
renderedFiles, err := l.renderTgzHelmChartReader(fileName, tgzReader)
if err != nil {
l.addInvalidObjects(InvalidObject{Metadata: ObjectMetadata{FilePath: fileName}, LoadErr: err})
return
return nil
}
l.loadHelmRenderedTemplates(fileName, renderedFiles)
return l.loadHelmRenderedTemplates(fileName, renderedFiles, ignoredPaths)
}

func (l *lintContextImpl) loadHelmRenderedTemplates(chartPath string, renderedFiles map[string]string) {
func (l *lintContextImpl) loadHelmRenderedTemplates(chartPath string, renderedFiles map[string]string, ignorePaths []string) error {
nextFile:
for path, contents := range renderedFiles {
pathToTemplate := filepath.Join(chartPath, path)

for _, path := range ignorePaths {
ignoreMatch, err := doublestar.PathMatch(path, pathToTemplate)
if err != nil {
return errors.Wrapf(err, "could not match pattern %s", path)
}
if ignoreMatch {
continue nextFile
}
}

// Skip NOTES.txt file that may be present among templates but is not a kubernetes resource.
if strings.HasSuffix(pathToTemplate, string(filepath.Separator)+chartutil.NotesName) {
continue
Expand All @@ -274,6 +287,8 @@ func (l *lintContextImpl) loadHelmRenderedTemplates(chartPath string, renderedFi
l.addInvalidObjects(InvalidObject{Metadata: ObjectMetadata{FilePath: pathToTemplate}, LoadErr: loadErr})
}
}

return nil
}

// normalizeDirectoryPaths removes the first element of the path that gets added by the Helm library.
Expand Down
Binary file modified tests/testdata/mychart-0.1.0.tgz
Binary file not shown.
6 changes: 6 additions & 0 deletions tests/testdata/mychart/Chart.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
dependencies:
- name: subchart
repository: file://../subchart
version: 0.1.0
digest: sha256:f8645a52640009a9d8905d929d3362e04720964c71e37af357b8b5e2852c365d
generated: "2023-11-23T15:35:33.69184771+01:00"
4 changes: 4 additions & 0 deletions tests/testdata/mychart/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ description: A Helm chart for Kubernetes
name: mychart
type: application
version: 0.1.0
dependencies:
- name: subchart
version: 0.1.0
repository: file://../subchart
Binary file added tests/testdata/mychart/charts/subchart-0.1.0.tgz
Binary file not shown.
23 changes: 23 additions & 0 deletions tests/testdata/subchart/.helmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Patterns to ignore when building packages.
# This supports shell glob matching, relative path matching, and
# negation (prefixed with !). Only one pattern per line.
.DS_Store
# Common VCS dirs
.git/
.gitignore
.bzr/
.bzrignore
.hg/
.hgignore
.svn/
# Common backup files
*.swp
*.bak
*.tmp
*.orig
*~
# Various IDEs
.project
.idea/
*.tmproj
.vscode/
6 changes: 6 additions & 0 deletions tests/testdata/subchart/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
apiVersion: v2
appVersion: 1.16.0
description: A Helm chart for Kubernetes
name: subchart
type: application
version: 0.1.0
61 changes: 61 additions & 0 deletions tests/testdata/subchart/templates/deployment.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ include "mychart.fullname" . }}
labels:
{{- include "mychart.labels" . | nindent 4 }}
spec:
{{- if not .Values.autoscaling.enabled }}
replicas: {{ .Values.replicaCount }}
{{- end }}
selector:
matchLabels:
{{- include "mychart.selectorLabels" . | nindent 6 }}
template:
metadata:
{{- with .Values.podAnnotations }}
annotations:
{{- toYaml . | nindent 8 }}
{{- end }}
labels:
{{- include "mychart.selectorLabels" . | nindent 8 }}
spec:
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
{{- end }}
serviceAccountName: {{ include "mychart.serviceAccountName" . }}
securityContext:
{{- toYaml .Values.podSecurityContext | nindent 8 }}
containers:
- name: {{ .Chart.Name }}
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
ports:
- name: http
containerPort: 80
protocol: TCP
livenessProbe:
httpGet:
path: /
port: http
readinessProbe:
httpGet:
path: /
port: http
resources:
{{- toYaml .Values.resources | nindent 12 }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.affinity }}
affinity:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.tolerations }}
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
79 changes: 79 additions & 0 deletions tests/testdata/subchart/values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Default values for mychart.
# This is a YAML-formatted file.
# Declare variables to be passed into your templates.

replicaCount: 1

image:
repository: nginx
pullPolicy: IfNotPresent
# Overrides the image tag whose default is the chart appVersion.
tag: ""

imagePullSecrets: []
nameOverride: ""
fullnameOverride: ""

serviceAccount:
# Specifies whether a service account should be created
create: true
# Annotations to add to the service account
annotations: {}
# The name of the service account to use.
# If not set and create is true, a name is generated using the fullname template
name: ""

podAnnotations: {}

podSecurityContext: {}
# fsGroup: 2000

securityContext: {}
# capabilities:
# drop:
# - ALL
# readOnlyRootFilesystem: true
# runAsNonRoot: true
# runAsUser: 1000

service:
type: ClusterIP
port: 80

ingress:
enabled: false
annotations: {}
# kubernetes.io/ingress.class: nginx
# kubernetes.io/tls-acme: "true"
hosts:
- host: chart-example.local
paths: []
tls: []
# - secretName: chart-example-tls
# hosts:
# - chart-example.local

resources: {}
# We usually recommend not to specify default resources and to leave this as a conscious
# choice for the user. This also increases chances charts run on environments with little
# resources, such as Minikube. If you do want to specify resources, uncomment the following
# lines, adjust them as necessary, and remove the curly braces after 'resources:'.
# limits:
# cpu: 100m
# memory: 128Mi
# requests:
# cpu: 100m
# memory: 128Mi

autoscaling:
enabled: true
minReplicas: 1
maxReplicas: 100
targetCPUUtilizationPercentage: 80
# targetMemoryUtilizationPercentage: 80

nodeSelector: {}

tolerations: []

affinity: {}

0 comments on commit e0225f4

Please sign in to comment.