From da1c55ad85f569c1e442e1b6233ea00b07026338 Mon Sep 17 00:00:00 2001 From: Oleg Bulatov Date: Thu, 1 Dec 2022 18:10:12 +0100 Subject: [PATCH 1/6] Add Additional trusted certificate authorities for image registries --- ...ficate-authorities-for-image-registries.md | 185 ++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 enhancements/machine-config/certificate-authorities-for-image-registries.md diff --git a/enhancements/machine-config/certificate-authorities-for-image-registries.md b/enhancements/machine-config/certificate-authorities-for-image-registries.md new file mode 100644 index 0000000000..ee6213c153 --- /dev/null +++ b/enhancements/machine-config/certificate-authorities-for-image-registries.md @@ -0,0 +1,185 @@ +--- +title: certificate-authorities-for-image-registries +authors: + - dmage +reviewers: # Include a comment about what domain expertise a reviewer is expected to bring and what area of the enhancement you expect them to focus on. For example: - "@networkguru, for networking aspects, please look at IP bootstrapping aspect" + - TBD +approvers: + - TBD +api-approvers: # In case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers). If there is no API change, use "None" + - TBD +creation-date: 2022-12-01 +last-updated: 2022-12-01 +tracking-link: + - https://issues.redhat.com/browse/IR-230 +see-also: +replaces: +superseded-by: +--- + +# Additional trusted certificate authorities for image registries + +## Summary + +This enhancement describes how certificate authorities for image registries +should be distributed to CRI-O, openshift-apiserver, and other clients when the +image registry operator is not installed. + +## Motivation + +The image registry operator manages a daemon set that distributes certificate +authorities for the integrated image registry, and it also distributes +certificate authorities for external registries that are provided by the +cluster administrator in the image config. This makes the image registry +operator required for the cluster while the integrated image registry is +optional and can be removed. + +The goal of this enhancement is to move this required functionality to an +essential cluster operator, and have the image registry operator only manage +the optional registry workload. + +### User Stories + +* As a , I want to so that I can . + +### Goals + +* Make additionalTrustedCA be managed by machine-config-operator. +* Create a mechanism for the cluster-image-registry-operator to distribute + certificate authorities for the integrated image registry. +* Remove the node-ca daemon set. + +### Non-Goals + +* Change additionalTrustedCA behavior for its users. + +## Proposal + +Create a controller inside machine-config-operator that will replace the +node-ca daemon set. + +This controller should handle and distribute to all nodes certificate +authorities from the user-provided config map that is specified in +`images.config.openshift.io/cluster`. It should also observe a config map in +the `openshift-config-managed` namespace and merge its content with the +user-provided config map. This will allow cluster-image-registry-operator to +provide certificate authorities for the integrated image registry without +changing the user-provided config map. + +Once this controller is created, the node-ca daemon set should be removed. + +### Workflow Description + +Let's suppose the user creates a config map with the certificate authorities +for their registries: + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: registry-ca + namespace: openshift-config +data: + registry.example.com: | + -----BEGIN CERTIFICATE----- + ... + -----END CERTIFICATE----- + registry-with-port.example.com..5000: + -----BEGIN CERTIFICATE----- + ... + -----END CERTIFICATE----- +``` + +Then updates the `images.config.openshift.io/cluster` object to point to this +config map: + +```yaml +spec: + additionalTrustedCA: + name: registry-ca +``` + +If the image registry operator is installed, there should be a config map: + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: image-registry-ca + namespace: openshift-config-managed +data: + image-registry.openshift-image-registry.svc..5000: | + -----BEGIN CERTIFICATE----- + ... + -----END CERTIFICATE----- + image-registry.openshift-image-registry.svc.cluster.local..5000: | + -----BEGIN CERTIFICATE----- + ... + -----END CERTIFICATE----- +``` + +The machine-config-operator should update every node to propagate these +certificate authorities to CRI-O, so it should create these files: + +* `/etc/docker/certs.d/registry.example.com/ca.crt` +* `/etc/docker/certs.d/registry-with-port.example.com:5000/ca.crt` +* `/etc/docker/certs.d/image-registry.openshift-image-registry.svc:5000/ca.crt` +* `/etc/docker/certs.d/image-registry.openshift-image-registry.svc.cluster.local:5000/ca.crt` + +If the config maps are updated, old certificate authorities should be removed +and new ones should be added. Nodes nor applications shouldn't be restarted to +apply changes. + +There are also consumers that don't have access to the host file system: +cluster-openshift-apiserver-operator needs a merged config map with all +certificate authorities for image registries, so that openshift-apiserver can +access registries. + +For them, the controller should create a merged config map in +`openshift-config-managed`. + +### API Extensions + +TBD + +### Risks and Mitigations + +There are might be consumers that expect the merged config map with certificate +authorities in the `openshift-image-registry` namespace. The image registry +operator should copy the config map from `openshift-config-managed` to +`openshift-image-registry`. + +### Drawbacks + +TBD + +## Design Details + +### Open Questions [optional] + +### Test Plan + +### Graduation Criteria + +#### Dev Preview -> Tech Preview + +#### Tech Preview -> GA + +#### Removing a deprecated feature + +### Upgrade / Downgrade Strategy + +TBD + +### Version Skew Strategy + +### Operational Aspects of API Extensions + +#### Failure Modes + +#### Support Procedures + +## Implementation History + +## Alternatives From c99f95621433930752de2dd560a2b93d3cc22f1c Mon Sep 17 00:00:00 2001 From: Flavian Missi Date: Mon, 13 Mar 2023 11:53:13 +0100 Subject: [PATCH 2/6] add reviewer, link MCO epic, s/controller/mechanism --- ...certificate-authorities-for-image-registries.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/enhancements/machine-config/certificate-authorities-for-image-registries.md b/enhancements/machine-config/certificate-authorities-for-image-registries.md index ee6213c153..f9285ef1ca 100644 --- a/enhancements/machine-config/certificate-authorities-for-image-registries.md +++ b/enhancements/machine-config/certificate-authorities-for-image-registries.md @@ -3,15 +3,15 @@ title: certificate-authorities-for-image-registries authors: - dmage reviewers: # Include a comment about what domain expertise a reviewer is expected to bring and what area of the enhancement you expect them to focus on. For example: - "@networkguru, for networking aspects, please look at IP bootstrapping aspect" - - TBD + - @sinnykumari approvers: - - TBD + - TBD, who can serve as an approver? api-approvers: # In case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers). If there is no API change, use "None" - TBD creation-date: 2022-12-01 last-updated: 2022-12-01 tracking-link: - - https://issues.redhat.com/browse/IR-230 + - https://issues.redhat.com/browse/MCO-499 see-also: replaces: superseded-by: @@ -56,10 +56,10 @@ goal>. ## Proposal -Create a controller inside machine-config-operator that will replace the +Create a mechanism inside machine-config-operator that will replace the node-ca daemon set. -This controller should handle and distribute to all nodes certificate +This mechanism should handle and distribute to all nodes certificate authorities from the user-provided config map that is specified in `images.config.openshift.io/cluster`. It should also observe a config map in the `openshift-config-managed` namespace and merge its content with the @@ -67,7 +67,7 @@ user-provided config map. This will allow cluster-image-registry-operator to provide certificate authorities for the integrated image registry without changing the user-provided config map. -Once this controller is created, the node-ca daemon set should be removed. +Once this mechanism is created, the node-ca daemon set should be removed. ### Workflow Description @@ -136,7 +136,7 @@ cluster-openshift-apiserver-operator needs a merged config map with all certificate authorities for image registries, so that openshift-apiserver can access registries. -For them, the controller should create a merged config map in +For them, the mechanism should create a merged config map in `openshift-config-managed`. ### API Extensions From 77e38ada5a01ab3fde19516202dfde50cd7514c3 Mon Sep 17 00:00:00 2001 From: Scott Dodson Date: Thu, 13 Apr 2023 14:21:56 -0400 Subject: [PATCH 3/6] Update enhancements/machine-config/certificate-authorities-for-image-registries.md --- .../certificate-authorities-for-image-registries.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/enhancements/machine-config/certificate-authorities-for-image-registries.md b/enhancements/machine-config/certificate-authorities-for-image-registries.md index f9285ef1ca..14c0260082 100644 --- a/enhancements/machine-config/certificate-authorities-for-image-registries.md +++ b/enhancements/machine-config/certificate-authorities-for-image-registries.md @@ -5,7 +5,8 @@ authors: reviewers: # Include a comment about what domain expertise a reviewer is expected to bring and what area of the enhancement you expect them to focus on. For example: - "@networkguru, for networking aspects, please look at IP bootstrapping aspect" - @sinnykumari approvers: - - TBD, who can serve as an approver? + - @sdodson + - @mrunalp api-approvers: # In case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers). If there is no API change, use "None" - TBD creation-date: 2022-12-01 From 30a4e6564fc3d17f1366682e8b39d6e983b3d9e7 Mon Sep 17 00:00:00 2001 From: Oleg Bulatov Date: Thu, 20 Apr 2023 11:40:35 +0200 Subject: [PATCH 4/6] Fix linter error --- .../certificate-authorities-for-image-registries.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/enhancements/machine-config/certificate-authorities-for-image-registries.md b/enhancements/machine-config/certificate-authorities-for-image-registries.md index 14c0260082..8b2edd5ed9 100644 --- a/enhancements/machine-config/certificate-authorities-for-image-registries.md +++ b/enhancements/machine-config/certificate-authorities-for-image-registries.md @@ -1,12 +1,12 @@ --- title: certificate-authorities-for-image-registries authors: - - dmage + - "@dmage" reviewers: # Include a comment about what domain expertise a reviewer is expected to bring and what area of the enhancement you expect them to focus on. For example: - "@networkguru, for networking aspects, please look at IP bootstrapping aspect" - - @sinnykumari + - "@sinnykumari" approvers: - - @sdodson - - @mrunalp + - "@sdodson" + - "@mrunalp" api-approvers: # In case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers). If there is no API change, use "None" - TBD creation-date: 2022-12-01 From eb3dbb6e646243864a3d514b30a09eb77d23752c Mon Sep 17 00:00:00 2001 From: Oleg Bulatov Date: Thu, 20 Apr 2023 11:47:10 +0200 Subject: [PATCH 5/6] Update enhancements/machine-config/certificate-authorities-for-image-registries.md Co-authored-by: Flavian Missi --- .../certificate-authorities-for-image-registries.md | 1 + 1 file changed, 1 insertion(+) diff --git a/enhancements/machine-config/certificate-authorities-for-image-registries.md b/enhancements/machine-config/certificate-authorities-for-image-registries.md index 8b2edd5ed9..b3f8fa2085 100644 --- a/enhancements/machine-config/certificate-authorities-for-image-registries.md +++ b/enhancements/machine-config/certificate-authorities-for-image-registries.md @@ -13,6 +13,7 @@ creation-date: 2022-12-01 last-updated: 2022-12-01 tracking-link: - https://issues.redhat.com/browse/MCO-499 + - https://issues.redhat.com/browse/IR-351 see-also: replaces: superseded-by: From d3e05862fca9de5ed9a8de57a631f5e949340cd8 Mon Sep 17 00:00:00 2001 From: Oleg Bulatov Date: Thu, 20 Apr 2023 11:48:28 +0200 Subject: [PATCH 6/6] Add api-approvers --- .../certificate-authorities-for-image-registries.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/enhancements/machine-config/certificate-authorities-for-image-registries.md b/enhancements/machine-config/certificate-authorities-for-image-registries.md index b3f8fa2085..daac97d5bf 100644 --- a/enhancements/machine-config/certificate-authorities-for-image-registries.md +++ b/enhancements/machine-config/certificate-authorities-for-image-registries.md @@ -8,7 +8,8 @@ approvers: - "@sdodson" - "@mrunalp" api-approvers: # In case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers). If there is no API change, use "None" - - TBD + - "@sdodson" + - "@mrunalp" creation-date: 2022-12-01 last-updated: 2022-12-01 tracking-link: