NE-761: Support for admin configured CA trust bundle in Ingress Operator #1514
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@bharath-b-rh: This pull request references NE-761 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@ShudiLi please also help review when you get a chance. Thanks |
|
/assign |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@candita Thank you for the review! |
bharath-b-rh
force-pushed
the
cfe-974
branch
2 times, most recently
from
d5a933c to
88bc293
Compare
There was a problem hiding this comment.
I like the overall design. Key points in my review:
- The name
admin-ca-bundledoesn't provide any hint that the ConfigMap is related to the router. - I'd like a better understanding of what could happen if the cluster-admin provided an invalid CA certificate.
- Some of the later sections in the EP need more details.
- We need clarity around the behavior with respect to multiple IngressControllers. Candace and I came away with different understandings of what you are proposing, so some clarification is needed.
Other than that, my comments are mostly small wording suggestions.
|
|
||
| #### Support Procedures | ||
|
|
||
| N/A. |
There was a problem hiding this comment.
Some ideas:
- If the route isn't working, check router logs for errors from HAProxy.
- Check the
DEFAULT_DESTINATION_CA_PATHenvironment variable. - Verify that the CA bundle has the expected certificate using
cat "$DEFAULT_DESTINATION_CA_PATH"oropenssl x509 -noout -text -in "$DEFAULT_DESTINATION_CA_PATH". - Curl the backend server from the router pod using
curl --cacert "$DEFAULT_DESTINATION_CA_PATH" https://<pod IP address>or whatever. - If all else fails and the router is broken, undo the config using
oc -n openshift-config delete configmaps/admin-ca-bundle.
There was a problem hiding this comment.
section updated.
|
|
||
| ## Alternatives | ||
|
|
||
| N/A. |
There was a problem hiding this comment.
The discussion on the RFE includes a suggestion to make the router trust the CA specified in the cluster proxy config's trustedCA field. This suggestion was rejected because we don't want to conflate chains of trust. That's the kind of thing I expect would be mentioned in the EP as an alternative that was considered and rejected.
Other alternatives could be to tell people just to set destinationCACertificate or to use cert-manager, or let the cluster-admin replace the service CA. The EP should explain why these alternatives are impractical.
There was a problem hiding this comment.
section updated.
| simpler alternative: a feature in which an administrator could define a CA bundle for verifying | ||
| certificates from a ConfigMap consisting of the list of CA certificates, associated OpenShift | ||
| Ingress Controller, and trusted OpenShift CA. |
There was a problem hiding this comment.
The example ConfigMap doesn't appear to specify an IngressController, so I don't know what you mean by "a ConfigMap consisting of the list of CA certificates, associated OpenShift Ingress Controller, [...]". The current design appears to allow the cluster-admin to specify only a single a trust bundle that applies to all IngressControllers; am I right?
To be clear, I like the current design; I just find this sentence confusing.
(One thing I like about the design proposed in this EP is that it leaves open the possibility of adding a field in the IngressController API to specify the default destination CA trust bundle. This field's default value could be "admin-ca-bundle" (or whatever name we ultimately use in this EP) for backwards-compatibility.)
There was a problem hiding this comment.
Sentences have got overlapped when I was apply the changes, updated it.
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
openshift-ci
bot
removed
the
lifecycle/stale
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
|
@openshift-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/reopen |
|
@bharath-b-rh: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@bharath-b-rh: This pull request references NE-761 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
removed
the
lifecycle/rotten
|
@bharath-b-rh: The In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
/remove-lifecycle stale |
|
/remove-lifecycle rotten |
openshift-ci
bot
removed
the
lifecycle/rotten
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
PR is an enhancement proposal to add support for admin configured CA trust bundle in Ingress Operator.
Enhancement proposes to have a provision for the OpenShift administrator to configure a CA trust bundle to be used as default destination CA along with OpenShift service CA for verifying service's certificate by the OpenShift router when the created route is having re-encrypt termination type.