-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix jetstream client certificate mount path #2167
Conversation
@@ -99,7 +99,7 @@ def test_nats_statefulset_with_jetstream_and_tls(self, kube_version): | |||
} in docs[10]["spec"]["template"]["spec"]["containers"][0]["volumeMounts"] | |||
assert { | |||
"name": "nats-jetstream-client-tls-volume", | |||
"mountPath": "/usr/local/share/ca-certificates/release-name-jetstream-tls-certificate-client/ca.crt", | |||
"mountPath": "/usr/local/share/ca-certificates/release-name-jetstream-tls-certificate-client.crt", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a test with a custom nats.jetstreamSSLSecretName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this one - let me add test case for custom cert as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danielhoherd jetstreamSSLSecretName this seems require additional changes - can i address this in a seperate PR - template is not rendering with provided values and this seems like byo certficate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good so far.
Need test cases that test nats.jetstreamSSLSecretName
, maybe just parametrize the existing test case.
PR description needs to be completed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now that we've removed nats.jetstreamSSLSecretName
Description
This PR fixes an issue where mounted client side tls are not respected by the system path due long nested path.
Related Issues
https://github.com/astronomer/issues/issues/6259
Testing
QA should not see any TLS error in nats service
Merging
cherry-pick to release-0.34