Reload certs 2237903 watch #7502
Conversation
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
from
f90676f
to
9dbf893
Compare
alphaprinz
changed the title
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
from
0ffb85b
to
9627ae2
Compare
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
2 times, most recently
from
3b806e3
to
4aea377
Compare
alphaprinz
changed the title
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
2 times, most recently
from
cc00f24
to
f6b1c3e
Compare
| } | ||
| return false; | ||
| try { | ||
| fs.watch(cert_info.dir, {}, cert_info.file_notification.bind(cert_info)); |
There was a problem hiding this comment.
shouldn't we avoid this for self-generated? Or maybe check in advance if the dir exists and then we will skip both reading the files and setting the watch.
There was a problem hiding this comment.
Personally I like that we can still update self-generated certs to "real" certs in containerized env, though I admit it's not a practical scenario.
At any rate, this is the rule-of-thumb I followed-
For containerized env, the directory should always exists.
Non-containerized envs are not relevant for this PR.
src/util/ssl_utils.js
Outdated
| if (err.code === 'ENOENT') { | ||
| dbg.warn("Certificate folder ", cert_info.dir, " does not exist. New certificate won't be loaded."); | ||
| } else { | ||
| dbg.error("Failed to watch certificate dir ", cert_info.dir); |
| fs.watch(cert_info.dir, {}, cert_info.file_notification.bind(cert_info)); | ||
| } catch (err) { | ||
| if (err.code === 'ENOENT') { | ||
| dbg.warn("Certificate folder ", cert_info.dir, " does not exist. New certificate won't be loaded."); |
There was a problem hiding this comment.
another reason to maybe check for the directory in advance and to handle this error there. although not sure why this split, the level of the print?
There was a problem hiding this comment.
I don't understand.
This is the only place non-existing directory is handled.
If you think there's still a problem despite what I wrote in previous comment (#7502 (comment)), let me know.
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
from
f6b1c3e
to
89cb829
Compare
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
from
89cb829
to
a86c782
Compare
Signed-off-by: Amit Prinz Setter <alphaprinz@gmail.com>
alphaprinz
force-pushed
the
reload_certs_2237903_watch
branch
from
a86c782
to
9484f46
Compare
Explain the changes
The certificate for S3 endpoint can change.
Endpoint now watches for changes in certificate file.
If so, the https servers are updated with new certificate.
(Endpoint pod doesn't need to be restarted).
Issues: Fixed #xxx / Gap #xxx
(https://bugzilla.redhat.com/show_bug.cgi?id=2237903)
Testing Instructions:
Note currently used certificate, eg-
openssl s_client -connect localhost:10443 -showcerts 2>/dev/null </dev/null | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' | openssl x509 -text -noout
Create a new certificate, eg-
openssl req -x509 -newkey rsa:4096 -keyout key.pem -out cert.pem -sha256 -days 3650 -nodes -subj "/C=XX/ST=StateName/L=CityName/O=CompanyName/OU=CompanySectionName/CN=CommonNameOrHostname"
From here, follow instructions in https://github.com/noobaa/noobaa-operator/blob/master/doc/ssl-dns-routing.md:
Delete current secret (if exists), eg-
kubectl delete secret noobaa-s3-serving-cert
Create a new secret from the new certificate. Note filenames must be tls.crt and tls.key-
kubectl create secret generic noobaa-s3-serving-cert --from-file=tls.crt --from-file=tls.key
Wait until new files are created in endpoint's pod /etc/s3-secret.
Wait until background worker runs.
Running openssl -showcerts now should have the new certificate.