From 87658629f265268e74a1e5e3664d8916aed69547 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Fri, 2 Aug 2019 15:48:43 -0400 Subject: [PATCH] Mount trusted-ca to registry base pki trust source Ensures that no other base trust is present for the registry. --- pkg/client/fake/fixtures.go | 224 +++++++++++++++++++++++++++ pkg/operator/controller.go | 1 + pkg/parameters/parameters.go | 3 + pkg/resource/podtemplatespec.go | 38 ++++- pkg/resource/podtemplatespec_test.go | 222 ++++++++++++++++++++++++++ 5 files changed, 485 insertions(+), 3 deletions(-) create mode 100644 pkg/client/fake/fixtures.go create mode 100644 pkg/resource/podtemplatespec_test.go diff --git a/pkg/client/fake/fixtures.go b/pkg/client/fake/fixtures.go new file mode 100644 index 0000000000..29cc181b4e --- /dev/null +++ b/pkg/client/fake/fixtures.go @@ -0,0 +1,224 @@ +package fake + +import ( + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/runtime" + kfake "k8s.io/client-go/kubernetes/fake" + appsv1listers "k8s.io/client-go/listers/apps/v1" + corev1listers "k8s.io/client-go/listers/core/v1" + rbacv1listers "k8s.io/client-go/listers/rbac/v1" + "k8s.io/client-go/tools/cache" + + configv1 "github.com/openshift/api/config/v1" + routev1 "github.com/openshift/api/route/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + routev1listers "github.com/openshift/client-go/route/listers/route/v1" + + regopv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" + "github.com/openshift/cluster-image-registry-operator/pkg/client" + regopv1listers "github.com/openshift/cluster-image-registry-operator/pkg/generated/listers/imageregistry/v1" +) + +// FixturesBuilder helps create an in-memory version of client.Listers. +type FixturesBuilder struct { + deploymentIndexer cache.Indexer + dsIndexer cache.Indexer + servicesIndexer cache.Indexer + secretsIndexer cache.Indexer + configMapsIndexer cache.Indexer + serviceAcctIndexer cache.Indexer + routesIndexer cache.Indexer + clusterRolesIndexer cache.Indexer + clusterRoleBindingsIndexer cache.Indexer + imageConfigsIndexer cache.Indexer + clusterOperatorsIndexer cache.Indexer + registryConfigsIndexer cache.Indexer + proxyConfigsIndexer cache.Indexer + infraIndexer cache.Indexer + + kClientSet []runtime.Object +} + +// Fixtures holds fixtures for unit testing, in forms that are easily consumed by k8s +// and OpenShift interfaces. +type Fixtures struct { + Listers *client.Listers + KubeClient *kfake.Clientset +} + +// NewFixturesBuilder initializes a new instance of FakeListersFactory +func NewFixturesBuilder() *FixturesBuilder { + factory := &FixturesBuilder{ + deploymentIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + dsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + servicesIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + secretsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + configMapsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + serviceAcctIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + routesIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + clusterRolesIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + clusterRoleBindingsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + imageConfigsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + clusterOperatorsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + registryConfigsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + proxyConfigsIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + infraIndexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}), + kClientSet: []runtime.Object{}, + } + return factory +} + +// AddDaemonSets adds appsv1.DaemonSets to the lister cache +func (f *FixturesBuilder) AddDaemonSets(objs ...*appsv1.DaemonSet) *FixturesBuilder { + for _, v := range objs { + f.dsIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddDeployments adds appsv1.Deployments to the lister cache +func (f *FixturesBuilder) AddDeployments(objs ...*appsv1.Deployment) *FixturesBuilder { + for _, v := range objs { + f.deploymentIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddNamespaces adds corev1.Namespaces to the fixture +func (f *FixturesBuilder) AddNamespaces(objs ...*corev1.Namespace) *FixturesBuilder { + for _, v := range objs { + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddServices adds corev1.Services to the lister cache +func (f *FixturesBuilder) AddServices(objs ...*corev1.Service) *FixturesBuilder { + for _, v := range objs { + f.servicesIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddSecrets adds corev1.Secrets to the lister cache +func (f *FixturesBuilder) AddSecrets(objs ...*corev1.Secret) *FixturesBuilder { + for _, v := range objs { + f.secretsIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddConfigMaps adds corev1.ConfigMaps to the lister cache +func (f *FixturesBuilder) AddConfigMaps(objs ...*corev1.ConfigMap) *FixturesBuilder { + for _, v := range objs { + f.configMapsIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddServiceAccounts adds corev1.ServiceAccounts to the lister cache +func (f *FixturesBuilder) AddServiceAccounts(objs ...*corev1.ServiceAccount) *FixturesBuilder { + for _, v := range objs { + f.serviceAcctIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddRoutes adds route.openshift.io/v1 Routes to the lister cahce +func (f *FixturesBuilder) AddRoutes(objs ...*routev1.Route) *FixturesBuilder { + for _, v := range objs { + f.routesIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddClusterRoles adds rbacv1.ClusterRoles to the lister cache +func (f *FixturesBuilder) AddClusterRoles(objs ...*rbacv1.ClusterRole) *FixturesBuilder { + for _, v := range objs { + f.clusterRolesIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddClusterRoleBindings adds rbacv1.ClusterRoleBindings to the lister cache +func (f *FixturesBuilder) AddClusterRoleBindings(objs ...*rbacv1.ClusterRoleBinding) *FixturesBuilder { + for _, v := range objs { + f.clusterRoleBindingsIndexer.Add(v) + f.kClientSet = append(f.kClientSet, v) + } + return f +} + +// AddImageConfig adds cluster-wide config.openshift.io/v1 Image to the lister cache +func (f *FixturesBuilder) AddImageConfig(config *configv1.Image) *FixturesBuilder { + f.imageConfigsIndexer.Add(config) + return f +} + +// AddClusterOperators adds config.openshift.io/v1 ClusterOperators to the lister cache +func (f *FixturesBuilder) AddClusterOperators(objs ...*configv1.ClusterOperator) *FixturesBuilder { + for _, v := range objs { + f.clusterOperatorsIndexer.Add(v) + } + return f +} + +// AddRegistryOperatorConfig adds imageregistry.operator.openshift.io/v1 Config to the lister cache +func (f *FixturesBuilder) AddRegistryOperatorConfig(config *regopv1.Config) *FixturesBuilder { + f.registryConfigsIndexer.Add(config) + return f +} + +// AddProxyConfig adds cluster-wide config.openshift.io/v1 Proxy to the lister cache +func (f *FixturesBuilder) AddProxyConfig(config *configv1.Proxy) *FixturesBuilder { + f.proxyConfigsIndexer.Add(config) + return f +} + +// AddInfraConfig adds cluster-wide config.openshift.io/v1 Infrastructure to the lister cache +func (f *FixturesBuilder) AddInfraConfig(config *configv1.Infrastructure) *FixturesBuilder { + f.infraIndexer.Add(config) + return f +} + +// Build creates the fixtures from the provided objects. +func (f *FixturesBuilder) Build() *Fixtures { + fixtures := &Fixtures{ + Listers: f.BuildListers(), + KubeClient: kfake.NewSimpleClientset(f.kClientSet...), + } + return fixtures +} + +// BuildListers creates an in-memory instance of client.Listers +func (f *FixturesBuilder) BuildListers() *client.Listers { + listers := &client.Listers{ + Deployments: appsv1listers.NewDeploymentLister(f.deploymentIndexer).Deployments("openshift-image-registry"), + DaemonSets: appsv1listers.NewDaemonSetLister(f.dsIndexer).DaemonSets("openshift-image-registry"), + Services: corev1listers.NewServiceLister(f.servicesIndexer).Services("openshift-image-registry"), + Secrets: corev1listers.NewSecretLister(f.secretsIndexer).Secrets("openshift-image-registry"), + ConfigMaps: corev1listers.NewConfigMapLister(f.configMapsIndexer).ConfigMaps("openshift-image-registry"), + ServiceAccounts: corev1listers.NewServiceAccountLister(f.serviceAcctIndexer).ServiceAccounts("openshift-image-registry"), + Routes: routev1listers.NewRouteLister(f.routesIndexer).Routes("openshift-image-registry"), + ClusterRoles: rbacv1listers.NewClusterRoleLister(f.clusterRolesIndexer), + ClusterRoleBindings: rbacv1listers.NewClusterRoleBindingLister(f.clusterRoleBindingsIndexer), + OpenShiftConfig: corev1listers.NewConfigMapLister(f.configMapsIndexer).ConfigMaps("openshift-config"), + ImageConfigs: configv1listers.NewImageLister(f.imageConfigsIndexer), + ClusterOperators: configv1listers.NewClusterOperatorLister(f.clusterOperatorsIndexer), + RegistryConfigs: regopv1listers.NewConfigLister(f.registryConfigsIndexer), + InstallerConfigMaps: corev1listers.NewConfigMapLister(f.configMapsIndexer).ConfigMaps("kube-system"), + ProxyConfigs: configv1listers.NewProxyLister(f.proxyConfigsIndexer), + Infrastructures: configv1listers.NewInfrastructureLister(f.infraIndexer), + } + return listers +} diff --git a/pkg/operator/controller.go b/pkg/operator/controller.go index de87cce993..6a0092bdc5 100644 --- a/pkg/operator/controller.go +++ b/pkg/operator/controller.go @@ -83,6 +83,7 @@ func NewController(kubeconfig *restclient.Config) (*Controller, error) { p.ImageConfig.Name = "cluster" p.CAConfig.Name = imageregistryv1.ImageRegistryCertificatesName p.ServiceCA.Name = "serviceca" + p.TrustedCA.Name = "trusted-ca" listers := ®opclient.Listers{} clients := ®opclient.Clients{} diff --git a/pkg/parameters/parameters.go b/pkg/parameters/parameters.go index 8f44f44911..9a12d66789 100644 --- a/pkg/parameters/parameters.go +++ b/pkg/parameters/parameters.go @@ -36,4 +36,7 @@ type Globals struct { ServiceCA struct { Name string } + TrustedCA struct { + Name string + } } diff --git a/pkg/resource/podtemplatespec.go b/pkg/resource/podtemplatespec.go index 63831155b0..09dd0678e3 100644 --- a/pkg/resource/podtemplatespec.go +++ b/pkg/resource/podtemplatespec.go @@ -15,7 +15,7 @@ import ( configapiv1 "github.com/openshift/api/config/v1" configlisters "github.com/openshift/client-go/config/listers/config/v1" - "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" + v1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" "github.com/openshift/cluster-image-registry-operator/pkg/storage" ) @@ -216,7 +216,7 @@ func makePodTemplateSpec(coreClient coreset.CoreV1Interface, proxyLister configl corev1.EnvVar{Name: "REGISTRY_HTTP_TLS_KEY", Value: "/etc/secrets/tls.key"}, ) - // Certificates + // Registry certificate authorities - mount as high-priority trust source anchors vol = corev1.Volume{ Name: "registry-certificates", VolumeSource: corev1.VolumeSource{ @@ -229,7 +229,39 @@ func makePodTemplateSpec(coreClient coreset.CoreV1Interface, proxyLister configl } volumes = append(volumes, vol) mounts = append(mounts, corev1.VolumeMount{Name: vol.Name, MountPath: "/etc/pki/ca-trust/source/anchors"}) - deps.AddConfigMap(vol.VolumeSource.ConfigMap.LocalObjectReference.Name) + deps.AddConfigMap(v1.ImageRegistryCertificatesName) + + // Cluster trusted certificate authorities - mount to /usr/share/pki/ca-trust-source/ to add + // CAs as low-priority trust sources. Registry runs update-ca-trust extract on startup, which + // merges the registry CAs with the cluster's trusted CAs into a single CA bundle. + // + // See man update-ca-trust for more information. + optional := true + vol = corev1.Volume{ + Name: "trusted-ca", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: params.TrustedCA.Name, + }, + // Trust bundle is in PEM format - needs to be mounted to /anchors so that + // update-ca-trust extract knows that these CAs should always be trusted. + // This also ensures that no other low-priority trust is present in the container. + // + // See man update-ca-trust for more information. + Items: []corev1.KeyToPath{ + { + Key: "ca-bundle.crt", + Path: "anchors/ca-bundle.crt", + }, + }, + Optional: &optional, + }, + }, + } + volumes = append(volumes, vol) + mounts = append(mounts, corev1.VolumeMount{Name: vol.Name, MountPath: "/usr/share/pki/ca-trust-source"}) + deps.AddConfigMap(params.TrustedCA.Name) image := os.Getenv("IMAGE") diff --git a/pkg/resource/podtemplatespec_test.go b/pkg/resource/podtemplatespec_test.go new file mode 100644 index 0000000000..e4a0399848 --- /dev/null +++ b/pkg/resource/podtemplatespec_test.go @@ -0,0 +1,222 @@ +package resource + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" + cirofake "github.com/openshift/cluster-image-registry-operator/pkg/client/fake" + "github.com/openshift/cluster-image-registry-operator/pkg/parameters" + "github.com/openshift/cluster-image-registry-operator/pkg/storage/emptydir" +) + +type volumeMount struct { + volExists bool + mountExists bool + refName string + mountPath string + items []corev1.KeyToPath + optional bool +} + +func TestMakePodTemplateSpec(t *testing.T) { + // TODO: Make this table-driven to verify all storage drivers + params := ¶meters.Globals{} + params.Deployment.Namespace = "openshift-image-registry" + params.TrustedCA.Name = "trusted-ca" + + testBuilder := cirofake.NewFixturesBuilder() + config := &v1.Config{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: v1.ImageRegistrySpec{ + Storage: v1.ImageRegistryConfigStorage{ + EmptyDir: &v1.ImageRegistryConfigStorageEmptyDir{}, + }, + }, + } + testBuilder.AddRegistryOperatorConfig(config) + + imageRegNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-image-registry", + Annotations: map[string]string{ + "openshift.io/node-selector": "", + "openshift.io/sa.scc.supplemental-groups": "1000430000/10000", + }, + Labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + }, + }, + Spec: corev1.NamespaceSpec{ + Finalizers: []corev1.FinalizerName{ + corev1.FinalizerKubernetes, + }, + }, + } + testBuilder.AddNamespaces(imageRegNs) + + fixture := testBuilder.Build() + emptyDirStorage := emptydir.NewDriver(config.Spec.Storage.EmptyDir, fixture.Listers) + pod, deps, err := makePodTemplateSpec(fixture.KubeClient.CoreV1(), fixture.Listers.ProxyConfigs, emptyDirStorage, params, config) + if err != nil { + t.Fatalf("error creating pod template: %v", err) + } + + // Verify volumes and mounts + expectedVolumes := map[string]*volumeMount{ + "registry-tls": { + refName: v1.ImageRegistryName + "-tls", + mountPath: "/etc/secrets", + }, + "registry-certificates": { + refName: v1.ImageRegistryCertificatesName, + mountPath: "/etc/pki/ca-trust/source/anchors", + }, + "trusted-ca": { + refName: params.TrustedCA.Name, + mountPath: "/usr/share/pki/ca-trust-source", + items: []corev1.KeyToPath{ + { + Key: "ca-bundle.crt", + Path: "anchors/ca-bundle.crt", + }, + }, + optional: true, + }, + } + // emptyDir adds an additional volume + expectedVolumes["registry-storage"] = &volumeMount{ + mountPath: "/registry", + } + // Verify volume mounts + for _, v := range pod.Spec.Volumes { + vol, ok := expectedVolumes[v.Name] + if !ok { + t.Errorf("volume %s was not expected", v.Name) + } else { + vol.volExists = true + verifyVolume(v, vol, t) + } + } + + // Verify registry mounts + registrySpec := pod.Spec.Containers[0] + for _, v := range registrySpec.VolumeMounts { + mount, ok := expectedVolumes[v.Name] + if !ok { + t.Errorf("volume mount %s was not expected", v.Name) + } else { + mount.mountExists = true + verifyMount(v, mount, t) + } + } + + // check volumes - exist and mounted + for name, vol := range expectedVolumes { + if !vol.volExists { + t.Errorf("volume %s was not found", name) + } + if !vol.mountExists { + t.Errorf("volume %s was not mounted", name) + } + } + + // Verify dependencies + expectedConfigMaps := map[string]bool{ + "trusted-ca": false, + "image-registry-certificates": false, + } + expectedSecrets := map[string]bool{ + "image-registry-tls": false, + } + for cm := range deps.configMaps { + if _, ok := expectedConfigMaps[cm]; !ok { + t.Errorf("unexpected dependent ConfigMap %s", cm) + } else { + expectedConfigMaps[cm] = true + } + } + for secret := range deps.secrets { + if _, ok := expectedSecrets[secret]; !ok { + t.Errorf("unexpected dependent Secret %s", secret) + } else { + expectedSecrets[secret] = true + } + } + for cm, found := range expectedConfigMaps { + if !found { + t.Errorf("ConfigMap %s was not listed as a dependency", cm) + } + } + for secret, found := range expectedSecrets { + if !found { + t.Errorf("Secret %s was not listed as a dependency", secret) + } + } + +} + +func verifyVolume(volume corev1.Volume, expected *volumeMount, t *testing.T) { + if volume.ConfigMap != nil { + if volume.ConfigMap.LocalObjectReference.Name != expected.refName { + t.Errorf("expected volume %s to reference ConfigMap %s, got %s", volume.Name, expected.refName, volume.ConfigMap.LocalObjectReference.Name) + } + if expected.optional { + if volume.ConfigMap.Optional == nil || *volume.ConfigMap.Optional != expected.optional { + t.Errorf("expected volume %s to be optional=%t, got %t", volume.Name, expected.optional, *volume.ConfigMap.Optional) + } + } + if len(expected.items) != len(volume.ConfigMap.Items) { + t.Errorf("expected volume %s to mount %d items, got %d", volume.Name, len(expected.items), len(volume.ConfigMap.Items)) + } + for i, mnt := range expected.items { + actual := volume.ConfigMap.Items[i] + if mnt.Key != actual.Key || mnt.Path != actual.Path { + t.Errorf("expected volume %s to mount %s -> %s, got %s -> %s", volume.Name, mnt.Key, mnt.Path, actual.Key, actual.Path) + } + } + } + if volume.Secret != nil { + if volume.Secret.SecretName != expected.refName { + t.Errorf("expected volume %s to reference Secret %s, got %s", volume.Name, "trusted-ca", volume.Secret.SecretName) + } + if expected.optional { + if volume.Secret.Optional == nil || *volume.Secret.Optional != expected.optional { + t.Errorf("expected volume %s to be optional=%t, got %t", volume.Name, expected.optional, *volume.Secret.Optional) + } + } + if len(expected.items) != len(volume.Secret.Items) { + t.Errorf("expected volume %s to mount %d items, got %d", volume.Name, len(expected.items), len(volume.Secret.Items)) + } + for i, mnt := range expected.items { + actual := volume.Secret.Items[i] + if mnt.Key != actual.Key || mnt.Path != actual.Path { + t.Errorf("expected volume %s to mount %s -> %s, got %s -> %s", volume.Name, mnt.Key, mnt.Path, actual.Key, actual.Path) + } + } + } + if volume.Projected != nil && len(volume.Projected.Sources) > 0 { + for _, src := range volume.Projected.Sources { + if src.ConfigMap != nil { + if src.ConfigMap.LocalObjectReference.Name != expected.refName { + t.Errorf("expected volume %s to reference ConfigMap %s, got %s", volume.Name, expected.refName, src.ConfigMap.LocalObjectReference.Name) + } + } + if src.Secret != nil { + if src.Secret.LocalObjectReference.Name != expected.refName { + t.Errorf("expected volume %s to reference Secret %s, got %s", volume.Name, expected.refName, src.Secret.LocalObjectReference.Name) + } + } + } + } +} + +func verifyMount(mount corev1.VolumeMount, expected *volumeMount, t *testing.T) { + if mount.MountPath != expected.mountPath { + t.Errorf("expected mount path to be %s, got %s", expected.mountPath, mount.MountPath) + } +}