fix: reload openshift csr-signer ca certificate in Operator trusted CA's #1742
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pjuarezd
commented
Aug 25, 2023
harshavardhana
previously approved these changes
Aug 25, 2023
pjuarezd
force-pushed
the
openshift-reload-controlplane-ca
branch
from
609fa94
to
1710578
Compare
harshavardhana
requested review from
vadmeste,
harshavardhana,
cniackz,
shtripat and
jiuker
harshavardhana
previously approved these changes
Aug 27, 2023
shtripat
previously approved these changes
Aug 28, 2023
|
|
||
| install_operator "certified-operators" "minio-operator" | ||
|
|
||
| #destroy_crc |
There was a problem hiding this comment.
Should we enable this line before merge?
There was a problem hiding this comment.
not for the moment, it was just causing disruption during development running on a personal device, github actions do not have enough power to run this therefore not automated.
Once we automatize the Openshift tests on the DC probably.
…the certificate in a local secret and reload the CA certificates * Add openshift test to deploy Operator from OperatorHub on minio-operator namespace * Add empty `securityContext` and `containerSecurityContext` in the Openshift kustomization example. * Fix to allow the tenant run with locally build operator image in crc test Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
pjuarezd
force-pushed
the
openshift-reload-controlplane-ca
branch
from
2c91aad
to
bae8e08
Compare
|
Thank you for your reviews @harshavardhana @shtripat, this is now ready to merge |
harshavardhana
approved these changes
Aug 29, 2023
|
fixes #1671 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the problem?
In Openshift the certificates generated using CertificateSigningRequests (CSR) are signed by a Certificate Authority (CA) in the control plane. The CA is stored on
csr-signersecret onopenshift-kube-controller-manager-operatornamespace, this CA certificate rotates every month.The certificates generated for the minio tenants when the
spec.requestAutocert: trueis set, are generated using CSR's, hence signed by thecsr-signerin the cluster.Operator rotates the minio tenant certificates when they are about to expire (80% of usage), which is good and expected.
The problem consists in the Operator being unable to trust the certificate presented by the minio tenant on the healt check cycles because a newly rotated minio tenant certificate is signed by a new
csr-signerCA, one that Operator doesn't know about.Operator was only loading the certificate when the pod was launched, so when the
csr-signerwas rotated, Operator was not aware of it.How the fix works
In the fix proposed in this PR, Operator will compare the certificate
tls.crtin thecsr-signersecret located in theopenshift-kube-controller-manager-operatornamespace with a copy of this certificate that we store in theopenshift-csr-signer-casecret in the operator namespace, if the certificate is different then we update the secret in the operator namespace.A layover specific only for Openshift will mount this secret in the operator deployment as an Optional secret and operator controller will load the CA on the pod start.
This approach have some benefits:
Mounting the secret will make sure that the deployment will restart the pods whenever the secret gets updated, therefore Operator will load the latest CA alwaysUpdate: Turns out that in practice the pods seemly are not being restarted when the secret gets modified (weird), so a a reload of the CA certificates whenever the secret changed was implemented.
We are planning to stop using CSR's to generate the tenant certificates and instead use
service-cato generate certificates using annotations as suggested in Securing service traffic using service serving certificate secrets docs, however... load the latestcsr-signerCA certificate in Operator will make sure that any certificate issued within the cluster using CSR can be trusted, either a different minio instance or third party services.Closes #1688