Add support for bringing your own LDAP CA certificate

[devasmith]

This PR support for custom CA certificates when using LDAP authentication. Users can now provide their own CA certificates in the caCertData value, which will be mounted as a file in the container. As this is my first contribution to this project, I am open to any feedback or suggestions for improvement.

Closes #119

[LeoColomb]

Thanks for this PR @devasmith.
The idea sounds good (I'll let @RangerRick comment on that if needed).
That being said, the certificate must be stored using the Secret API from Kubernetes, like any potentially credential.
Checkout the documentation: https://kubernetes.io/docs/concepts/configuration/secret/

That would probably drop the need for a value for the path as well.

[devasmith]

Thanks for this PR @devasmith. The idea sounds good (I'll let @RangerRick comment on that if needed). That being said, the certificate must be stored using the Secret API from Kubernetes, like any potentially credential. Checkout the documentation: https://kubernetes.io/docs/concepts/configuration/secret/

That would probably drop the need for a value for the path as well.

Ok cheers! I thought for this purpose that the CA isn't sensitive, I'll take a look at moving it into secrets instead.

[bootc]

That being said, the certificate must be stored using the Secret API from Kubernetes, like any potentially credential.

I agree with @devasmith, I don't think certificates necessarily need to be kept in Secrets at all - I would have thought a ConfigMap was perfectly fine. They contain only public keys after all.

[RangerRick]

seems aright to me

[RangerRick]

That being said, the certificate must be stored using the Secret API from Kubernetes, like any potentially credential.

I agree with @devasmith, I don't think certificates necessarily need to be kept in Secrets at all - I would have thought a ConfigMap was perfectly fine. They contain only public keys after all.

Agreed, a secret is overkill since this is just public cert data. If you were doing client certs for MITM then you'd want to use a kubernetes.io/tls Secret but that's not necessary for just this.

[RangerRick]

On the other hand, the tests failed. ;)

[devasmith]

On the other hand, the tests failed. ;)

I noticed. This works for example without any other options for ldap.

  {{- if .Values.remoteAuth.ldap }}
  ldap_ca.crt: |-
    {{ .Values.remoteAuth.ldap.caCertData | nindent 2 }}
  {{- end }}

It also works if I have it under {{- if eq . "netbox.authentication.LDAPBackend" }}

  {{- if .Values.remoteAuth.ldap.caCertData }}
  ldap_ca.crt: |-
    {{ .Values.remoteAuth.ldap.caCertData }}
  {{- end }}

[RangerRick]

this looks like it should work, to me...

@LeoColomb if you agree, I'd say go ahead and merge it

Not blocking

[LeoColomb]

@RangerRick Well, it's up to you at the end on this one 😊
I still consider Secrets a much more future-proof storage for external data, especially when involving security parameters that:

• Can require rotation relatively regularly
• Might be shared with more than one app/deployment (i.e., more than just Netbox)
• Could be injected using secret operators and other vaults

Might be overkill, but users are pushing to do it that way, for example #185 and #160. It makes sense to me, you would rather not update all apps/charts values, one by one, manually every time your IT decides to rotate the certs.

Again, at the end you're probably more experienced than me for LDAP, so up to you 😊

[devasmith]

I’d like to share our specific use case to provide some context for our approach. We use Flux to manage our GitOps flow. By embedding configuration directly in the HelmRelease files, we streamline the process, as it allows us to update configurations without separately managing secrets. This method ensures that the application automatically reloads with the new configuration whenever the HelmRelease is updated.

If we were to use secrets instead, we would need an additional mechanism to trigger a reload of the application when a secret value is updated, such as a configuration reloader or file watcher.

[LeoColomb]

@devasmith Thanks for your reply. That is a very valid workflow. Storing in ConfigMap or Secret won't change the fact the value can still be defined in the values.yaml or in the HelmRelease.
From this perspective, the trigger will be the same.

[RangerRick]

I'll go ahead and approve it. If we want to deal with also allowing secrets or other cert stuff later, we can always add it, but this is a valid use-case as-is.