MON-3701: clean-up injection of trusted CA bundle for main Alertmanager #2310
Conversation
simonpasquier
commented
Apr 10, 2024
•
simonpasquier
commented
- I added CHANGELOG entry for this change.
- No user facing changes, so no entry in CHANGELOG was needed.
openshift-ci-robot
added
the
jira/valid-reference
|
@simonpasquier: This pull request references MON-3701 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 task to target the "4.16.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
added
the
do-not-merge/work-in-progress
openshift-ci
bot
added
the
approved
simonpasquier
force-pushed
the
MON-3701-trustedCA
branch
from
820f3d8
to
fc13b53
Compare
|
/retest-required |
1 similar comment
|
/retest-required |
|
/skip |
simonpasquier
force-pushed
the
MON-3701-trustedCA
branch
from
fc13b53
to
b39666c
Compare
|
/test e2e-aws-ovn-upgrade |
simonpasquier
changed the title
simonpasquier
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
CMO needed to inject the hashed version of the trusted CA bundle ConfigMap into the Alertmanager resource when OAuth proxy was used because it couldn't detect updates to the bundle and reload it. The trusted CA bundle is still required to be mounted into the pod for the Alertmanager container but it is declared directly in the static manifest instead of being injected at runtime by CMO since Alertmanager reloads the CA whenever it changes. Signed-off-by: Simon Pasquier <spasquie@redhat.com>
simonpasquier
force-pushed
the
MON-3701-trustedCA
branch
from
b39666c
to
ca92ea3
Compare
|
/skip |
|
@simonpasquier: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
/skip |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
openshift-ci
bot
removed
the
do-not-merge/hold
| factory: t.factory, | ||
| prefix: "alertmanager", | ||
| } | ||
| trustedCA, err = cbs.syncTrustedCABundle(ctx, trustedCA) |
There was a problem hiding this comment.
Cool that we're simplifying code like this.
I had a question about a similar practice (generating secrets with hashes...) here #2293 (comment)
and it'd be great if we can get rid of the logic there as well and only use the "secret/config hash on Deployment's template" approach as simpler.
I have a question about the root CA hot-reloading though, I know KRP supports that, but I think it only concerns --tls-cert-file and --tls-private-key-file, I think root CA from /etc/pki/ca-trust/extracted/pem/ etc. are managed by Go itself (in the case of KRP) and according to golang/go#41888 they're not reloaded.
There was a problem hiding this comment.
kube-rbac-proxy doesn't rely on the cluster CA bundle. Here is the CA bundle is mounted into the Alertmanager container because it may be configured to send notifications to 3rd party services that use certificates generated by the cluster CA.
There was a problem hiding this comment.
Ah, ok. I don't know if alertmanager supports hot-reloading then.
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: machine424, simonpasquier 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 |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
openshift-merge-bot
bot
merged commit 8dc9c77
into
openshift:master
