[installer] Fix server GitHub app cert mount #9799
Conversation
| @@ -201,7 +202,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) { | |||
|
|
|||
| volumeMounts = append(volumeMounts, corev1.VolumeMount{ | |||
| Name: githubAppCertSecret, | |||
| MountPath: cfg.WebApp.Server.GithubApp.CertPath, | |||
| MountPath: path.Dir(cfg.WebApp.Server.GithubApp.CertPath), | |||
There was a problem hiding this comment.
Do we validate the path somewhere? If the path is empty, path.Dir() returns . which is unlikely what we want here. https://go.dev/play/p/P9N0NNBdfdq
There was a problem hiding this comment.
Thought about that as well, but I could not come up with anything sensible here.
As a reminder, we have a single client for this anyway, and we need to ensure config stability in general at a higher level anyway, so: I don't think it make sense to add tests for properties on the strings we pipe through. 🤷
There was a problem hiding this comment.
For SaaS yes, but if this is also used by self-hosted, chances are someone will misconfigure it. Perhaps just checking that the path is not empty and erroring when it is makes sense here to avoid unexpected behaviour.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, makes sense for now then. Thanks for the clarification.
Description
Related Issue(s)
Fixes a path from #9795
How to test
Release Notes
Documentation