Repository: Azure/ARO-RP
Pull request #1791 (2021-10-14 03:57:41)

Add some egress/networking config checks in the Operator
### Which issue this PR addresses:

Fixes ADO-12201105

### What this PR does / why we need it:

Adds checks for if a cluster has a custom Ingress certificate on the default ingress, or custom DNS operator configuration. These can lead to problems with the Authentication operator not working, so knowing about these configurations being non-default will help debugging.

### Test plan for issue:

Includes tests.

### Is there any documentation that needs to be updated for this PR?

Not really, it should show up on the dashboard with other Operator checks.

hawkowl: /azp run e2e
azure-pipelines[bot]: <samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
azure-pipelines[bot]: <samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
hawkowl: /azp run e2e
github-actions[bot]: Please rebase pull request.
petrkotas: Damn, @hawkowl I am terribly sorry. Would you please rebase your PR and we merge right after. Thank you.

Patch:
diff --git a/cmd/aro/operator.go b/cmd/aro/operator.go
index 83a3f8fb0..0b02c219f 100644
--- a/cmd/aro/operator.go
+++ b/cmd/aro/operator.go
@@ -12,6 +12,7 @@ import (
 	consoleclient "github.com/openshift/client-go/console/clientset/versioned"
 	imageregistryclient "github.com/openshift/client-go/imageregistry/clientset/versioned"
 	machineclient "github.com/openshift/client-go/machine/clientset/versioned"
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
 	securityclient "github.com/openshift/client-go/security/clientset/versioned"
 	mcoclient "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned"
 	"github.com/sirupsen/logrus"
@@ -108,6 +109,10 @@ func operator(ctx context.Context, log *logrus.Entry) error {
 	if err != nil {
 		return err
 	}
+	operatorcli, err := operatorclient.NewForConfig(restConfig)
+	if err != nil {
+		return err
+	}
 	// TODO (NE): dh is sometimes passed, sometimes created later. Can we standardize?
 	dh, err := dynamichelper.New(log, restConfig)
 	if err != nil {
@@ -225,7 +230,7 @@ func operator(ctx context.Context, log *logrus.Entry) error {
 
 	if err = (checker.NewReconciler(
 		log.WithField("controller", checker.ControllerName),
-		arocli, kubernetescli, maocli, role)).SetupWithManager(mgr); err != nil {
+		arocli, kubernetescli, maocli, operatorcli, configcli, role)).SetupWithManager(mgr); err != nil {
 		return fmt.Errorf("unable to create controller %s: %v", checker.ControllerName, err)
 	}
 
diff --git a/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go b/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go
index 17e82c1b2..8c25725c1 100644
--- a/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go
+++ b/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go
@@ -24,17 +24,34 @@ const (
 	ServicePrincipalValid       = "ServicePrincipalValid"
 
 	ManagedUpgradeOperatorStatus = "ManagedUpgradeOperatorStatus"
+
+	// advisor checks
+	DefaultIngressCertificate = "DefaultIngressCertificate"
+	DefaultClusterDNS         = "DefaultClusterDNS"
 )
 
 // AllConditionTypes is a operator conditions currently in use, any condition not in this list is not
 // added to the operator.status.conditions list
 func AllConditionTypes() []string {
-	return []string{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid, ServicePrincipalValid, ManagedUpgradeOperatorStatus}
+	return []string{
+		InternetReachableFromMaster,
+		InternetReachableFromWorker,
+		MachineValid,
+		ServicePrincipalValid,
+		ManagedUpgradeOperatorStatus,
+		DefaultIngressCertificate,
+		DefaultClusterDNS,
+	}
 }
 
 // ClusterChecksTypes represents checks performed on the cluster to verify basic functionality
 func ClusterChecksTypes() []string {
-	return []string{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid, ServicePrincipalValid}
+	return []string{
+		InternetReachableFromMaster,
+		InternetReachableFromWorker,
+		MachineValid,
+		ServicePrincipalValid,
+	}
 }
 
 type GenevaLoggingSpec struct {
diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go
index c40f431d0..4dad8cae8 100644
--- a/pkg/operator/controllers/checker/checker_controller.go
+++ b/pkg/operator/controllers/checker/checker_controller.go
@@ -7,7 +7,9 @@ import (
 	"context"
 	"time"
 
+	configclient "github.com/openshift/client-go/config/clientset/versioned"
 	machineclient "github.com/openshift/client-go/machine/clientset/versioned"
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -37,12 +39,14 @@ type Reconciler struct {
 	arocli   aroclient.Interface
 }
 
-func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, kubernetescli kubernetes.Interface, maocli machineclient.Interface, role string) *Reconciler {
+func NewReconciler(log *logrus.Entry, arocli aroclient.Interface, kubernetescli kubernetes.Interface, machinecli machineclient.Interface, operatorcli operatorclient.Interface, configcli configclient.Interface, role string) *Reconciler {
 	checkers := []Checker{NewInternetChecker(log, arocli, role)}
 
 	if role == operator.RoleMaster {
 		checkers = append(checkers,
-			NewServicePrincipalChecker(log, arocli, kubernetescli, maocli, role),
+			NewServicePrincipalChecker(log, arocli, kubernetescli, machinecli, role),
+			NewIngressCertificateChecker(log, arocli, operatorcli, configcli, role),
+			NewClusterDNSChecker(log, arocli, operatorcli, role),
 		)
 	}
 
diff --git a/pkg/operator/controllers/checker/clusterdns.go b/pkg/operator/controllers/checker/clusterdns.go
new file mode 100644
index 000000000..b6e8b4b96
--- /dev/null
+++ b/pkg/operator/controllers/checker/clusterdns.go
@@ -0,0 +1,89 @@
+// Implements a check that provides detail on openshift-dns-operator
+// configurations.
+//
+// Included checks are:
+//  - existence of custom DNS entries
+//  - malformed zone forwarding configuration
+
+package checker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
+	"github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned"
+	"github.com/Azure/ARO-RP/pkg/util/conditions"
+)
+
+type ClusterDNSChecker struct {
+	log *logrus.Entry
+
+	arocli      aroclient.Interface
+	operatorcli operatorclient.Interface
+
+	role string
+}
+
+func NewClusterDNSChecker(log *logrus.Entry, arocli aroclient.Interface, operatorcli operatorclient.Interface, role string) *ClusterDNSChecker {
+	return &ClusterDNSChecker{
+		log:         log,
+		arocli:      arocli,
+		operatorcli: operatorcli,
+	}
+}
+
+func (r *ClusterDNSChecker) Name() string {
+	return "ClusterDNSChecker"
+}
+
+func (r *ClusterDNSChecker) Check(ctx context.Context) error {
+	cond := &operatorv1.OperatorCondition{
+		Type:    arov1alpha1.DefaultClusterDNS,
+		Status:  operatorv1.ConditionUnknown,
+		Message: "",
+		Reason:  "CheckDone",
+	}
+
+	dns, err := r.operatorcli.OperatorV1().DNSes().Get(ctx, "default", metav1.GetOptions{})
+	if err != nil {
+		cond.Message = err.Error()
+		cond.Reason = "CheckFailed"
+		return conditions.SetCondition(ctx, r.arocli, cond, r.role)
+	}
+
+	var upstreams []string
+	for _, s := range dns.Spec.Servers {
+		for _, z := range s.Zones {
+			if z == "." {
+				// If "." is set as a zone, bail out and warn about the
+				// malformed config, as this will prevent CoreDNS from rolling
+				// out
+				cond.Message = `Malformed config: "." in zones`
+				cond.Status = operatorv1.ConditionFalse
+				return conditions.SetCondition(ctx, r.arocli, cond, r.role)
+			}
+		}
+
+		upstreams = append(upstreams, s.ForwardPlugin.Upstreams...)
+	}
+
+	if len(upstreams) > 0 {
+		cond.Status = operatorv1.ConditionFalse
+		cond.Message = fmt.Sprintf("Custom upstream DNS servers in use: %s", strings.Join(upstreams, ", "))
+	} else {
+		cond.Status = operatorv1.ConditionTrue
+		cond.Message = "No in-cluster upstream DNS servers"
+	}
+
+	return conditions.SetCondition(ctx, r.arocli, cond, r.role)
+}
diff --git a/pkg/operator/controllers/checker/ingresscert.go b/pkg/operator/controllers/checker/ingresscert.go
new file mode 100644
index 000000000..fae96c126
--- /dev/null
+++ b/pkg/operator/controllers/checker/ingresscert.go
@@ -0,0 +1,81 @@
+// Implements a check that provides detail on potentially faulty or customised
+// IngressController configurations on the default controller.
+//
+// Included checks are:
+//  - existence of custom ingress certificate
+//  - existence of default ingresscontroller
+
+package checker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	configclient "github.com/openshift/client-go/config/clientset/versioned"
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
+	"github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned"
+	"github.com/Azure/ARO-RP/pkg/util/conditions"
+)
+
+type IngressCertificateChecker struct {
+	arocli      aroclient.Interface
+	operatorcli operatorclient.Interface
+	configcli   configclient.Interface
+
+	role string
+}
+
+func NewIngressCertificateChecker(log *logrus.Entry, arocli aroclient.Interface, operatorcli operatorclient.Interface, configcli configclient.Interface, role string) *IngressCertificateChecker {
+	return &IngressCertificateChecker{
+		arocli:      arocli,
+		operatorcli: operatorcli,
+		configcli:   configcli,
+	}
+}
+
+func (r *IngressCertificateChecker) Name() string {
+	return "IngressCertificateChecker"
+}
+
+func (r *IngressCertificateChecker) Check(ctx context.Context) error {
+	cond := &operatorv1.OperatorCondition{
+		Type:    arov1alpha1.DefaultIngressCertificate,
+		Status:  operatorv1.ConditionUnknown,
+		Message: "",
+		Reason:  "CheckDone",
+	}
+
+	cv, err := r.configcli.ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
+	if err != nil {
+		cond.Message = err.Error()
+		cond.Reason = "CheckFailed"
+		return conditions.SetCondition(ctx, r.arocli, cond, r.role)
+	}
+
+	ingress, err := r.operatorcli.OperatorV1().IngressControllers("openshift-ingress-operator").Get(ctx, "default", metav1.GetOptions{})
+	if err != nil {
+		cond.Message = err.Error()
+		cond.Reason = "CheckFailed"
+		return conditions.SetCondition(ctx, r.arocli, cond, r.role)
+	}
+
+	if ingress.Spec.DefaultCertificate == nil {
+		cond.Status = operatorv1.ConditionFalse
+		cond.Message = "Ingress has no certificate yet"
+	} else if ingress.Spec.DefaultCertificate.Name != string(cv.Spec.ClusterID)+"-ingress" {
+		cond.Status = operatorv1.ConditionFalse
+		cond.Message = "Custom ingress certificate in use: " + ingress.Spec.DefaultCertificate.Name
+	} else {
+		cond.Status = operatorv1.ConditionTrue
+		cond.Message = "Default ingress certificate in use"
+	}
+
+	return conditions.SetCondition(ctx, r.arocli, cond, r.role)
+}


Test patch:
diff --git a/pkg/operator/controllers/checker/clusterdns_test.go b/pkg/operator/controllers/checker/clusterdns_test.go
new file mode 100644
index 000000000..2ac515560
--- /dev/null
+++ b/pkg/operator/controllers/checker/clusterdns_test.go
@@ -0,0 +1,193 @@
+package checker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/go-test/deep"
+	operatorv1 "github.com/openshift/api/operator/v1"
+	operatorfake "github.com/openshift/client-go/operator/clientset/versioned/fake"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake"
+)
+
+func TestDefaultClusterDNS(t *testing.T) {
+	ctx := context.Background()
+
+	for _, tt := range []struct {
+		name          string
+		aroCluster    *arov1alpha1.Cluster
+		dns           *operatorv1.DNS
+		expectedState operatorv1.OperatorCondition
+	}{
+		{
+			name: "run: has default DNS",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+				Spec: operatorv1.DNSSpec{},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultClusterDNS,
+				Status:  operatorv1.ConditionTrue,
+				Reason:  "CheckDone",
+				Message: "No in-cluster upstream DNS servers",
+			},
+		},
+		{
+			name: "run: has changed DNS",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							Name:  "test-server",
+							Zones: []string{"example.com"},
+							ForwardPlugin: operatorv1.ForwardPlugin{
+								Upstreams: []string{
+									"1.2.3.4", "5.6.7.8",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultClusterDNS,
+				Status:  operatorv1.ConditionFalse,
+				Reason:  "CheckDone",
+				Message: "Custom upstream DNS servers in use: 1.2.3.4, 5.6.7.8",
+			},
+		},
+		{
+			name: "run: malformed, servers but no forwardplugin",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							Name:  "test-server",
+							Zones: []string{"example.com"},
+						},
+					},
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultClusterDNS,
+				Status:  operatorv1.ConditionTrue,
+				Reason:  "CheckDone",
+				Message: "No in-cluster upstream DNS servers",
+			},
+		},
+		{
+			name: "fail: malformed, using . for zones",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "default",
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							Name:  "test-server",
+							Zones: []string{"."},
+							ForwardPlugin: operatorv1.ForwardPlugin{
+								Upstreams: []string{
+									"1.2.3.4", "5.6.7.8",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultClusterDNS,
+				Status:  operatorv1.ConditionFalse,
+				Reason:  "CheckDone",
+				Message: `Malformed config: "." in zones`,
+			},
+		},
+		{
+			name: "fail: no DNS",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultClusterDNS,
+				Status:  operatorv1.ConditionUnknown,
+				Reason:  "CheckFailed",
+				Message: `dnses.operator.openshift.io "default" not found`,
+			},
+		},
+	} {
+		arocli := arofake.NewSimpleClientset()
+		operatorcli := operatorfake.NewSimpleClientset()
+
+		if tt.aroCluster != nil {
+			arocli = arofake.NewSimpleClientset(tt.aroCluster)
+		}
+		if tt.dns != nil {
+			operatorcli = operatorfake.NewSimpleClientset(tt.dns)
+		}
+
+		sp := NewClusterDNSChecker(nil, arocli, operatorcli, "")
+
+		t.Run(tt.name, func(t *testing.T) {
+			err := sp.Check(ctx)
+
+			if err != nil {
+				t.Error(err)
+			}
+
+			cluster, err := arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
+			if err != nil {
+				t.Error(err)
+			}
+
+			conds := []operatorv1.OperatorCondition{}
+
+			// nil out time
+			for _, c := range cluster.Status.Conditions {
+				c.LastTransitionTime = metav1.NewTime(time.Time{})
+				conds = append(conds, c)
+			}
+
+			errs := deep.Equal(conds, []operatorv1.OperatorCondition{tt.expectedState})
+			for _, err := range errs {
+				t.Error(err)
+			}
+		})
+	}
+}
diff --git a/pkg/operator/controllers/checker/ingresscert_test.go b/pkg/operator/controllers/checker/ingresscert_test.go
new file mode 100644
index 000000000..7601650ce
--- /dev/null
+++ b/pkg/operator/controllers/checker/ingresscert_test.go
@@ -0,0 +1,221 @@
+package checker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/go-test/deep"
+	configv1 "github.com/openshift/api/config/v1"
+	operatorv1 "github.com/openshift/api/operator/v1"
+	configfake "github.com/openshift/client-go/config/clientset/versioned/fake"
+	operatorfake "github.com/openshift/client-go/operator/clientset/versioned/fake"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake"
+)
+
+func TestDefaultIngressCertificate(t *testing.T) {
+	ctx := context.Background()
+
+	for _, tt := range []struct {
+		name              string
+		aroCluster        *arov1alpha1.Cluster
+		clusterVersion    *configv1.ClusterVersion
+		ingressController *operatorv1.IngressController
+		expectedState     operatorv1.OperatorCondition
+	}{
+		{
+			name: "run: has default certificate",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			clusterVersion: &configv1.ClusterVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "version",
+				},
+				Spec: configv1.ClusterVersionSpec{
+					ClusterID: "00000000-0000-0000-0000-000000000001",
+				},
+			},
+			ingressController: &operatorv1.IngressController{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "default",
+					Namespace: "openshift-ingress-operator",
+				},
+				Spec: operatorv1.IngressControllerSpec{
+					DefaultCertificate: &corev1.LocalObjectReference{
+						Name: "00000000-0000-0000-0000-000000000001-ingress",
+					},
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultIngressCertificate,
+				Status:  operatorv1.ConditionTrue,
+				Reason:  "CheckDone",
+				Message: "Default ingress certificate in use",
+			},
+		},
+		{
+			name: "run: does not have default certificate",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			clusterVersion: &configv1.ClusterVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "version",
+				},
+				Spec: configv1.ClusterVersionSpec{
+					ClusterID: "00000000-0000-0000-0000-000000000001",
+				},
+			},
+			ingressController: &operatorv1.IngressController{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "default",
+					Namespace: "openshift-ingress-operator",
+				},
+				Spec: operatorv1.IngressControllerSpec{
+					DefaultCertificate: &corev1.LocalObjectReference{
+						Name: "fancy-custom-cert",
+					},
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultIngressCertificate,
+				Status:  operatorv1.ConditionFalse,
+				Reason:  "CheckDone",
+				Message: "Custom ingress certificate in use: fancy-custom-cert",
+			},
+		},
+		{
+			name: "run: does not have default certificate defined",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			clusterVersion: &configv1.ClusterVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "version",
+				},
+				Spec: configv1.ClusterVersionSpec{
+					ClusterID: "00000000-0000-0000-0000-000000000001",
+				},
+			},
+			ingressController: &operatorv1.IngressController{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "default",
+					Namespace: "openshift-ingress-operator",
+				},
+				Spec: operatorv1.IngressControllerSpec{
+					DefaultCertificate: nil,
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultIngressCertificate,
+				Status:  operatorv1.ConditionFalse,
+				Reason:  "CheckDone",
+				Message: "Ingress has no certificate yet",
+			},
+		},
+		{
+			name: "fail: no ingress controller",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			clusterVersion: &configv1.ClusterVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "version",
+				},
+				Spec: configv1.ClusterVersionSpec{
+					ClusterID: "00000000-0000-0000-0000-000000000001",
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultIngressCertificate,
+				Status:  operatorv1.ConditionUnknown,
+				Reason:  "CheckFailed",
+				Message: `ingresscontrollers.operator.openshift.io "default" not found`,
+			},
+		},
+
+		{
+			name: "fail: no clusterversion",
+			aroCluster: &arov1alpha1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: arov1alpha1.SingletonClusterName,
+				},
+			},
+			ingressController: &operatorv1.IngressController{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "default",
+					Namespace: "openshift-ingress-operator",
+				},
+				Spec: operatorv1.IngressControllerSpec{
+					DefaultCertificate: &corev1.LocalObjectReference{
+						Name: "fancy-custom-cert",
+					},
+				},
+			},
+			expectedState: operatorv1.OperatorCondition{
+				Type:    arov1alpha1.DefaultIngressCertificate,
+				Status:  operatorv1.ConditionUnknown,
+				Reason:  "CheckFailed",
+				Message: `clusterversions.config.openshift.io "version" not found`,
+			},
+		},
+	} {
+		arocli := arofake.NewSimpleClientset()
+		configcli := configfake.NewSimpleClientset()
+		operatorcli := operatorfake.NewSimpleClientset()
+
+		if tt.aroCluster != nil {
+			arocli = arofake.NewSimpleClientset(tt.aroCluster)
+		}
+		if tt.clusterVersion != nil {
+			configcli = configfake.NewSimpleClientset(tt.clusterVersion)
+		}
+		if tt.ingressController != nil {
+			operatorcli = operatorfake.NewSimpleClientset(tt.ingressController)
+		}
+
+		sp := NewIngressCertificateChecker(nil, arocli, operatorcli, configcli, "")
+
+		t.Run(tt.name, func(t *testing.T) {
+			err := sp.Check(ctx)
+
+			if err != nil {
+				t.Error(err)
+			}
+
+			cluster, err := arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{})
+			if err != nil {
+				t.Error(err)
+			}
+
+			conds := []operatorv1.OperatorCondition{}
+
+			// nil out time
+			for _, c := range cluster.Status.Conditions {
+				c.LastTransitionTime = metav1.NewTime(time.Time{})
+				conds = append(conds, c)
+			}
+
+			errs := deep.Equal(conds, []operatorv1.OperatorCondition{tt.expectedState})
+			for _, err := range errs {
+				t.Error(err)
+			}
+		})
+	}
+}