Add TLS support between Ironic and BMO #631
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maelk The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test-integration |
There was a problem hiding this comment.
I think there is a confusion around CA cert vs cert. Unless you want to establish a proper chain OR use client certificates, you don't need 3 files. You only need a private key (passed to ironic as a secret) and a certificate (used everywhere else). I suggest we limit at least the initial proposal to this simplest case to avoid confusion. I'd also prefer we generate certificates rather than make an operator do it.
There was a problem hiding this comment.
Let me clarify a bit our scenario. We do not want to do client authentication, that is out of the picture, so as @dtantsur pointed out, we do not need the CA in Ironic, just the certs. However, we want to deploy ironic using cert-manager to generate and rotate the certificates used, based on a CA given by the user (or automatically generated). In that case, BMO should accept any certificate signed by that CA, and not only the current certificate, keeping in mind that there isn't any tight link between ironic and BMO anymore, they could even be deployed in different clusters. Hence, not taking the specific certificate, but the CA, allows for not having to re-deploy BMO when cert-manager rotates the ironic certificate.
|
/test-integration |
|
I have now modified the ironic images. we are providing the certificate and the key for ironic and inspector, and in addition the CA to validate the certificate to both of them so that they can validate each other's certificate . we provide that certificate to BMO to validate the TLS connection with ironic and inspector. @dtantsur and @dhellmann please take a look |
Got it, thank you for explanation. |
|
/test-integration |
|
/test-integration |
|
/test-integration |
|
/test-integration |
|
/test-integration |
|
lgtm |
|
lgtm |
42e9eba to
3c83cc1
Compare
|
/test-integration |
|
/test-integration |
81af46b to
6e59ff4
Compare
|
/test-integration |
|
/test-integration |
1 similar comment
|
/test-integration |
|
I would like to freeze go code changes for a few days to try to land #650 without having to rebase it, because rebasing will mean redoing the work from scratch. /hold |
|
#655 has merged /hold cancel |
|
/test-integration |
|
We will address the change to the config folder, when we work on the metal3-dev-env adaptation. We currently do not have ways to test it in the CI otherwise. |
If IRONIC_CACERT_FILE is set as environment variable, BMO will use that file as CA certificate when connecting to Ironic.
This commit adds the possibility to deploy Ironic and BMO with TLS enabled
|
/test-integration |
This PR introduces support for TLS using a custom CA certificate in BMO and modifies the deployment file to be able to deploy Ironic, inspector and BMO with TLS enabled.
This requires metal3-io/ironic-image#198 and metal3-io/ironic-inspector-image#62