diff --git a/cmd/machine-config-operator/start.go b/cmd/machine-config-operator/start.go index 08bb5fbe51..442e779367 100644 --- a/cmd/machine-config-operator/start.go +++ b/cmd/machine-config-operator/start.go @@ -92,6 +92,7 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.OpenShiftKubeAPIServerKubeNamespacedInformerFactory.Core().V1().ConfigMaps(), ctrlctx.KubeInformerFactory.Core().V1().Nodes(), ctrlctx.KubeMAOSharedInformer.Core().V1().Secrets(), + ctrlctx.ConfigInformerFactory.Config().V1().Images(), ) ctrlctx.NamespacedInformerFactory.Start(ctrlctx.Stop) diff --git a/lib/resourcemerge/machineconfig.go b/lib/resourcemerge/machineconfig.go index f4185d21fc..d972054dd8 100644 --- a/lib/resourcemerge/machineconfig.go +++ b/lib/resourcemerge/machineconfig.go @@ -82,6 +82,15 @@ func ensureControllerConfigSpec(modified *bool, existing *mcfgv1.ControllerConfi setBytesIfSet(modified, &existing.KubeAPIServerServingCAData, required.KubeAPIServerServingCAData) setBytesIfSet(modified, &existing.CloudProviderCAData, required.CloudProviderCAData) + if required.ImageRegistryBundleData != nil && !equality.Semantic.DeepEqual(existing.ImageRegistryBundleData, required.ImageRegistryBundleData) { + *modified = true + existing.ImageRegistryBundleData = required.ImageRegistryBundleData + } + + if required.ImageRegistryBundleUserData != nil && !equality.Semantic.DeepEqual(existing.ImageRegistryBundleUserData, required.ImageRegistryBundleUserData) { + *modified = true + existing.ImageRegistryBundleUserData = required.ImageRegistryBundleUserData + } if required.Infra != nil && !equality.Semantic.DeepEqual(existing.Infra, required.Infra) { *modified = true existing.Infra = required.Infra diff --git a/manifests/controllerconfig.crd.yaml b/manifests/controllerconfig.crd.yaml index 72f723d9db..f2d2c63e5f 100644 --- a/manifests/controllerconfig.crd.yaml +++ b/manifests/controllerconfig.crd.yaml @@ -170,6 +170,34 @@ spec: description: images is map of images that are used by the controller to render templates under ./templates/ type: object + imageRegistryBundleUserData: + description: imageRegistryBundleUserData is Image Registry Data provided by the user. This is pulled from image.config.openshift.io/cluster which contains an additionalCA field. + items: + description: ImageRegistryBundle contains the file and data for each CA + properties: + file: + description: file of the CA + type: string + data: + description: data of the CA + type: string + format: byte + type: object + type: array + imageRegistryBundleData: + description: imageRegistryBundleData is Image Registry Data provided by the openshift-config-managed/image-registry-ca configmap. + items: + description: ImageRegistryBundle contains the file and data for each CA + properties: + file: + description: file of the CA + type: string + data: + description: data of the CA + type: string + format: byte + type: object + type: array infra: description: infra holds the infrastructure details nullable: true diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/types.go b/pkg/apis/machineconfiguration.openshift.io/v1/types.go index 85f9b5847b..c0b2057fdd 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/types.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/types.go @@ -63,6 +63,12 @@ type ControllerConfigSpec struct { // +nullable AdditionalTrustBundle []byte `json:"additionalTrustBundle"` + // imageRegistryBundleUserData is Image Registry Data provided by the user + ImageRegistryBundleUserData []ImageRegistryBundle `json:"imageRegistryBundleUserData"` + + // imageRegistryBundleData is the ImageRegistryData + ImageRegistryBundleData []ImageRegistryBundle `json:"imageRegistryBundleData"` + // TODO: Investigate using a ConfigMapNameReference for the PullSecret and OSImageURL // pullSecret is the default pull secret that needs to be installed @@ -113,6 +119,11 @@ type ControllerConfigSpec struct { Network *NetworkInfo `json:"network"` } +type ImageRegistryBundle struct { + File string `json:"file"` + Data []byte `json:"data"` +} + // IPFamiliesType indicates whether the cluster network is IPv4-only, IPv6-only, or dual-stack type IPFamiliesType string diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go b/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go index 19ebc88102..3adfe85b32 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go @@ -259,6 +259,20 @@ func (in *ControllerConfigSpec) DeepCopyInto(out *ControllerConfigSpec) { (*out)[key] = val } } + if in.ImageRegistryBundleData != nil { + in, out := &in.ImageRegistryBundleData, &out.ImageRegistryBundleData + *out = make([]ImageRegistryBundle, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.ImageRegistryBundleUserData != nil { + in, out := &in.ImageRegistryBundleUserData, &out.ImageRegistryBundleUserData + *out = make([]ImageRegistryBundle, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Proxy != nil { in, out := &in.Proxy, &out.Proxy *out = new(configv1.ProxyStatus) @@ -328,6 +342,14 @@ func (in *ControllerConfigStatus) DeepCopy() *ControllerConfigStatus { return out } + +func (in *ImageRegistryBundle) DeepCopyInto(out *ImageRegistryBundle) { + *out = *in + return + +} + + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ControllerConfigStatusCondition) DeepCopyInto(out *ControllerConfigStatusCondition) { *out = *in diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index a09f704093..dbf5879df3 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -7,6 +7,8 @@ import ( "encoding/json" "encoding/pem" "fmt" + "os" + "path/filepath" "reflect" "sort" "time" @@ -430,39 +432,86 @@ func updateControllerConfigCerts(config *mcfgv1.ControllerConfig) bool { names := []string{ "KubeAPIServerServingCAData", "CloudProviderCAData", "RootCAData", "AdditionalTrustBundle", } - config.Status.ControllerCertificates = []v1.ControllerCertificate{} certs := [][]byte{ config.Spec.KubeAPIServerServingCAData, config.Spec.CloudProviderCAData, config.Spec.RootCAData, config.Spec.AdditionalTrustBundle, } + newImgCerts := []v1.ControllerCertificate{} + newCtrlCerts := []v1.ControllerCertificate{} for i, cert := range certs { - for len(cert) > 0 { - b, next := pem.Decode(cert) - if b == nil { + certs := createNewCert(cert, names[i]) + if len(certs) > 0 { + modified = true + newCtrlCerts = append(newCtrlCerts, certs...) + } + } + for _, entry := range config.Spec.ImageRegistryBundleData { + names = append(names, entry.File) + certs := createNewCert(entry.Data, entry.File) + if len(certs) > 0 { + modified = true + newImgCerts = append(newImgCerts, certs...) + } + } + for _, entry := range config.Spec.ImageRegistryBundleUserData { + names = append(names, entry.File) + certs := createNewCert(entry.Data, entry.File) + if len(certs) > 0 { + modified = true + newImgCerts = append(newImgCerts, certs...) + } + } + stillExists := false + for _, cert := range config.Status.ControllerCertificates { + // skip the non-IR certs + if cert.BundleFile == "KubeAPIServerServingCAData" || cert.BundleFile == "CloudProviderCAData" || cert.BundleFile == "RootCAData" || cert.BundleFile == "AdditionalTrustBundle" { + continue + } + for _, newC := range newImgCerts { + if newC.BundleFile == cert.BundleFile { + stillExists = true break } - c, err := x509.ParseCertificate(b.Bytes) - if err != nil { - klog.Infof("Malformed Cert, not syncing") - continue + } + // need to remove old cert path if it does not still exists (only applies to img certs) + if !stillExists { + if err := os.RemoveAll(filepath.Join("/etc/docker/certs.d", cert.BundleFile)); err != nil { + klog.Infof("Could not remove old certificate: %s", filepath.Join("/etc/docker/certs.d", cert.BundleFile)) } - cert = next - config.Status.ControllerCertificates = append(config.Status.ControllerCertificates, v1.ControllerCertificate{ - Subject: c.Subject.String(), - Signer: c.Issuer.String(), - NotBefore: c.NotBefore.String(), - NotAfter: c.NotAfter.String(), - BundleFile: names[i], - }) - modified = true } - + stillExists = false } + config.Status.ControllerCertificates = append(newCtrlCerts, newImgCerts...) return modified } +func createNewCert(cert []byte, name string) []v1.ControllerCertificate { + certs := []v1.ControllerCertificate{} + for len(cert) > 0 { + b, next := pem.Decode(cert) + if b == nil { + klog.Infof("Unable to decode cert %s into a pem block. Cert is either empty or invalid.", string(cert)) + break + } + c, err := x509.ParseCertificate(b.Bytes) + if err != nil { + klog.Infof("Malformed Cert, not syncing") + continue + } + cert = next + certs = append(certs, v1.ControllerCertificate{ + Subject: c.Subject.String(), + Signer: c.Issuer.String(), + NotBefore: c.NotBefore.String(), + NotAfter: c.NotAfter.String(), + BundleFile: name, + }) + } + return certs +} + // syncControllerConfig will sync the controller config with the given key. // This function is not meant to be invoked concurrently with the same key. func (ctrl *Controller) syncControllerConfig(key string) error { diff --git a/pkg/daemon/certificate_writer.go b/pkg/daemon/certificate_writer.go index 3b07fd8a10..0f39ebc873 100644 --- a/pkg/daemon/certificate_writer.go +++ b/pkg/daemon/certificate_writer.go @@ -2,6 +2,8 @@ package daemon import ( "fmt" + "os" + "path/filepath" "time" @@ -109,6 +111,24 @@ func (dn *Daemon) syncControllerConfigHandler(key string) error { return err } + for _, CA := range controllerConfig.Spec.ImageRegistryBundleData { + if err := os.MkdirAll(filepath.Join(imageCAFilePath, CA.File), defaultDirectoryPermissions); err != nil { + return err + } + if err := writeFileAtomicallyWithDefaults(filepath.Join(imageCAFilePath, CA.File, "ca.crt"), CA.Data); err != nil { + return err + } + } + + for _, CA := range controllerConfig.Spec.ImageRegistryBundleUserData { + if err := os.MkdirAll(filepath.Join(imageCAFilePath, CA.File), defaultDirectoryPermissions); err != nil { + return err + } + if err := writeFileAtomicallyWithDefaults(filepath.Join(imageCAFilePath, CA.File, "ca.crt"), CA.Data); err != nil { + return err + } + } + annos := map[string]string{ constants.ControllerConfigResourceVersionKey: controllerConfig.ObjectMeta.ResourceVersion, } diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 08684f647d..2fda85c0c4 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -176,6 +176,8 @@ const ( configMapConfigKey = "config" configMapHashKey = "hash" + imageCAFilePath = "/etc/docker/certs.d" + // used for certificate syncing caBundleFilePath = "/etc/kubernetes/kubelet-ca.crt" diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 6122bca055..071b265d90 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -72,6 +72,7 @@ type Operator struct { syncHandler func(ic string) error + imgLister configlistersv1.ImageLister crdLister apiextlistersv1.CustomResourceDefinitionLister mcpLister mcfglistersv1.MachineConfigPoolLister ccLister mcfglistersv1.ControllerConfigLister @@ -105,6 +106,7 @@ type Operator struct { nodeListerSynced cache.InformerSynced dnsListerSynced cache.InformerSynced maoSecretInformerSynced cache.InformerSynced + imgListerSynced cache.InformerSynced // queue only ever has one item, but it has nice error handling backoff/retry semantics queue workqueue.RateLimitingInterface @@ -139,6 +141,7 @@ func New( oseKubeAPIInformer coreinformersv1.ConfigMapInformer, nodeInformer coreinformersv1.NodeInformer, maoSecretInformer coreinformersv1.SecretInformer, + imgInformer configinformersv1.ImageInformer, ) *Operator { eventBroadcaster := record.NewBroadcaster() eventBroadcaster.StartLogging(klog.Infof) @@ -172,6 +175,7 @@ func New( clusterRoleInformer.Informer(), clusterRoleBindingInformer.Informer(), mcoCmInformer.Informer(), + clusterCmInfomer.Informer(), infraInformer.Informer(), networkInformer.Informer(), mcpInformer.Informer(), @@ -186,6 +190,7 @@ func New( optr.syncHandler = optr.sync + optr.imgLister = imgInformer.Lister() optr.clusterCmLister = clusterCmInfomer.Lister() optr.clusterCmListerSynced = clusterCmInfomer.Informer().HasSynced optr.mcpLister = mcpInformer.Lister() @@ -201,6 +206,7 @@ func New( optr.nodeLister = nodeInformer.Lister() optr.nodeListerSynced = nodeInformer.Informer().HasSynced + optr.imgListerSynced = imgInformer.Informer().HasSynced optr.maoSecretInformerSynced = maoSecretInformer.Informer().HasSynced optr.serviceAccountInformerSynced = serviceAccountInfomer.Informer().HasSynced optr.clusterRoleInformerSynced = clusterRoleInformer.Informer().HasSynced diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index ba87e572a3..bf0c56154f 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -18,6 +18,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -35,6 +36,7 @@ import ( mcoResourceRead "github.com/openshift/machine-config-operator/lib/resourceread" "github.com/openshift/machine-config-operator/manifests" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" templatectrl "github.com/openshift/machine-config-operator/pkg/controller/template" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" @@ -244,6 +246,94 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { return fmt.Errorf("refusing to read images.json version %q, operator version %q", imgs.ReleaseVersion, optrVersion) } + // handle image registry certificates. + // parse these, add them to ctrlcfgspec and then handle these in the daemon write to disk function + cfg, err := optr.imgLister.Get("cluster") + if err != nil { + return err + } + imgRegistryUsrData := []v1.ImageRegistryBundle{} + if cfg.Spec.AdditionalTrustedCA.Name != "" { + cm, err := optr.clusterCmLister.ConfigMaps("openshift-config").Get(cfg.Spec.AdditionalTrustedCA.Name) + if err != nil { + klog.Warningf("could not find configmap specified in image.config.openshift.io/cluster with the name %s", cfg.Spec.AdditionalTrustedCA.Name) + } else { + for key, d := range cm.Data { + raw, err := base64.StdEncoding.DecodeString(d) + if err != nil { + imgRegistryUsrData = append(imgRegistryUsrData, v1.ImageRegistryBundle{ + File: key, + Data: []byte(d), + }) + } else { + imgRegistryUsrData = append(imgRegistryUsrData, v1.ImageRegistryBundle{ + File: key, + Data: raw, + }) + } + } + for key, bd := range cm.BinaryData { + imgRegistryUsrData = append(imgRegistryUsrData, v1.ImageRegistryBundle{ + File: key, + Data: bd, + }) + } + } + } + + imgRegistryData := []v1.ImageRegistryBundle{} + cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("image-registry-ca") + if err == nil { + for key, d := range cm.Data { + raw, err := base64.StdEncoding.DecodeString(d) + if err != nil { + imgRegistryData = append(imgRegistryData, v1.ImageRegistryBundle{ + File: key, + Data: []byte(d), + }) + } else { + imgRegistryData = append(imgRegistryData, v1.ImageRegistryBundle{ + File: key, + Data: raw, + }) + } + } + for key, bd := range cm.BinaryData { + imgRegistryData = append(imgRegistryData, v1.ImageRegistryBundle{ + File: key, + Data: bd, + }) + } + } + + mergedData := append(imgRegistryData, imgRegistryUsrData...) + caData := make(map[string]string, len(mergedData)) + for _, CA := range mergedData { + caData[CA.File] = string(CA.Data) + } + + err = optr.kubeClient.CoreV1().ConfigMaps("openshift-config-managed").Delete(context.TODO(), "merged-trusted-image-registry-ca", metav1.DeleteOptions{}) + if err != nil && !errors.IsNotFound(err) { + return err + } + + cmAnnotations := make(map[string]string) + cmAnnotations["openshift.io/description"] = "Created and managed by the machine-config-operator" + _, err = optr.kubeClient.CoreV1().ConfigMaps("openshift-config-managed").Create( + context.TODO(), + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "merged-trusted-image-registry-ca", + Annotations: cmAnnotations, + }, + Data: caData, + }, + metav1.CreateOptions{}, + ) + if err != nil { + return err + } + // sync up CAs rootCA, err := optr.getCAsFromConfigMap("kube-system", "root-ca", "ca.crt") if err != nil { @@ -358,6 +448,8 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { spec.KubeAPIServerServingCAData = kubeAPIServerServingCABytes spec.RootCAData = bundle + spec.ImageRegistryBundleData = imgRegistryData + spec.ImageRegistryBundleUserData = imgRegistryUsrData spec.PullSecret = &corev1.ObjectReference{Namespace: "openshift-config", Name: "pull-secret"} spec.OSImageURL = imgs.MachineOSContent spec.BaseOSContainerImage = imgs.BaseOSContainerImage @@ -1122,6 +1214,7 @@ func mergeCertWithCABundle(initialBundle, newBundle []byte, subject string) []by for len(initialBundle) > 0 { b, next := pem.Decode(initialBundle) if b == nil { + klog.Infof("Unable to decode cert %s into a pem block. Cert is either empty or invalid.", string(initialBundle)) break } c, err := x509.ParseCertificate(b.Bytes) diff --git a/test/helpers/utils.go b/test/helpers/utils.go index 2801c14195..ef4f011fbb 100644 --- a/test/helpers/utils.go +++ b/test/helpers/utils.go @@ -373,7 +373,23 @@ func WaitForCertStatusToChange(t *testing.T, cs *framework.ClientSet, oldData st } return nil +} +func WaitForCADataToAppear(t *testing.T, cs *framework.ClientSet) error { + err := wait.PollUntilContextTimeout(context.TODO(), 2*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + controllerConfig, err := cs.ControllerConfigs().Get(context.TODO(), "machine-config-controller", metav1.GetOptions{}) + require.Nil(t, err) + nodes, err := GetNodesByRole(cs, "worker") + require.Nil(t, err) + for _, cert := range controllerConfig.Spec.ImageRegistryBundleUserData { + foundCA, _ := ExecCmdOnNodeWithError(cs, nodes[0], "ls", canonicalizeNodeFilePath(filepath.Join("/etc/docker/certs.d", cert.File))) + if strings.Contains(foundCA, "ca.crt") { + return true, nil + } + } + return false, nil + }) + return err } // WaitForMCDToSyncCert waits for the MCD to write annotation on the latest controllerconfig resourceVersion, @@ -406,6 +422,19 @@ func ForceKubeApiserverCertificateRotation(cs *framework.ClientSet) error { return err } +// ForceImageRegistryCertRotationCertificateRotation sets the imae registry cert's not-after date to nil, which causes the +// apiserver to rotate it +func ForceImageRegistryCertRotationCertificateRotation(cs *framework.ClientSet) error { + cfg, err := cs.ConfigV1Interface.Images().Get(context.TODO(), "cluster", metav1.GetOptions{}) + if err != nil { + return err + } + // Take note that the slash had to be encoded as ~1 because it's a reference: https://www.rfc-editor.org/rfc/rfc6901#section-3 + certPatch := fmt.Sprintf(`[{"op":"replace","path":"/metadata/annotations/auth.openshift.io~1certificate-not-after","value": null }]`) + _, err = cs.Secrets("openshift-config").Patch(context.TODO(), cfg.Spec.AdditionalTrustedCA.Name, types.JSONPatchType, []byte(certPatch), metav1.PatchOptions{}) + return err +} + // GetKubeletCABundleFromConfigmap fetches the latest kubelet ca bundle data from func GetKubeletCABundleFromConfigmap(cs *framework.ClientSet) (string, error) { certBundle, err := cs.ConfigMaps("openshift-config-managed").Get(context.TODO(), "kube-apiserver-client-ca", metav1.GetOptions{})