search

netbox-community / netbox-chart

A Helm chart for NetBox
https://netbox.readthedocs.io/
Apache License 2.0
252 stars 152 forks source link

`extraConfig` fails #376

Open pimvandenbroek opened 2 days ago

pimvandenbroek commented 2 days ago

Hi,

remoteAuth.backends accepts an array according to the documentation, however this breaks in https://github.com/netbox-community/netbox-chart/compare/netbox-5.0.0-beta.112...netbox-5.0.0-beta.113

The range option is removed here, causing a Error: YAML parse error on netbox/templates/cronjob.yaml: error converting YAML to JSON: yaml: line 77: mapping values are not allowed in this context

Is the documentation outdated in this case?

Thanks in advance!

LeoColomb commented 2 days ago

Thanks for filing this issue, @pimvandenbroek. The range function has been replaced with has, which checks whether a value is in an array (actually list/slice) or not. Would you mind providing your values?

pimvandenbroek commented 2 days ago

The relevant values:

backends:
    - netbox.authentication.RemoteUserBackend
    - social_core.backends.google.GoogleOAuth2

This should still work, as the important part (for us) is in the configmap: REMOTE_AUTH_BACKEND: {{ toJson .Values.remoteAuth.backends }}

LeoColomb commented 2 days ago

I can't reproduce, no error when templating and the output looks fine.

$ helm template charts/netbox --values 376-values.yaml | grep 'REMOTE_AUTH_BACKEND'
    REMOTE_AUTH_BACKEND: ["netbox.authentication.RemoteUserBackend","social_core.backends.google.GoogleOAuth2"]

Where 376-values.yaml:

remoteAuth:
  enabled: true
  backends:
      - netbox.authentication.RemoteUserBackend
      - social_core.backends.google.GoogleOAuth2
pimvandenbroek commented 2 days ago

You are right. After some additional checking, I've found the following to be the culprit:

extraConfig:
  - secret: # same as pod.spec.volumes.secret
      secretName: "netbox-google-sso-dev"
  - values:
      SOCIAL_AUTH_GOOGLE_OAUTH2_WHITELISTED_DOMAINS: ["domain.com"]

Not sure if we're trying to implement it incorrectly, but in 112 it worked correctly.

LeoColomb commented 2 days ago

Should be fixed via #377