Bug 1771773: webhooks: Rely on service-ca-operator for CA injection #132
Conversation
This relies on the recently added support in the service-ca-operator for automatically injecting its CA certificate into the webhook configurations. We no longer need to have the CA injected into a ConfigMap that we mount into the operator's pod, and rotation of the CA should be handled automatically.
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@bison: This pull request references Bugzilla bug 1771773, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 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. |
openshift-ci-robot
added
the
size/L
| const WebhookConfigurationName = "autoscaling.openshift.io" | ||
|
|
||
| // InjectCABundleAnnotationName is the annotation used by the | ||
| // service-ca-operator to indicate which resources it should inject the CA into. | ||
| const InjectCABundleAnnotationName = "service.beta.openshift.io/inject-cabundle" |
There was a problem hiding this comment.
Might it be worth using the const from the service-ca-operator so that if it changes there, it's updated when we update the dependency?
https://github.com/openshift/service-ca-operator/blob/61c93f3832ad2acb8cef1f4e0088d4c06965a47f/pkg/controller/api/api.go#L20
There was a problem hiding this comment.
Pulling in an entire new dependency for one constant, and still having to take manual action to update it if it changes doesn't seem worth it to me.
There was a problem hiding this comment.
+1 to just defining it here. Also, since the name of the annotation is supported for customers, it is effectively a stable API and any changes would follow a reasonable deprecation cycle.
|
/hold Needs testing on a recent 4.4 cluster. |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel Works for me on latest 4.4 nightly. |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre 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 |
openshift-ci-robot
added
the
approved
|
@bison: All pull requests linked via external trackers have merged. Bugzilla bug 1771773 has been moved to the MODIFIED state. 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. |
This relies on the recently added support in the service-ca-operator
for automatically injecting its CA certificate into the webhook
configurations. We no longer need to have the CA injected into a
ConfigMap that we mount into the operator's pod, and rotation of the
CA should be handled automatically.
See: openshift/service-ca-operator#79