You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/08/31 21:14:28 UTC
[GitHub] [solr-operator] thelabdude opened a new pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
thelabdude opened a new pull request #318:
URL: https://github.com/apache/solr-operator/pull/318
Fixes #317
Use `fsnotify` to watch the mTLS `tls.crt` file (mounted from a secret) to hot reload the certificate for the Http client used to make calls to Solr.
For files loaded from a secret, the watched file will be a symlink that gets removed and re-added when the underlying secret changes. Thus, our watcher responds to a REMOVE event on the symlink to reload the certificate.
TODO: Need to investigate if we can just update the Certificate on the existing TLS Transport or whether we need to re-create the `http.Client` with a new `Transport` as the code is doing now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] thelabdude commented on a change in pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#discussion_r699702285
##########
File path: helm/solr-operator/templates/deployment.yaml
##########
@@ -64,6 +64,10 @@ spec:
{{- if .Values.mTLS.insecureSkipVerify }}
- --tls-skip-verify-server={{ .Values.mTLS.insecureSkipVerify }}
{{- end }}
+ {{- if .Values.mTLS.watchForUpdates }}
+ - --tls-watch-cert={{ .Values.mTLS.watchForUpdates }}
Review comment:
ha ha! good catch, was just copy-and-pasting senselessly ...
##########
File path: helm/solr-operator/templates/deployment.yaml
##########
@@ -64,6 +64,10 @@ spec:
{{- if .Values.mTLS.insecureSkipVerify }}
- --tls-skip-verify-server={{ .Values.mTLS.insecureSkipVerify }}
{{- end }}
+ {{- if .Values.mTLS.watchForUpdates }}
+ - --tls-watch-cert={{ .Values.mTLS.watchForUpdates }}
Review comment:
verified by doing:
```
helm template helm/solr-operator --set mTLS.watchForUpdates=false
```
resulting config is:
```
args:
- -zk-operator=true
- --tls-skip-verify-server=true
- --tls-watch-cert=false
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] HoustonPutman commented on a change in pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#discussion_r699685355
##########
File path: helm/solr-operator/templates/deployment.yaml
##########
@@ -64,6 +64,10 @@ spec:
{{- if .Values.mTLS.insecureSkipVerify }}
- --tls-skip-verify-server={{ .Values.mTLS.insecureSkipVerify }}
{{- end }}
+ {{- if .Values.mTLS.watchForUpdates }}
+ - --tls-watch-cert={{ .Values.mTLS.watchForUpdates }}
Review comment:
`{{- if .Values.mTLS.watchForUpdates }}` returns false for `false` values, so the only way for the if statement to be passed is if `watchForUpdates` is true. Since the default value in the operator is `true`, there's no way to set `watchForUpdates` to *false* using the helm chart.
I would probably just remove the if statement here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] thelabdude merged pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
thelabdude merged pull request #318:
URL: https://github.com/apache/solr-operator/pull/318
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] thelabdude commented on a change in pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#discussion_r699702285
##########
File path: helm/solr-operator/templates/deployment.yaml
##########
@@ -64,6 +64,10 @@ spec:
{{- if .Values.mTLS.insecureSkipVerify }}
- --tls-skip-verify-server={{ .Values.mTLS.insecureSkipVerify }}
{{- end }}
+ {{- if .Values.mTLS.watchForUpdates }}
+ - --tls-watch-cert={{ .Values.mTLS.watchForUpdates }}
Review comment:
ha ha! good catch, was just copy-and-pasting senselessly ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] thelabdude commented on pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#issuecomment-909688647
> Would be good to add a changelog entry as well.
>
> Awesome work with the fsnotify stuff. It looks like that is a BSD3 licensed library, so I think you need to include the [License text](https://github.com/fsnotify/fsnotify/blob/master/LICENSE) in the [notice-additions file](https://github.com/apache/solr-operator/blob/main/build/NOTICE-ADDITION) that generates the notice file found in the docker image.
Change log entry added
Not sure if I did the NOTICE correctly? just followed what was there ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] HoustonPutman commented on a change in pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#discussion_r699685355
##########
File path: helm/solr-operator/templates/deployment.yaml
##########
@@ -64,6 +64,10 @@ spec:
{{- if .Values.mTLS.insecureSkipVerify }}
- --tls-skip-verify-server={{ .Values.mTLS.insecureSkipVerify }}
{{- end }}
+ {{- if .Values.mTLS.watchForUpdates }}
+ - --tls-watch-cert={{ .Values.mTLS.watchForUpdates }}
Review comment:
`{{- if .Values.mTLS.watchForUpdates }}` returns false for `false` values, so the only way for the if statement to be passed is if `watchForUpdates` is true. Since the default value in the operator is `true`, there's no way to set `watchForUpdates` to *false* using the helm chart.
I would probably just remove the if statement here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] thelabdude commented on pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#issuecomment-909688647
> Would be good to add a changelog entry as well.
>
> Awesome work with the fsnotify stuff. It looks like that is a BSD3 licensed library, so I think you need to include the [License text](https://github.com/fsnotify/fsnotify/blob/master/LICENSE) in the [notice-additions file](https://github.com/apache/solr-operator/blob/main/build/NOTICE-ADDITION) that generates the notice file found in the docker image.
Change log entry added
Not sure if I did the NOTICE correctly? just followed what was there ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr-operator] thelabdude commented on a change in pull request #318: Option to watch for updates to the mTLS cert used by the operator to call Solr pods
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #318:
URL: https://github.com/apache/solr-operator/pull/318#discussion_r699711318
##########
File path: helm/solr-operator/templates/deployment.yaml
##########
@@ -64,6 +64,10 @@ spec:
{{- if .Values.mTLS.insecureSkipVerify }}
- --tls-skip-verify-server={{ .Values.mTLS.insecureSkipVerify }}
{{- end }}
+ {{- if .Values.mTLS.watchForUpdates }}
+ - --tls-watch-cert={{ .Values.mTLS.watchForUpdates }}
Review comment:
verified by doing:
```
helm template helm/solr-operator --set mTLS.watchForUpdates=false
```
resulting config is:
```
args:
- -zk-operator=true
- --tls-skip-verify-server=true
- --tls-watch-cert=false
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org