nikhilsbhat / helm-images

Helm plugin to fetch all possible images from the chart before deployment or from a deployed release
https://artifacthub.io/packages/helm-plugin/images/images
MIT License
59 stars 12 forks source link

Replace template parsing with yaml.Decoder #42

Open philk opened 2 months ago

philk commented 2 months ago

This solves some of the edge cases around parsing for empty or invalid document by replacing the string splitting with the yaml package's native handling of multi-document files. Along the way it also simplifies upstream functions by returning parsed yaml as a map[string]interface{} instead of a string. Even after #29 I still had issues with documents like:

---
# <snip, normal document>
          env:
            - name: "GOMAXPROCS"
              value: "5"
            - name: "GOMEMLIMIT"
              value: "6442450944"
            - name: "JAEGER_REPORTER_MAX_QUEUE_SIZE"
              value: "1000"
---
# Source: mimir/charts/mimir-distributed/templates/minio/create-bucket-job.yaml
# Minio provides post-install hook to create bucket
# however the hook won't be executed if helm install is run
# with --wait flag. Hence this job is a workaround for that.
# See https://github.com/grafana/mimir/issues/2464
---
# Source: mimir/charts/mimir-distributed/templates/metamonitoring/mixin-alerts.yaml
apiVersion: monitoring.coreos.com/v1
kind: PrometheusRule
metadata:
  name: mimir-alerts
# <etc>

In the event of an un-parseable document you'll receive an error with the parsing error and the document the parsing failed on. This isn't extremely useful and returning the chunk around the failure seems like it'd be more useful, but that's beyond my skills and this should point folks in the right direction at least.

One other note is I had to fix a test error that currently exists in master (see 7d26eaf) with the image-regex functionality. Along the way I spent a little time digging in and I'm pretty sure that it doesn't work at all since none of the Get calls (see Deployments) are passing through the regex except ConfigMap. I have a partial branch that attempts to make image-regex work but didn't want to block this fix with that.

nikhilsbhat commented 2 months ago

@philk , thanks for the PR. I'll review this and update.

nikhilsbhat commented 3 weeks ago

@philk , can you please help me with the helm chart to test the use case of empty doc or invalid doc?