Merge pull request #26 from rthallisey/simplify-api

Simplify the Cincinnati Operator API

deploy/crds/cincinnati.openshift.io_v1alpha1_cincinnati_cr.yaml

@@ -6,5 +6,4 @@ spec:
   replicas: 1
   registry: "quay.io"
   repository: "openshift-release-dev/ocp-release"
-  certConfigMapKey: ""
-  graphDataImage: "your-registry/your-repo/your-init-container"
+  graphDataImage: "your-registry/your-repo/your-init-container"

deploy/olm-catalog/cincinnati-operator/0.0.1/cincinnati-operator.v0.0.1.clusterserviceversion.yaml

@@ -14,7 +14,6 @@ metadata:
             "registry": "quay.io",
             "replicas": 1,
             "repository": "openshift-release-dev/ocp-release",
-            "certConfigMapKey": "",
             "graphDataImage": "quay.io/cincinnati/demo-graph-data:for-testing-only",
           }
         }

docs/external-registry-ca.md

@@ -0,0 +1,38 @@
+# External Registry CA Injection
+
+If you are using a secure external container registry to hold mirrored OpenShift
+release images, Cincinnati will need access to this registry in order to build
+an upgrade graph.  Here's how you can inject your CA Cert into the Cincinnati
+pod.
+
+OpenShift has an external registry API, located at `image.config.openshift.io`,
+that we'll use to store the external registry CA Cert.  You can read more about
+this API in the [OpenShift documentation](https://docs.openshift.com/container-platform/4.3/registry/configuring-registry-operator.html#images-configuration-cas_configuring-registry-operator).
+
+Create a ConfigMap in the `openshift-config` namespace.  Fill in your CA Cert
+under the key `cincinnati-registry`, because it's how Cincinnati locates your Cert:
+```yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: trusted-ca
+data:
+  cincinnati-registry: |
+    -----BEGIN CERTIFICATE-----
+    ...
+    -----END CERTIFICATE-----
+```
+
+Edit the `cluster` resource from the `image.config.openshift.io` API and set
+the `additionalTrustedCA` field to the name of the ConfigMap you just created
+above.
+```bash
+$ oc edit image.config.openshift.io cluster
+spec:
+  additionalTrustedCA:
+    name: trusted-ca
+```
+
+The Cincinnati Operator will watch the `image.config.openshift.io` API and the
+ConfigMap you created in the `openshift-config` namespace for changes, then
+restart the deployment if the Cert has changed.

pkg/apis/cincinnati/v1alpha1/cincinnati_types.go

@@ -25,12 +25,6 @@ type CincinnatiSpec struct {
 	// GraphDataImage is a container image that contains the Cincinnati graph
 	// data. The data is copied to /var/lib/cincinnati/graph-data.
 	GraphDataImage string `json:"graphDataImage"`
-
-	// CertConfigMapKey refers to the ConfigMap key that holds a CA cert to the registry
-	// Cincinnati will contact to build the upgrade graph.  The name of the
-	// ConfigMap holding the key comes from the field additionalTrustedCA
-	// in the image.config.openshift.io API.
-	CertConfigMapKey string `json:"certConfigMapKey,omitempty"`
 }
 
 // CincinnatiStatus defines the observed state of Cincinnati

pkg/controller/cincinnati/cincinnati_controller.go

@@ -217,6 +217,11 @@ func (r *ReconcileCincinnati) ensureAdditionalTrustedCA(ctx context.Context, req
 		return err
 	}
 
+	if _, ok := sourceCM.Data[NameCertConfigMapKey]; !ok {
+		reqLogger.Info("Found ConfigMap referenced by ImageConfig.Spec.AdditionalTrustedCA.Name but did not find key 'cincinnati-registry' for registry CA cert.", "Name", image.Spec.AdditionalTrustedCA.Name, "Namespace", openshiftConfigNamespace)
+		return nil
+	}
+
 	localCM := &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      nameAdditionalTrustedCA(instance),
@@ -234,6 +239,11 @@ func (r *ReconcileCincinnati) ensureAdditionalTrustedCA(ctx context.Context, req
 		handleErr(reqLogger, &instance.Status, "EnsureConfigMapFailed", err)
 		return err
 	}
+	// Mount in ConfigMap data from the cincinnati-registry key
+	externalCACert := true
+	resources.graphBuilderContainer = resources.newGraphBuilderContainer(instance, r.operandImage, externalCACert)
+	resources.deployment = resources.newDeployment(instance, externalCACert)
+
 	return nil
 }
 

