Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-35727: Ingress controller related certificates' validate dates gathering #945

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ linters:
disable-all: true
enable:
- bodyclose
- deadcode
- dogsled
- dupl
- errcheck
Expand All @@ -90,13 +89,11 @@ linters:
- noctx
- rowserrcheck
- staticcheck
- structcheck
- stylecheck
- typecheck
- unconvert
- unparam
- unused
- varcheck
ncaak marked this conversation as resolved.
Show resolved Hide resolved
- whitespace

# don't enable:
Expand Down
28 changes: 28 additions & 0 deletions docs/gathered-data.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,34 @@ None
None


## ClusterIngressCertificates

Collects the certificate's NotBefore and NotAfter dates from the cluster's ingress controller certificates.
It also collects the name and namespace of any Ingress Controllers using the certificates.

### API Reference
- https://docs.openshift.com/container-platform/4.13/rest_api/operator_apis/ingresscontroller-operator-openshift-io-v1.html
- https://docs.openshift.com/container-platform/4.13/rest_api/security_apis/secret-v1.html

### Sample data
- [docs/insights-archive-sample/aggregated/ingress_controllers_certs.json](./insights-archive-sample/aggregated/ingress_controllers_certs.json)

### Location in archive
- `aggregated/ingress_controllers_certs.json`

### Config ID
`clusterconfig/ingress_certificates`

### Released version
- 4.17

### Backported versions
TBD

### Changes
None


## ClusterNetwork

Collects the cluster Network with cluster name.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[
{
"name": "router-ca",
"namespace": "openshift-ingress-operator",
"not_before": "2024-06-07T07:27:34Z",
"not_after": "2026-06-07T07:27:35Z",
"controllers": [
{
"name": "test-ingresscontroller-name",
"namespace": "openshift-ingress-operator"
}
]
},
{
"name": "router-certs-default",
"namespace": "openshift-ingress",
"not_before": "2024-06-07T07:27:35Z",
"not_after": "2026-06-07T07:27:36Z",
"controllers": []
}
]
1 change: 1 addition & 0 deletions pkg/gatherers/clusterconfig/clusterconfig_gatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var gatheringFunctions = map[string]gathererFuncPtr{
"image_registries": (*Gatherer).GatherClusterImageRegistry,
"infrastructures": (*Gatherer).GatherClusterInfrastructure,
"ingress": (*Gatherer).GatherClusterIngress,
"ingress_certificates": (*Gatherer).GatherClusterIngressCertificates,
"install_plans": (*Gatherer).GatherInstallPlans,
"jaegers": (*Gatherer).GatherJaegerCR,
"kube_controller_manager_logs": (*Gatherer).GatherKubeControllerManagerLogs,
Expand Down
239 changes: 239 additions & 0 deletions pkg/gatherers/clusterconfig/gather_cluster_ingress_certificates.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
package clusterconfig

import (
"context"
"crypto/x509"
"encoding/pem"
"fmt"
"time"

operatorclient "github.com/openshift/client-go/operator/clientset/versioned"

"k8s.io/klog/v2"

"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"
)

const ingressCertificatesLimits = 64

type CertificateInfo struct {
Name string `json:"name"`
Namespace string `json:"namespace"`
NotBefore time.Time `json:"not_before"`
NotAfter time.Time `json:"not_after"`
Controllers []ControllerInfo `json:"controllers"`
}

type ControllerInfo struct {
Name string `json:"name"`
Namespace string `json:"namespace"`
}

type CertificateInfoKey struct {
Name string `json:"name"`
Namespace string `json:"namespace"`
}

// GatherClusterIngressCertificates Collects the certificate's NotBefore and NotAfter dates from the cluster's ingress controller certificates.
// It also collects the name and namespace of any Ingress Controllers using the certificates.
//
// ### API Reference
// - https://docs.openshift.com/container-platform/4.13/rest_api/operator_apis/ingresscontroller-operator-openshift-io-v1.html
// - https://docs.openshift.com/container-platform/4.13/rest_api/security_apis/secret-v1.html
//
// ### Sample data
// - docs/insights-archive-sample/aggregated/ingress_controllers_certs.json
//
// ### Location in archive
// - `aggregated/ingress_controllers_certs.json`
//
// ### Config ID
// `clusterconfig/ingress_certificates`
//
// ### Released version
// - 4.17
//
// ### Backported versions
// TBD
//
// ### Changes
// None
func (g *Gatherer) GatherClusterIngressCertificates(ctx context.Context) ([]record.Record, []error) {
const Filename = "aggregated/ingress_controllers_certs"

gatherKubeClient, err := kubernetes.NewForConfig(g.gatherProtoKubeConfig)
if err != nil {
return nil, []error{err}
}

operatorClient, err := operatorclient.NewForConfig(g.gatherKubeConfig)
if err != nil {
return nil, []error{err}
}

certificates, errs := gatherClusterIngressCertificates(ctx, gatherKubeClient.CoreV1(), operatorClient)
if len(errs) > 0 {
return nil, errs
}

return []record.Record{{
Name: Filename,
Item: record.JSONMarshaller{Object: certificates},
}}, nil
}

func gatherClusterIngressCertificates(
ctx context.Context,
coreClient corev1client.CoreV1Interface,
operatorClient operatorclient.Interface) ([]*CertificateInfo, []error) {
//
var ingressAllowedNS = [2]string{"openshift-ingress-operator", "openshift-ingress"}

var certificatesInfo = make(map[CertificateInfoKey]*CertificateInfo)
var errs []error

// Step 1: Collect router-ca and router-certs-default
rCAinfo, err := getRouterCACertInfo(ctx, coreClient)
if err != nil {
errs = append(errs, err)
}
if rCAinfo != nil {
certificatesInfo[infereKey(rCAinfo)] = rCAinfo
}

rCDinfo, err := getRouterCertsDefaultCertInfo(ctx, coreClient)
if err != nil {
errs = append(errs, err)
}
if rCDinfo != nil {
certificatesInfo[infereKey(rCDinfo)] = rCDinfo
}

// Step 2: List all Ingress Controllers
for _, namespace := range ingressAllowedNS {
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
}
if err != nil {
errs = append(errs, err)
continue
}

// Step 3: Filter Ingress Controllers with spec.defaultCertificate and get certificate info
for i := range controllers.Items {
controller := controllers.Items[i]

// Step 4: Check the certificate limits
if len(certificatesInfo) >= ingressCertificatesLimits {
klog.V(2).Infof("Reached the limit of ingress certificates (%d), skipping additional certificates", ingressCertificatesLimits)
ncaak marked this conversation as resolved.
Show resolved Hide resolved
break
}

if controller.Spec.DefaultCertificate != nil {
secretName := controller.Spec.DefaultCertificate.Name
certInfo, certErr := getCertificateInfoFromSecret(ctx, coreClient, namespace, secretName)
if certErr != nil {
errs = append(errs, certErr)
continue
}

// Step 5: Add certificate info to the certificates list
c, exists := certificatesInfo[infereKey(certInfo)]
if exists {
c.Controllers = append(c.Controllers,
ControllerInfo{Name: controller.Name, Namespace: controller.Namespace},
)
//
} else {
certInfo.Controllers = append(certInfo.Controllers,
ControllerInfo{Name: controller.Name, Namespace: controller.Namespace},
)
certificatesInfo[infereKey(certInfo)] = certInfo
}
}
}
}

var ci []*CertificateInfo
if len(certificatesInfo) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also not necessary, but let's not run the CI again :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aw yeah... After the refactor of the inner code the conditional is of no use :(

I'll leave it marked in my local repo for the next PR to include it as an improvement. Thanks.

// Step 6: Generate the certificates record
ci = make([]*CertificateInfo, len(certificatesInfo))
i := 0
for _, v := range certificatesInfo {
ci[i] = v
i++
ncaak marked this conversation as resolved.
Show resolved Hide resolved
}
}

