anchore / anchore-charts

Helm charts for Anchore tools and services
http://charts.anchore.io
Apache License 2.0
47 stars 72 forks source link

Anchore Engine Chart - invalid default values #409

Closed misterjacko closed 1 week ago

misterjacko commented 1 week ago

In the v5.10.0 release a section was added to the default helm values that is invalid

anchoreConfig:
  policy_engine:
# vvv this part was added vvv
    vulnerabilities:
      matching:
        exclude:
          providers: null
          package_types: null

This conflicts with the requirements of the anchore_configmap template where those cant be null.

{{- $exclude_providers := required "anchoreConfig.policy_engine.vulnerabilities.matching.exclude.providers is required" .Values.anchoreConfig.policy_engine.vulnerabilities.matching.exclude.providers -}}
{{- $exclude_package := required "anchoreConfig.policy_engine.vulnerabilities.matching.exclude.package_types is required" .Values.anchoreConfig.policy_engine.vulnerabilities.matching.exclude.package_types -}}

The code expects a list there so the null values can be changed to empty lists [] and the base values will deploy.

new values:

anchoreConfig:
  policy_engine:
    vulnerabilities:
      matching:
        exclude:
          providers: []
          package_types: []

will be submitting PR to resolve.

bdelvecchio commented 1 week ago

I encountered the same error during upgrade, and came to the same conclusion.

:+1:

HN23 commented 1 week ago

Hello,

Thanks for the contribution. This was actually an intentional breaking change, hence the major version bump. This was done because the feeds service is now removed as a dependency from the chart. Anchore Enterprise will now use Anchore's hosted feeds/data service. By default, Anchore's hosted feeds service will not exclude any providers or package_types. If your deployment was using an external/separately deployed feeds service prior, we could not have visibility into what packages/providers may have been disabled, so the decision was made to have users explicitly pass in which providers/packages they want to disable to keep vulnerability results the same.

You can refer to the release notes where this is called out with a WARNING to see how to exclude providers or package types. In response to your concerns, we will make a patch to try to set it to no exclusions if certain conditions apply (eg. used dependent feeds chart and no package/providers exclusions were provided), but in the case where an external feeds service was used, we will still have this fail and require manual action from the user.

Thanks again

misterjacko commented 1 week ago

I don't know if this is just an issue of a breaking change. I am a potential customer and this is my initial deploy for a POC. Not any sort of upgrade for anything.

The values in that spot are a list/array. you are using the wrong "empty" values.

if the intention is to force the user to address via braking a vanilla helm deploy such a requirement should be outlined in the install docs. https://docs.anchore.com/current/docs/deployment/helm/

HN23 commented 1 week ago

I don't know if this is just an issue of a breaking change. I am a potential customer and this is my initial deploy for a POC. Not any sort of upgrade for anything.

The values in that spot are a list/array. you are using the wrong "empty" values.

if the intention is to force the user to address via braking a vanilla helm deploy such a requirement should be outlined in the install docs. https://docs.anchore.com/current/docs/deployment/helm/

Thanks for the input. This was probably an oversight on our part for the new customers/user scenario. We'll try to get this patched asap and will update the docs from that page as well.

Thanks again, Hung

misterjacko commented 1 week ago

If you must keep it null, I humbly suggest a comment explaining in the default values.yaml pointing to your other docs as that is where people will initially go to investigate.

HN23 commented 1 week ago

That makes perfect sense. I think the plan is going to be:

if its a new install:
  set it as an empty list.
else if its an upgrade:
  if the feeds chart was not enabled (ie. using an externally deployed feeds service): make a manual intervention required
  else if the feeds chart _was_ enabled, we'll try to map any excluded packages/providers accordingly as we'll have the values.

In addition to the above, we'll be sure to update the docs and values files to make this more clear. Thanks again. We definitely appreciate any and all feedback, especially when it comes to usability and where we might have shortcomings.

Hung