pkg/controller/cincinnati/cincinnati_controller_test.go

@@ -200,10 +200,12 @@ func TestEnsureDeployment(t *testing.T) {
 	tests := []struct {
 		name       string
 		cincinnati *cv1alpha1.Cincinnati
+		caCert     bool
 	}{
 		{
 			name:       "EnsureDeployment",
 			cincinnati: newDefaultCincinnati(),
+			caCert:     false,
 		},
 		{
 			name: "EnsureDeploymentWithGraphDataImage",
@@ -212,6 +214,12 @@ func TestEnsureDeployment(t *testing.T) {
 				cincinnati.Spec.GraphDataImage = testGraphDataImage
 				return cincinnati
 			}(),
+			caCert: false,
+		},
+		{
+			name:       "EnsureDeploymentWithCaCert",
+			cincinnati: newDefaultCincinnati(),
+			caCert:     true,
 		},
 	}
 	for _, test := range tests {
@@ -220,6 +228,11 @@ func TestEnsureDeployment(t *testing.T) {
 			r := newTestReconciler(cincinnati)
 
 			resources, err := newKubeResources(cincinnati, testOperandImage)
+
+			if test.caCert {
+				resources.graphBuilderContainer = resources.newGraphBuilderContainer(cincinnati, testOperandImage, test.caCert)
+				resources.deployment = resources.newDeployment(cincinnati, test.caCert)
+			}
 			err = r.ensureDeployment(context.TODO(), log, cincinnati, resources)
 			if err != nil {
 				t.Fatal(err)
@@ -242,6 +255,10 @@ func TestEnsureDeployment(t *testing.T) {
 			assert.Equal(t, found.Spec.Template.Spec.Containers[1].Image, resources.graphBuilderContainer.Image)
 			assert.Equal(t, found.Spec.Template.Spec.Containers[1].Name, resources.policyEngineContainer.Name)
 			assert.Equal(t, found.Spec.Template.Spec.Containers[1].Image, resources.graphBuilderContainer.Image)
+			if test.caCert {
+				assert.Equal(t, found.Spec.Template.Spec.Volumes[2].Name, NameTrustedCAVolume)
+				assert.Equal(t, found.Spec.Template.Spec.Containers[0].VolumeMounts[2].Name, NameTrustedCAVolume)
+			}
 
 			initContainer := found.Spec.Template.Spec.InitContainers[0]
 			assert.Equal(t, &initContainer, resources.graphDataInitContainer)

pkg/controller/cincinnati/names.go

@@ -14,6 +14,10 @@ const (
 	NameInitContainerGraphData string = "graph-data"
 	// openshiftConfigNamespace is the name of openshift's configuration namespace
 	openshiftConfigNamespace = "openshift-config"
+	// NameTrustedCAVolume is the name of the Volume used in Cincinnati's deployment containing the CA Cert
+	NameTrustedCAVolume = "trusted-ca"
+	// NameCertConfigMapKey is the ConfigMap key name where the operator expects the external registry CA Cert
+	NameCertConfigMapKey = "cincinnati-registry"
 )
 
 func nameDeployment(instance *cv1alpha1.Cincinnati) string {
@@ -44,10 +48,6 @@ func nameAdditionalTrustedCA(instance *cv1alpha1.Cincinnati) string {
 	return instance.Name + "-trusted-ca"
 }
 
-func nameDeploymentTrustedCA() string {
-	return "trusted-ca"
-}
-
 // When running a single replica, allow 0 available so we don't block node
 // drains. Otherwise require 1.
 func getMinAvailablePBD(instance *cv1alpha1.Cincinnati) intstr.IntOrString {

pkg/controller/cincinnati/new.go

@@ -98,12 +98,13 @@ func newKubeResources(instance *cv1alpha1.Cincinnati, image string) (*kubeResour
 	if err != nil {
 		return nil, err
 	}
+	externalCACert := false
 	k.envConfigHash = envConfigHash
 	k.podDisruptionBudget = k.newPodDisruptionBudget(instance)
-	k.graphBuilderContainer = k.newGraphBuilderContainer(instance, image)
+	k.graphBuilderContainer = k.newGraphBuilderContainer(instance, image, externalCACert)
 	k.graphDataInitContainer = k.newGraphDataInitContainer(instance)
 	k.policyEngineContainer = k.newPolicyEngineContainer(instance, image)
-	k.deployment = k.newDeployment(instance)
+	k.deployment = k.newDeployment(instance, externalCACert)
 	k.graphBuilderService = k.newGraphBuilderService(instance)
 	k.policyEngineService = k.newPolicyEngineService(instance)
 	return &k, nil
@@ -233,8 +234,7 @@ func (k *kubeResources) newGraphBuilderConfig(instance *cv1alpha1.Cincinnati) (*
 	}, nil
 }
 
-func (k *kubeResources) newDeployment(instance *cv1alpha1.Cincinnati) *appsv1.Deployment {
-	trustedCaName := nameDeploymentTrustedCA()
+func (k *kubeResources) newDeployment(instance *cv1alpha1.Cincinnati, externalCACert bool) *appsv1.Deployment {
 	name := nameDeployment(instance)
 	maxUnavailable := intstr.FromString("50%")
 	maxSurge := intstr.FromString("100%")
@@ -303,9 +303,9 @@ func (k *kubeResources) newDeployment(instance *cv1alpha1.Cincinnati) *appsv1.De
 		}
 	}
 
-	if instance.Spec.CertConfigMapKey != "" {
+	if externalCACert {
 		v := corev1.Volume{
-			Name: trustedCaName,
+			Name: NameTrustedCAVolume,
 			VolumeSource: corev1.VolumeSource{
 				ConfigMap: &corev1.ConfigMapVolumeSource{
 					DefaultMode: &mode,
@@ -315,7 +315,7 @@ func (k *kubeResources) newDeployment(instance *cv1alpha1.Cincinnati) *appsv1.De
 					Items: []corev1.KeyToPath{
 						corev1.KeyToPath{
 							Path: "tls-ca-bundle.pem",
-							Key:  instance.Spec.CertConfigMapKey,
+							Key:  NameCertConfigMapKey,
 						},
 					},
 				},
@@ -340,8 +340,7 @@ func (k *kubeResources) newGraphDataInitContainer(instance *cv1alpha1.Cincinnati
 	}
 }
 
-func (k *kubeResources) newGraphBuilderContainer(instance *cv1alpha1.Cincinnati, image string) *corev1.Container {
-	trustedCaName := nameDeploymentTrustedCA()
+func (k *kubeResources) newGraphBuilderContainer(instance *cv1alpha1.Cincinnati, image string, externalCACert bool) *corev1.Container {
 	g := &corev1.Container{
 		Name:            NameContainerGraphBuilder,
 		Image:           image,
@@ -418,9 +417,9 @@ func (k *kubeResources) newGraphBuilderContainer(instance *cv1alpha1.Cincinnati,
 			},
 		},
 	}
-	if instance.Spec.CertConfigMapKey != "" {
+	if externalCACert {
 		v := corev1.VolumeMount{
-			Name:      trustedCaName,
+			Name:      NameTrustedCAVolume,
 			ReadOnly:  true,
 			MountPath: "/etc/pki/ca-trust/extracted/pem",
 		}