diff --git a/pkg/operator/controller/certificate/controller.go b/pkg/operator/controller/certificate/controller.go index cf1295fb32..26eb2a46e5 100644 --- a/pkg/operator/controller/certificate/controller.go +++ b/pkg/operator/controller/certificate/controller.go @@ -25,7 +25,6 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" - "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" runtimecontroller "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -43,7 +42,6 @@ var log = logf.Logger.WithName(controllerName) func New(mgr manager.Manager, operatorNamespace string) (runtimecontroller.Controller, error) { reconciler := &reconciler{ client: mgr.GetClient(), - cache: mgr.GetCache(), recorder: mgr.GetEventRecorderFor(controllerName), operatorNamespace: operatorNamespace, } @@ -59,7 +57,6 @@ func New(mgr manager.Manager, operatorNamespace string) (runtimecontroller.Contr type reconciler struct { client client.Client - cache cache.Cache recorder record.EventRecorder operatorNamespace string } @@ -110,15 +107,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err } } - // This section creates the legacy router-ca configmap which only ever contains the operator-managed default signing - // cert used for signing the default wildcard serving cert - ingresses := &operatorv1.IngressControllerList{} - if err := r.cache.List(context.TODO(), ingresses, client.InNamespace(r.operatorNamespace)); err != nil { - errs = append(errs, fmt.Errorf("failed to list ingresscontrollers: %v", err)) - } else if err := r.ensureRouterCAConfigMap(ca, ingresses.Items); err != nil { - errs = append(errs, fmt.Errorf("failed to publish router CA: %v", err)) - } - // We need to construct the CA bundle that can be used to verify the ingress used to serve the console and the oauth-server. // In an operator maintained cluster, this is always `oc get -n openshift-ingress-operator ingresscontroller/default`, skip the rest and return here. // TODO if network-edge wishes to expand the scope of the CA bundle (and you could legitimately see a need/desire to have one CA that verifies all ingress traffic). diff --git a/pkg/operator/controller/certificate/publish_ca.go b/pkg/operator/controller/certificate/publish_ca.go index 7234f3634c..4c9fe5a8fd 100644 --- a/pkg/operator/controller/certificate/publish_ca.go +++ b/pkg/operator/controller/certificate/publish_ca.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" corev1 "k8s.io/api/core/v1" @@ -28,16 +27,6 @@ func (r *reconciler) ensureDefaultIngressCertConfigMap(caBundle string) error { return r.ensureConfigMap(name, desired) } -// ensureRouterCAConfigMap will create, update, or delete the configmap for the -// router CA as appropriate. -func (r *reconciler) ensureRouterCAConfigMap(secret *corev1.Secret, ingresses []operatorv1.IngressController) error { - desired, err := desiredRouterCAConfigMap(secret, ingresses) - if err != nil { - return err - } - return r.ensureConfigMap(controller.RouterCAConfigMapName(), desired) -} - // ensureConfigMap will create, update, or delete the configmap as appropriate. func (r *reconciler) ensureConfigMap(name types.NamespacedName, desired *corev1.ConfigMap) error { current, err := r.currentConfigMap(name) @@ -73,36 +62,6 @@ func (r *reconciler) ensureConfigMap(name types.NamespacedName, desired *corev1. return nil } -// desiredRouterCAConfigMap returns the desired router CA configmap. -func desiredRouterCAConfigMap(secret *corev1.Secret, ingresses []operatorv1.IngressController) (*corev1.ConfigMap, error) { - if !shouldPublishRouterCA(ingresses) { - return nil, nil - } - - name := controller.RouterCAConfigMapName() - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: name.Name, - Namespace: name.Namespace, - }, - Data: map[string]string{ - "ca-bundle.crt": string(secret.Data["tls.crt"]), - }, - } - return cm, nil -} - -// shouldPublishRouterCA checks if some IngressController uses the default -// certificate, in which case the CA certificate needs to be published. -func shouldPublishRouterCA(ingresses []operatorv1.IngressController) bool { - for _, ci := range ingresses { - if ci.Spec.DefaultCertificate == nil { - return true - } - } - return false -} - // currentConfigMap returns the current state of the desired configmap namespace/name. func (r *reconciler) currentConfigMap(name types.NamespacedName) (*corev1.ConfigMap, error) { cm := &corev1.ConfigMap{} diff --git a/pkg/operator/controller/certificate/publish_ca_test.go b/pkg/operator/controller/certificate/publish_ca_test.go deleted file mode 100644 index ecfc9d9f04..0000000000 --- a/pkg/operator/controller/certificate/publish_ca_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package certificate - -import ( - "fmt" - "testing" - - corev1 "k8s.io/api/core/v1" - - operatorv1 "github.com/openshift/api/operator/v1" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestShouldPublishRouterCA(t *testing.T) { - var replicas int32 = 1 - var ( - empty = "" - x = "x" - y = "y" - ) - testCases := []struct { - description string - input []*string - output bool - }{ - {"no ingresses", []*string{}, false}, - {"all using no default certificates", []*string{&empty, &empty, &empty}, false}, - {"all using generated default certificates", []*string{nil, nil, nil}, true}, - {"all using custom default certificates", []*string{&x, &x, &y}, false}, - {"mix of custom default certificate and no default certificate", []*string{&x, &x, &empty}, false}, - {"mix of custom default certificate and generated default certificate", []*string{&x, &x, nil}, true}, - {"mix of no default certificate and generated default certificate", []*string{&empty, &empty, nil}, true}, - } - for _, tc := range testCases { - ingresses := []operatorv1.IngressController{} - for i := range tc.input { - var cert *corev1.LocalObjectReference - if secret := tc.input[i]; secret != nil { - cert = &corev1.LocalObjectReference{Name: *secret} - } - ingress := operatorv1.IngressController{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("router-%d", i), - }, - Spec: operatorv1.IngressControllerSpec{ - Replicas: &replicas, - DefaultCertificate: cert, - }, - } - ingresses = append(ingresses, ingress) - } - if e, a := tc.output, shouldPublishRouterCA(ingresses); e != a { - t.Errorf("%q: expected %t, got %t", tc.description, e, a) - } - - } -} diff --git a/pkg/operator/controller/names.go b/pkg/operator/controller/names.go index 32ee147127..8033a03fd6 100644 --- a/pkg/operator/controller/names.go +++ b/pkg/operator/controller/names.go @@ -53,16 +53,6 @@ func RouterCASecretName(operatorNamespace string) types.NamespacedName { } } -// RouterCAConfigMapName returns the namespaced name for the router CA configmap. -// The operator uses this configmap to publish the public key for the CA -// certificate, so that other operators can include it into their trust bundles. -func RouterCAConfigMapName() types.NamespacedName { - return types.NamespacedName{ - Namespace: GlobalMachineSpecifiedConfigNamespace, - Name: "router-ca", - } -} - // DefaultIngressCertConfigMapName returns the namespaced name for the default ingress cert configmap. // The operator uses this configmap to publish the public key that golang clients can use to trust // the default ingress wildcard serving cert. diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go index c2c4727a75..1212b196c5 100644 --- a/test/e2e/operator_test.go +++ b/test/e2e/operator_test.go @@ -194,10 +194,6 @@ func TestUpdateDefaultIngressController(t *testing.T) { t.Fatalf("failed to observe expected conditions: %v", err) } - routerCAConfigmap := &corev1.ConfigMap{} - if err := kclient.Get(context.TODO(), controller.RouterCAConfigMapName(), routerCAConfigmap); err != nil { - t.Fatalf("failed to get CA certificate configmap: %v", err) - } defaultIngressCAConfigmap := &corev1.ConfigMap{} if err := kclient.Get(context.TODO(), controller.DefaultIngressCertConfigMapName(), defaultIngressCAConfigmap); err != nil { t.Fatalf("failed to get CA certificate configmap: %v", err) @@ -248,19 +244,6 @@ func TestUpdateDefaultIngressController(t *testing.T) { t.Fatalf("failed to observe updated deployment: %v", err) } - // Wait for the CA certificate configmap to be deleted. - err = wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) { - if err := kclient.Get(context.TODO(), controller.RouterCAConfigMapName(), routerCAConfigmap); err != nil { - if errors.IsNotFound(err) { - return true, nil - } - t.Logf("failed to get CA certificate configmap, will retry: %v", err) - } - return false, nil - }) - if err != nil { - t.Fatalf("failed to observe clean-up of CA certificate configmap: %v", err) - } // Wait for the default ingress configmap to be updated previousDefaultIngressCAConfigmap := defaultIngressCAConfigmap.DeepCopy() err = wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) { @@ -285,19 +268,6 @@ func TestUpdateDefaultIngressController(t *testing.T) { t.Errorf("failed to reset default ingresscontroller: %v", err) } - // Wait for the CA certificate configmap to be recreated. - err = wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) { - if err := kclient.Get(context.TODO(), controller.RouterCAConfigMapName(), routerCAConfigmap); err != nil { - if !errors.IsNotFound(err) { - t.Logf("failed to get CA certificate configmap, will retry: %v", err) - } - return false, nil - } - return true, nil - }) - if err != nil { - t.Fatalf("failed to get recreated CA certificate configmap: %v", err) - } // Wait for the default ingress configmap to be updated back to the original previousDefaultIngressCAConfigmap = defaultIngressCAConfigmap.DeepCopy() err = wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) { @@ -449,7 +419,9 @@ func getScaleClient() (scale.ScalesGetter, error) { return scale.NewForConfig(kubeConfig, restMapper, dynamic.LegacyAPIPathResolverFunc, scaleKindResolver) } -func TestRouterCACertificate(t *testing.T) { +// TestDefaultIngressCertificate verifies that the "default-ingress-cert" +// configmap is published and can be used to connect to the router. +func TestDefaultIngressCertificate(t *testing.T) { ic := &operatorv1.IngressController{} if err := kclient.Get(context.TODO(), defaultName, ic); err != nil { t.Fatalf("failed to get default ingresscontroller: %v", err) @@ -464,22 +436,16 @@ func TestRouterCACertificate(t *testing.T) { t.Fatalf("failed to observe expected conditions: %v", err) } + defaultIngressCAConfigmap := &corev1.ConfigMap{} + if err := kclient.Get(context.TODO(), controller.DefaultIngressCertConfigMapName(), defaultIngressCAConfigmap); err != nil { + t.Fatalf("failed to get CA certificate configmap: %v", err) + } + var certData []byte - err := wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) { - cm := &corev1.ConfigMap{} - err := kclient.Get(context.TODO(), controller.RouterCAConfigMapName(), cm) - if err != nil { - return false, nil - } - if val, ok := cm.Data["ca-bundle.crt"]; !ok { - return false, fmt.Errorf("router-ca secret is missing %q", "ca-bundle.crt") - } else { - certData = []byte(val) - } - return true, nil - }) - if err != nil { - t.Fatalf("failed to get CA certificate: %v", err) + if val, ok := defaultIngressCAConfigmap.Data["ca-bundle.crt"]; !ok { + t.Fatalf("%s configmap is missing %q", controller.DefaultIngressCertConfigMapName(), "ca-bundle.crt") + } else { + certData = []byte(val) } certPool := x509.NewCertPool() @@ -489,8 +455,7 @@ func TestRouterCACertificate(t *testing.T) { wildcardRecordName := controller.WildcardDNSRecordName(ic) wildcardRecord := &iov1.DNSRecord{} - err = kclient.Get(context.TODO(), wildcardRecordName, wildcardRecord) - if err != nil { + if err := kclient.Get(context.TODO(), wildcardRecordName, wildcardRecord); err != nil { t.Fatalf("failed to get wildcard dnsrecord %s: %v", wildcardRecordName, err) } // TODO: handle >0 targets