refactor: invert ingress certificate logic

pkg/gatherers/clusterconfig/gather_cluster_ingress_certificates.go

@@ -5,36 +5,36 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
-	"time"
 
-	"k8s.io/klog/v2"
+	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
 
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/klog/v2"
 
-	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 
 	"github.com/openshift/insights-operator/pkg/record"
 )
 
-// This map defines the namespace and the certificates that we are looking for
-var ingressCertsMap = map[string][]string{
-	"openshift-ingress-operator": {"router-ca"},
-	"openshift-ingress":          {"router-certs-default"},
+const ingressCertificatesLimits = int64(64)
+
+var ingressNamespaces = []string{
+	"openshift-ingress-operator",
+	"openshift-ingress",
 }
 
-type IngressCertificateInfo struct {
-	Name      string    `json:"name"`
-	NotBefore time.Time `json:"not_before"`
-	NotAfter  time.Time `json:"not_after"`
+type CertificateInfo struct {
+	Name        string
+	NotBefore   metav1.Time
+	NotAfter    metav1.Time
+	Controllers []ControllerInfo
 }
 
-type IngressControllerInfo struct {
-	Name                         string                   `json:"name"`
-	OperatorGeneratedCertificate []IngressCertificateInfo `json:"operator_generated_certificate"`
-	CustomCertificates           []IngressCertificateInfo `json:"custom_certificates"`
+type ControllerInfo struct {
+	Name      string
+	Namespace string
 }
 
 func (g *Gatherer) GatherClusterIngressCertificates(ctx context.Context) ([]record.Record, []error) {
@@ -43,119 +43,117 @@ func (g *Gatherer) GatherClusterIngressCertificates(ctx context.Context) ([]reco
 		return nil, []error{err}
 	}
 
-	configClient, err := configv1client.NewForConfig(g.gatherKubeConfig)
+	operatorClient, err := operatorclient.NewForConfig(g.gatherKubeConfig)
 	if err != nil {
 		return nil, []error{err}
 	}
 
-	return gatherClusterIngressCertificates(ctx, gatherKubeClient.CoreV1(), configClient)
+	return gatherClusterIngressCertificates(ctx, gatherKubeClient.CoreV1(), operatorClient)
 }
 
-func gatherClusterIngressCertificates(ctx context.Context, coreClient corev1client.CoreV1Interface, configClient configv1client.ConfigV1Interface) ([]record.Record, []error) {
-	var records []record.Record
-	var errors []error
-
-	for namespace, certs := range ingressCertsMap {
-		ingressCerts, errs := getIngressCertificates(ctx, coreClient, configClient, namespace, certs)
-		if len(errs) > 0 {
-			errors = append(errors, errs...)
-			continue
-		}
-
-		if len(ingressCerts) > 0 {
-			records = append(records, record.Record{
-				Name: fmt.Sprintf("config/ingress/%s/ingress_certificates.json", namespace),
-				Item: record.JSONMarshaller{Object: ingressCerts},
-			})
-		}
-	}
-
-	return records, nil
-}
-
-func getIngressCertificates(
+func gatherClusterIngressCertificates(
 	ctx context.Context,
 	coreClient corev1client.CoreV1Interface,
-	openshiftClient configv1client.ConfigV1Interface,
-	namespace string,
-	certs []string) ([]IngressControllerInfo, []error) {
+	operatorClient operatorclient.Interface) ([]record.Record, []error) {
 
-	var controllers []IngressControllerInfo
-	var errors []error
+	var certificates []*CertificateInfo
+	var errs []error
 
-	ingressControllers, err := openshiftClient.Ingresses().List(ctx, metav1.ListOptions{})
-	if err != nil {
-		klog.V(2).Infof("failed to list IngressControllers: %v", err)
-		return nil, []error{err}
+	// Step 1: Collect router-ca and router-certs-default
+	routerCACert, routerCACertErr := getCertificateInfoFromSecret(ctx, coreClient, "openshift-ingress-operator", "router-ca")
+	if routerCACertErr != nil {
+		errs = append(errs, routerCACertErr)
+	} else {
+		certificates = append(certificates, routerCACert)
 	}
 
-	for _, controller := range ingressControllers.Items {
-		controllerName := controller.Name
-		controllerCertificates, errs := getControllerCertificates(ctx, coreClient, namespace, certs)
-		if len(errs) > 0 {
-			errors = append(errors, errs...)
-			continue
-		}
-
-		controllers = append(controllers, IngressControllerInfo{
-			Name:                         controllerName,
-			OperatorGeneratedCertificate: controllerCertificates,
-			CustomCertificates:           make([]IngressCertificateInfo, 0),
-		})
+	routerCertsDefaultCert, routerCertsDefaultCertErr := getCertificateInfoFromSecret(ctx, coreClient, "openshift-ingress", "router-certs-default")
+	if routerCertsDefaultCertErr != nil {
+		errs = append(errs, routerCertsDefaultCertErr)
+	} else {
+		certificates = append(certificates, routerCertsDefaultCert)
 	}
 
-	return controllers, errors
-}
-
-func getControllerCertificates(
-	ctx context.Context,
-	coreClient corev1client.CoreV1Interface,
-	namespace string,
-	certs []string) ([]IngressCertificateInfo, []error) {
-
-	var certInfos []IngressCertificateInfo
-	var errors []error
-
-	for _, secretName := range certs {
-		secret, err := coreClient.Secrets(namespace).Get(ctx, secretName, metav1.GetOptions{})
-		if err != nil {
-			klog.V(2).Infof("failed to fetch secret: %v", err)
-			errors = append(errors, err)
+	// Step 2: List all Ingress Controllers
+	for _, namespace := range ingressNamespaces {
+		controllers, err := operatorClient.OperatorV1().IngressControllers(namespace).List(ctx, metav1.ListOptions{})
+		if errors.IsNotFound(err) {
+			klog.V(2).Infof("Ingress Controllers not found in '%s' namespace", namespace)
 			continue
 		}
-
-		certInfo, err := certificateInfoFromSecret(secret)
 		if err != nil {
-			klog.V(2).Infof("failed to parse the ingress certificate: %v", err)
-			errors = append(errors, err)
+			errs = append(errs, err)
 			continue
 		}
 
-		certInfos = append(certInfos, *certInfo)
+		// Step 3: Filter Ingress Controllers with spec.defaultCertificate and get certificate info
+		for _, controller := range controllers.Items {
+			if controller.Spec.DefaultCertificate != nil {
+				certName := controller.Spec.DefaultCertificate.Name
+				certInfo, certErr := getCertificateInfoFromSecret(ctx, coreClient, namespace, certName)
+				if certErr != nil {
+					errs = append(errs, certErr)
+					continue
+				}
+
+				// Step 4: Add certificate info to the certificates list
+				found := false
+				for _, cert := range certificates {
+					if cert.Name == certInfo.Name {
+						// Certificate already exists, add the controller to its list
+						cert.Controllers = append(cert.Controllers, ControllerInfo{Name: controller.Name, Namespace: controller.Namespace})
+						found = true
+						break
+					}
+				}
+
+				if !found {
+					// Certificate not found, create a new entry
+					certificates = append(certificates, certInfo)
+				}
+			}
+		}
+	}
+
+	var records []record.Record
+	if len(certificates) > 0 {
+		// Step 5: Generate the certificates record
+		records = append(records, record.Record{
+			Name: "config/ingress/certificates",
+			Item: record.JSONMarshaller{Object: certificates},
+		})
 	}
 
-	return certInfos, errors
+	return records, nil
 }
 
-func certificateInfoFromSecret(secret *v1.Secret) (*IngressCertificateInfo, error) {
+func getCertificateInfoFromSecret(ctx context.Context, coreClient corev1client.CoreV1Interface, namespace, secretName string) (*CertificateInfo, error) {
+	secret, err := coreClient.Secrets(namespace).Get(ctx, secretName, metav1.GetOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to get secret '%s' in namespace '%s': %v", secretName, namespace, err)
+	}
+
 	crtData, found := secret.Data["tls.crt"]
 	if !found {
-		return nil, fmt.Errorf("'tls.crt' not found")
+		return nil, fmt.Errorf("'tls.crt' not found in secret '%s' in namespace '%s'", secretName, namespace)
 	}
 
 	block, _ := pem.Decode(crtData)
 	if block == nil {
-		return nil, fmt.Errorf("unable to decode certificate (x509)")
+		return nil, fmt.Errorf("unable to decode certificate (x509) from secret '%s' in namespace '%s'", secretName, namespace)
 	}
 
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to parse certificate from secret '%s' in namespace '%s': %v", secretName, namespace, err)
 	}
 
-	return &IngressCertificateInfo{
-		Name:      secret.Name,
-		NotBefore: cert.NotBefore,
-		NotAfter:  cert.NotAfter,
+	return &CertificateInfo{
+		Name:      secretName,
+		NotBefore: metav1.NewTime(cert.NotBefore),
+		NotAfter:  metav1.NewTime(cert.NotAfter),
+		Controllers: []ControllerInfo{
+			{Name: "router", Namespace: namespace},
+		},
 	}, nil
 }

pkg/gatherers/clusterconfig/gather_cluster_ingress_certificates_test.go

@@ -0,0 +1,115 @@
+package clusterconfig
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/runtime"
+
+	configv1 "github.com/openshift/api/config/v1"
+	configfake "github.com/openshift/client-go/config/clientset/versioned/fake"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corefake "k8s.io/client-go/kubernetes/fake"
+
+	"github.com/openshift/insights-operator/pkg/record"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_gatherClusterIngressCertificates(t *testing.T) {
+	certData := `
+-----BEGIN CERTIFICATE-----
+MIIDazCCAlOgAwIBAgIUfTstqHMAhGLL+j3n6pmwLw8vt84wDQYJKoZIhvcNAQEL
+BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
+GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yNDAzMDYxMjU5MTFaFw0yNTAz
+MDYxMjU5MTFaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
+HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB
+AQUAA4IBDwAwggEKAoIBAQDoqkPZMeMi5qjkG384ZwpAc3QScOGYBWOEDFAioq5C
+YhtDGBSMq2VwS0r8RvEEhbebvXuH5PLcIuEVO/MZRQD9gSacCfLlMLRKZYpv168m
+KUYyhx1bXKmUlbQxCnpAPZ7nf14A3Pb0TzLfsKjoUdUOv/1eorA6+oU78StWx/Nt
+W94ad9n3o+cjiMPu/RS3g9b+x07bG5mFYuzpWk/Svb5Lb42g8AtonzqFJBbhlStU
+A+9UyzmyXMeTlbI9fFmku7mb5Uq0SZ8jhpH+fyCoQOxefTfVrvjrkkdavDn43hjz
+5hCwGJ3mV96MU9hh398oBguOHaJ6V3/UHtW1spsFY83RAgMBAAGjUzBRMB0GA1Ud
+DgQWBBR/zvuHjFadvifzAGBHegOxmRXnCzAfBgNVHSMEGDAWgBR/zvuHjFadvifz
+AGBHegOxmRXnCzAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBs
+E2U+jQzJuEt9e6UEnS1T0cb2NxaGb7CYsSX0TjZK1VgloAKbnxaCLjRruTOOwfm6
+s5CFzFjJoIhUASzoA295Np2AR0UEYr5fendIjKCztCMlpj0fp92jFL6/RZWNGM1A
+qECHYtZckeqJjg9vUdfHtiBRoyEHJUJ/tsDDlslwzocdJUqKL8V6KerZsh5SIAkS
+rJ8EgVyDvwQaLPQMttjk62croI1Wi3FLmkvvtTbNcMgTnVhFfGjyHOiGnQeBfqax
+5P0VBuAUCihegskKEUCJB8HFPkC4hqbrEk0+psQ2Gm8kjoll/SpltFLS77Xjhrz9
+1qaiDHuWnUSifz6SGpWr
+-----END CERTIFICATE-----
+`
+
+	tests := []struct {
+		name               string
+		ingressDefinitions []configv1.Ingress
+		secretData         map[string][]byte
+		wantRecords        []record.Record
+		wantErrCount       int
+	}{
+		{
+			name: "successful retrieval cluster ingress certificates",
+			ingressDefinitions: []configv1.Ingress{
+				{ObjectMeta: metav1.ObjectMeta{Name: "example-ingress", Namespace: "openshift-ingress-operator"}},
+			},
+			secretData: map[string][]byte{
+				"tls.crt": []byte(certData),
+			},
+			wantRecords: []record.Record{
+				{
+					Name: "config/ingress/openshift-ingress-operator/ingress_certificates.json",
+					Item: record.JSONMarshaller{
+						Object: []IngressControllerInfo{
+							{
+								Name: "example-ingress",
+								OperatorGeneratedCertificate: []IngressCertificateInfo{
+									{
+										Name:      "router-ca",
+										NotBefore: time.Date(2024, time.March, 6, 12, 59, 11, 0, time.UTC),
+										NotAfter:  time.Date(2025, time.March, 6, 12, 59, 11, 0, time.UTC),
+									},
+								},
+								CustomCertificates: []IngressCertificateInfo{},
+							},
+						},
+					},
+				},
+			},
+			wantErrCount: 0,
+		},
+		{
+			name: "failed retrieval cluster ingress certificates",
+			ingressDefinitions: []configv1.Ingress{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "example-ingress",
+					},
+					Spec: configv1.IngressSpec{},
+				},
+			},
+			secretData:   map[string][]byte{},
+			wantRecords:  nil,
+			wantErrCount: 1, // There should be an error due to missing 'tls.crt'
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			objects := make([]runtime.Object, len(tt.ingressDefinitions))
+			for i, ingress := range tt.ingressDefinitions {
+				objects[i] = &ingress
+			}
+			configClient := configfake.NewSimpleClientset(objects...)
+			coreClient := corefake.NewSimpleClientset(&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "router-ca", Namespace: "openshift-ingress-operator"},
+				Data:       tt.secretData,
+			})
+
+			records, errs := gatherClusterIngressCertificates(context.TODO(), coreClient.CoreV1(), configClient.ConfigV1())
+			assert.Equal(t, tt.wantRecords, records)
+			assert.Len(t, errs, tt.wantErrCount)
+		})
+	}
+}