return ci, errs
}

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 in secret '%s' in namespace '%s'", secretName, namespace)
}

block, _ := pem.Decode(crtData)
if block == nil {
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, fmt.Errorf("failed to parse certificate from secret '%s' in namespace '%s': %v", secretName, namespace, err)
}

return &CertificateInfo{
Name: secretName,
Namespace: namespace,
NotBefore: cert.NotBefore,
NotAfter: cert.NotAfter,
Controllers: []ControllerInfo{},
}, nil
}

func getRouterCACertInfo(ctx context.Context, coreClient corev1client.CoreV1Interface) (*CertificateInfo, error) {
const (
routerCASecret string = "router-ca"
routerCANamespace string = "openshift-ingress-operator"
)
certInfo, err := getCertificateInfoFromSecret(ctx, coreClient, routerCANamespace, routerCASecret)
if err != nil {
return nil, err
}

return certInfo, nil
}

func getRouterCertsDefaultCertInfo(ctx context.Context, coreClient corev1client.CoreV1Interface) (*CertificateInfo, error) {
const (
routerCertsSecret string = "router-certs-default"
routerCertsNamespace string = "openshift-ingress"
)
certInfo, err := getCertificateInfoFromSecret(ctx, coreClient, routerCertsNamespace, routerCertsSecret)
if err != nil {
return nil, err
}

return certInfo, nil
ncaak marked this conversation as resolved.
Show resolved Hide resolved
}

func infereKey(info *CertificateInfo) CertificateInfoKey {
ncaak marked this conversation as resolved.
Show resolved Hide resolved
return CertificateInfoKey{Name: info.Name, Namespace: info.Namespace}
}
Loading