Skip to content

Commit b718b24

Browse files
ncaakRicardo Lüders
ncaak
and
Ricardo Lüders
authoredJun 18, 2024··
OCPBUGS-35727: Ingress controller related certificates' validate dates gathering (#945)
* feat: ingress certificates gather * refactor: invert ingress certificate logic * refactor: better data serialization * refactor: adjust cert check * chore: remove deprecated linters * fix: remove hardcored test data * fix: missing controller for new certs * Minor refactor filename and time format * (draft) Add new unit test for getCertificateInfoFromSecret func * Refactor unit tests for gatherClusterIngressCertificates func * Add new unit test for custom certificate and minor refactor * Add new unit test for failure in cluster certs and errors control * Refactor cert match logic to a map and change on signature to improve unit tests * Fix unit tests to adapt better to gather function return * Fix linting * Add documentation for ClusterIngressCertificates gatherer * Add documentation for ClusterIngressCertificates gatherer * Fix logline moved to error trigger * Rollback golangci configuration * Refactor function naming and minor slice accesses --------- Co-authored-by: Ricardo Lüders <[email protected]>
1 parent c712388 commit b718b24

File tree

5 files changed

+481
-0
lines changed

5 files changed

+481
-0
lines changed
 

‎docs/gathered-data.md

+28
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,34 @@ None
418418
None
419419

420420

421+
## ClusterIngressCertificates
422+
423+
Collects the certificate's NotBefore and NotAfter dates from the cluster's ingress controller certificates.
424+
It also collects the name and namespace of any Ingress Controllers using the certificates.
425+
426+
### API Reference
427+
- https://docs.openshift.com/container-platform/4.13/rest_api/operator_apis/ingresscontroller-operator-openshift-io-v1.html
428+
- https://docs.openshift.com/container-platform/4.13/rest_api/security_apis/secret-v1.html
429+
430+
### Sample data
431+
- [docs/insights-archive-sample/aggregated/ingress_controllers_certs.json](./insights-archive-sample/aggregated/ingress_controllers_certs.json)
432+
433+
### Location in archive
434+
- `aggregated/ingress_controllers_certs.json`
435+
436+
### Config ID
437+
`clusterconfig/ingress_certificates`
438+
439+
### Released version
440+
- 4.17
441+
442+
### Backported versions
443+
TBD
444+
445+
### Changes
446+
None
447+
448+
421449
## ClusterNetwork
422450

423451
Collects the cluster Network with cluster name.

‎docs/insights-archive-sample/aggregated/ingress_controllers_certs.json

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
[
2+
{
3+
"name": "router-ca",
4+
"namespace": "openshift-ingress-operator",
5+
"not_before": "2024-06-07T07:27:34Z",
6+
"not_after": "2026-06-07T07:27:35Z",
7+
"controllers": [
8+
{
9+
"name": "test-ingresscontroller-name",
10+
"namespace": "openshift-ingress-operator"
11+
}
12+
]
13+
},
14+
{
15+
"name": "router-certs-default",
16+
"namespace": "openshift-ingress",
17+
"not_before": "2024-06-07T07:27:35Z",
18+
"not_after": "2026-06-07T07:27:36Z",
19+
"controllers": []
20+
}
21+
]

‎pkg/gatherers/clusterconfig/clusterconfig_gatherer.go

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ var gatheringFunctions = map[string]gathererFuncPtr{
4545
"image_registries": (*Gatherer).GatherClusterImageRegistry,
4646
"infrastructures": (*Gatherer).GatherClusterInfrastructure,
4747
"ingress": (*Gatherer).GatherClusterIngress,
48+
"ingress_certificates": (*Gatherer).GatherClusterIngressCertificates,
4849
"install_plans": (*Gatherer).GatherInstallPlans,
4950
"jaegers": (*Gatherer).GatherJaegerCR,
5051
"kube_controller_manager_logs": (*Gatherer).GatherKubeControllerManagerLogs,

‎pkg/gatherers/clusterconfig/gather_cluster_ingress_certificates.go

+225
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
package clusterconfig
2+
3+
import (
4+
"context"
5+
"crypto/x509"
6+
"encoding/pem"
7+
"fmt"
8+
"time"
9+
10+
operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
11+
12+
"k8s.io/klog/v2"
13+
14+
"k8s.io/apimachinery/pkg/api/errors"
15+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
"k8s.io/client-go/kubernetes"
17+
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
18+
19+
"github.com/openshift/insights-operator/pkg/record"
20+
)
21+
22+
const ingressCertificatesLimits = 64
23+
24+
type CertificateInfo struct {
25+
Name string `json:"name"`
26+
Namespace string `json:"namespace"`
27+
NotBefore time.Time `json:"not_before"`
28+
NotAfter time.Time `json:"not_after"`
29+
Controllers []ControllerInfo `json:"controllers"`
30+
}
31+
32+
type ControllerInfo struct {
33+
Name string `json:"name"`
34+
Namespace string `json:"namespace"`
35+
}
36+
37+
type CertificateInfoKey struct {
38+
Name string `json:"name"`
39+
Namespace string `json:"namespace"`
40+
}
41+
42+
// GatherClusterIngressCertificates Collects the certificate's NotBefore and NotAfter dates from the cluster's ingress controller certificates.
43+
// It also collects the name and namespace of any Ingress Controllers using the certificates.
44+
//
45+
// ### API Reference
46+
// - https://docs.openshift.com/container-platform/4.13/rest_api/operator_apis/ingresscontroller-operator-openshift-io-v1.html
47+
// - https://docs.openshift.com/container-platform/4.13/rest_api/security_apis/secret-v1.html
48+
//
49+
// ### Sample data
50+
// - docs/insights-archive-sample/aggregated/ingress_controllers_certs.json
51+
//
52+
// ### Location in archive
53+
// - `aggregated/ingress_controllers_certs.json`
54+
//
55+
// ### Config ID
56+
// `clusterconfig/ingress_certificates`
57+
//
58+
// ### Released version
59+
// - 4.17
60+
//
61+
// ### Backported versions
62+
// TBD
63+
//
64+
// ### Changes
65+
// None
66+
func (g *Gatherer) GatherClusterIngressCertificates(ctx context.Context) ([]record.Record, []error) {
67+
const Filename = "aggregated/ingress_controllers_certs"
68+
69+
gatherKubeClient, err := kubernetes.NewForConfig(g.gatherProtoKubeConfig)
70+
if err != nil {
71+
return nil, []error{err}
72+
}
73+
74+
operatorClient, err := operatorclient.NewForConfig(g.gatherKubeConfig)
75+
if err != nil {
76+
return nil, []error{err}
77+
}
78+
79+
certificates, errs := gatherClusterIngressCertificates(ctx, gatherKubeClient.CoreV1(), operatorClient)
80+
81+
return []record.Record{{
82+
Name: Filename,
83+
Item: record.JSONMarshaller{Object: certificates},
84+
}}, errs
85+
}
86+
87+
func gatherClusterIngressCertificates(
88+
ctx context.Context,
89+
coreClient corev1client.CoreV1Interface,
90+
operatorClient operatorclient.Interface) ([]*CertificateInfo, []error) {
91+
//
92+
var ingressAllowedNS = [2]string{"openshift-ingress-operator", "openshift-ingress"}
93+
94+
var certificatesInfo = make(map[CertificateInfoKey]*CertificateInfo)
95+
var errs []error
96+
97+
// Step 1: Collect router-ca and router-certs-default
98+
rCAinfo, err := getRouterCACertInfo(ctx, coreClient)
99+
if err != nil {
100+
errs = append(errs, err)
101+
}
102+
if rCAinfo != nil {
103+
certificatesInfo[newCertificateInfoKey(rCAinfo)] = rCAinfo
104+
}
105+
106+
rCDinfo, err := getRouterCertsDefaultCertInfo(ctx, coreClient)
107+
if err != nil {
108+
errs = append(errs, err)
109+
}
110+
if rCDinfo != nil {
111+
certificatesInfo[newCertificateInfoKey(rCDinfo)] = rCDinfo
112+
}
113+
114+
// Step 2: List all Ingress Controllers
115+
for _, namespace := range ingressAllowedNS {
116+
controllers, err := operatorClient.OperatorV1().IngressControllers(namespace).List(ctx, metav1.ListOptions{})
117+
if errors.IsNotFound(err) {
118+
klog.V(2).Infof("Ingress Controllers not found in '%s' namespace", namespace)
119+
continue
120+
}
121+
if err != nil {
122+
errs = append(errs, err)
123+
continue
124+
}
125+
126+
// Step 3: Filter Ingress Controllers with spec.defaultCertificate and get certificate info
127+
for i := range controllers.Items {
128+
controller := controllers.Items[i]
129+
130+
// Step 4: Check the certificate limits
131+
if len(certificatesInfo) >= ingressCertificatesLimits {
132+
errs = append(errs, fmt.Errorf(
133+
"reached the limit of ingress certificates (%d), skipping additional certificates",
134+
ingressCertificatesLimits))
135+
break
136+
}
137+
138+
if controller.Spec.DefaultCertificate != nil {
139+
secretName := controller.Spec.DefaultCertificate.Name
140+
certInfo, certErr := getCertificateInfoFromSecret(ctx, coreClient, namespace, secretName)
141+
if certErr != nil {
142+
errs = append(errs, certErr)
143+
continue
144+
}
145+
146+
// Step 5: Add certificate info to the certificates list
147+
c, exists := certificatesInfo[newCertificateInfoKey(certInfo)]
148+
if exists {
149+
c.Controllers = append(c.Controllers,
150+
ControllerInfo{Name: controller.Name, Namespace: controller.Namespace},
151+
)
152+
//
153+
} else {
154+
certInfo.Controllers = append(certInfo.Controllers,
155+
ControllerInfo{Name: controller.Name, Namespace: controller.Namespace},
156+
)
157+
certificatesInfo[newCertificateInfoKey(certInfo)] = certInfo
158+
}
159+
}
160+
}
161+
}
162+
163+
var ci []*CertificateInfo
164+
if len(certificatesInfo) > 0 {
165+
// Step 6: Generate the certificates record
166+
for _, v := range certificatesInfo {
167+
ci = append(ci, v)
168+
}
169+
}
170+
171+
return ci, errs
172+
}
173+
174+
func getCertificateInfoFromSecret(
175+
ctx context.Context, coreClient corev1client.CoreV1Interface,
176+
namespace, secretName string) (*CertificateInfo, error) {
177+
//
178+
secret, err := coreClient.Secrets(namespace).Get(ctx, secretName, metav1.GetOptions{})
179+
if err != nil {
180+
return nil, fmt.Errorf("failed to get secret '%s' in namespace '%s': %v", secretName, namespace, err)
181+
}
182+
183+
crtData, found := secret.Data["tls.crt"]
184+
if !found {
185+
return nil, fmt.Errorf("'tls.crt' not found in secret '%s' in namespace '%s'", secretName, namespace)
186+
}
187+
188+
block, _ := pem.Decode(crtData)
189+
if block == nil {
190+
return nil, fmt.Errorf("unable to decode certificate (x509) from secret '%s' in namespace '%s'", secretName, namespace)
191+
}
192+
193+
cert, err := x509.ParseCertificate(block.Bytes)
194+
if err != nil {
195+
return nil, fmt.Errorf("failed to parse certificate from secret '%s' in namespace '%s': %v", secretName, namespace, err)
196+
}
197+
198+
return &CertificateInfo{
199+
Name: secretName,
200+
Namespace: namespace,
201+
NotBefore: cert.NotBefore,
202+
NotAfter: cert.NotAfter,
203+
Controllers: []ControllerInfo{},
204+
}, nil
205+
}
206+
207+
func getRouterCACertInfo(ctx context.Context, coreClient corev1client.CoreV1Interface) (*CertificateInfo, error) {
208+
const (
209+
routerCASecret string = "router-ca"
210+
routerCANamespace string = "openshift-ingress-operator"
211+
)
212+
return getCertificateInfoFromSecret(ctx, coreClient, routerCANamespace, routerCASecret)
213+
}
214+
215+
func getRouterCertsDefaultCertInfo(ctx context.Context, coreClient corev1client.CoreV1Interface) (*CertificateInfo, error) {
216+
const (
217+
routerCertsSecret string = "router-certs-default"
218+
routerCertsNamespace string = "openshift-ingress"
219+
)
220+
return getCertificateInfoFromSecret(ctx, coreClient, routerCertsNamespace, routerCertsSecret)
221+
}
222+
223+
func newCertificateInfoKey(info *CertificateInfo) CertificateInfoKey {
224+
return CertificateInfoKey{Name: info.Name, Namespace: info.Namespace}
225+
}

‎pkg/gatherers/clusterconfig/gather_cluster_ingress_certificates_test.go

+206
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
package clusterconfig
2+
3+
import (
4+
"context"
5+
"crypto/x509"
6+
"encoding/pem"
7+
"testing"
8+
9+
operatorv1 "github.com/openshift/api/operator/v1"
10+
operatorfake "github.com/openshift/client-go/operator/clientset/versioned/fake"
11+
12+
corev1 "k8s.io/api/core/v1"
13+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
corefake "k8s.io/client-go/kubernetes/fake"
15+
16+
"github.com/stretchr/testify/assert"
17+
)
18+
19+
func Test_gatherClusterIngressCertificates(t *testing.T) {
20+
// Mocks
21+
mockBytes, mockX509 := getCertMock()
22+
23+
tests := []struct {
24+
name string
25+
ingressDef []operatorv1.IngressController
26+
secretDef []corev1.Secret
27+
wantInfo []*CertificateInfo
28+
wantErrCount int
29+
}{
30+
{
31+
name: "Custom Ingress controller with a cluster certificate is added to the collection",
32+
ingressDef: []operatorv1.IngressController{{
33+
ObjectMeta: metav1.ObjectMeta{Name: "test-ingress-controller", Namespace: "openshift-ingress-operator"},
34+
Spec: operatorv1.IngressControllerSpec{DefaultCertificate: &corev1.LocalObjectReference{
35+
Name: "router-ca"},
36+
}}},
37+
secretDef: []corev1.Secret{{
38+
ObjectMeta: metav1.ObjectMeta{Name: "router-ca", Namespace: "openshift-ingress-operator"},
39+
Data: map[string][]byte{"tls.crt": mockBytes},
40+
}, {
41+
ObjectMeta: metav1.ObjectMeta{Name: "router-certs-default", Namespace: "openshift-ingress"},
42+
Data: map[string][]byte{"tls.crt": mockBytes},
43+
}},
44+
wantInfo: []*CertificateInfo{{
45+
Name: "router-ca",
46+
Namespace: "openshift-ingress-operator",
47+
NotBefore: mockX509.NotBefore, NotAfter: mockX509.NotAfter,
48+
Controllers: []ControllerInfo{
49+
{Name: "test-ingress-controller", Namespace: "openshift-ingress-operator"},
50+
},
51+
}, {
52+
Name: "router-certs-default",
53+
Namespace: "openshift-ingress",
54+
NotBefore: mockX509.NotBefore, NotAfter: mockX509.NotAfter,
55+
Controllers: []ControllerInfo{},
56+
}},
57+
}, {
58+
name: "Custom Ingress Controller with custom certificate adds a new entry to the collection",
59+
ingressDef: []operatorv1.IngressController{{
60+
ObjectMeta: metav1.ObjectMeta{Name: "test-custom-ingress", Namespace: "openshift-ingress-operator"},
61+
Spec: operatorv1.IngressControllerSpec{DefaultCertificate: &corev1.LocalObjectReference{
62+
Name: "test-custom-secret"},
63+
}}},
64+
secretDef: []corev1.Secret{{
65+
ObjectMeta: metav1.ObjectMeta{Name: "router-ca", Namespace: "openshift-ingress-operator"},
66+
Data: map[string][]byte{"tls.crt": mockBytes},
67+
}, {
68+
ObjectMeta: metav1.ObjectMeta{Name: "router-certs-default", Namespace: "openshift-ingress"},
69+
Data: map[string][]byte{"tls.crt": mockBytes},
70+
}, {
71+
ObjectMeta: metav1.ObjectMeta{Name: "test-custom-secret", Namespace: "openshift-ingress-operator"},
72+
Data: map[string][]byte{"tls.crt": mockBytes},
73+
}},
74+
wantInfo: []*CertificateInfo{{
75+
Name: "router-ca",
76+
Namespace: "openshift-ingress-operator",
77+
NotBefore: mockX509.NotBefore, NotAfter: mockX509.NotAfter,
78+
Controllers: []ControllerInfo{},
79+
}, {
80+
Name: "router-certs-default",
81+
Namespace: "openshift-ingress",
82+
NotBefore: mockX509.NotBefore, NotAfter: mockX509.NotAfter,
83+
Controllers: []ControllerInfo{},
84+
}, {
85+
Name: "test-custom-secret",
86+
Namespace: "openshift-ingress-operator",
87+
NotBefore: mockX509.NotBefore, NotAfter: mockX509.NotAfter,
88+
Controllers: []ControllerInfo{
89+
{Name: "test-custom-ingress", Namespace: "openshift-ingress-operator"},
90+
},
91+
}},
92+
}, {
93+
name: "Cluster default certificates that fail to validate return an error per cert",
94+
secretDef: []corev1.Secret{{
95+
ObjectMeta: metav1.ObjectMeta{Name: "router-ca", Namespace: "openshift-ingress-operator"},
96+
}, {
97+
ObjectMeta: metav1.ObjectMeta{Name: "router-certs-default", Namespace: "openshift-ingress"},
98+
}},
99+
wantErrCount: 2,
100+
},
101+
}
102+
103+
for _, tc := range tests {
104+
t.Run(tc.name, func(t *testing.T) {
105+
// Given
106+
operatorClient := operatorfake.NewSimpleClientset()
107+
for _, ic := range tc.ingressDef {
108+
assert.NoError(t,
109+
operatorClient.Tracker().Add(ic.DeepCopy()))
110+
}
111+
coreClient := corefake.NewSimpleClientset()
112+
for _, sec := range tc.secretDef {
113+
assert.NoError(t,
114+
coreClient.Tracker().Add(&sec))
115+
}
116+
117+
// When
118+
test, errs := gatherClusterIngressCertificates(context.TODO(), coreClient.CoreV1(), operatorClient)
119+
120+
// Assert
121+
assert.ElementsMatch(t, tc.wantInfo, test)
122+
assert.Len(t, errs, tc.wantErrCount)
123+
})
124+
}
125+
}
126+
127+
func Test_getCertificateInfoFromSecret(t *testing.T) {
128+
// Mocks
129+
mockBytes, mockX509 := getCertMock()
130+
131+
testCases := []struct {
132+
name string
133+
secret *corev1.Secret
134+
expected *CertificateInfo
135+
expectErr bool
136+
secretName string
137+
secretNamespace string
138+
}{
139+
{
140+
name: "a Secret containing a certificate returns a CertificateInfo struct",
141+
secretName: "test",
142+
secretNamespace: "test-namespace",
143+
secret: &corev1.Secret{
144+
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "test-namespace"},
145+
Data: map[string][]byte{"tls.crt": mockBytes},
146+
},
147+
expected: &CertificateInfo{
148+
Name: "test", Namespace: "test-namespace",
149+
NotBefore: mockX509.NotBefore, NotAfter: mockX509.NotAfter,
150+
Controllers: []ControllerInfo{},
151+
},
152+
}, {
153+
name: "a Secret without a certificate returns an error",
154+
secretName: "test",
155+
secretNamespace: "test-namespace",
156+
secret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "test-namespace"}},
157+
expectErr: true,
158+
},
159+
}
160+
161+
for _, tc := range testCases {
162+
t.Run(tc.name, func(t *testing.T) {
163+
// Given
164+
client := corefake.NewSimpleClientset(tc.secret)
165+
166+
// When
167+
test, err := getCertificateInfoFromSecret(
168+
context.Background(), client.CoreV1(), tc.secretNamespace, tc.secretName)
169+
170+
// Assert
171+
if tc.expectErr {
172+
assert.Error(t, err)
173+
} else {
174+
assert.NoError(t, err)
175+
}
176+
assert.EqualValues(t, tc.expected, test)
177+
})
178+
}
179+
}
180+
181+
func getCertMock() ([]byte, *x509.Certificate) {
182+
mockCertbytes := []byte(`-----BEGIN CERTIFICATE-----
183+
MIIDazCCAlOgAwIBAgIUfTstqHMAhGLL+j3n6pmwLw8vt84wDQYJKoZIhvcNAQEL
184+
BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
185+
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yNDAzMDYxMjU5MTFaFw0yNTAz
186+
MDYxMjU5MTFaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
187+
HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB
188+
AQUAA4IBDwAwggEKAoIBAQDoqkPZMeMi5qjkG384ZwpAc3QScOGYBWOEDFAioq5C
189+
YhtDGBSMq2VwS0r8RvEEhbebvXuH5PLcIuEVO/MZRQD9gSacCfLlMLRKZYpv168m
190+
KUYyhx1bXKmUlbQxCnpAPZ7nf14A3Pb0TzLfsKjoUdUOv/1eorA6+oU78StWx/Nt
191+
W94ad9n3o+cjiMPu/RS3g9b+x07bG5mFYuzpWk/Svb5Lb42g8AtonzqFJBbhlStU
192+
A+9UyzmyXMeTlbI9fFmku7mb5Uq0SZ8jhpH+fyCoQOxefTfVrvjrkkdavDn43hjz
193+
5hCwGJ3mV96MU9hh398oBguOHaJ6V3/UHtW1spsFY83RAgMBAAGjUzBRMB0GA1Ud
194+
DgQWBBR/zvuHjFadvifzAGBHegOxmRXnCzAfBgNVHSMEGDAWgBR/zvuHjFadvifz
195+
AGBHegOxmRXnCzAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBs
196+
E2U+jQzJuEt9e6UEnS1T0cb2NxaGb7CYsSX0TjZK1VgloAKbnxaCLjRruTOOwfm6
197+
s5CFzFjJoIhUASzoA295Np2AR0UEYr5fendIjKCztCMlpj0fp92jFL6/RZWNGM1A
198+
qECHYtZckeqJjg9vUdfHtiBRoyEHJUJ/tsDDlslwzocdJUqKL8V6KerZsh5SIAkS
199+
rJ8EgVyDvwQaLPQMttjk62croI1Wi3FLmkvvtTbNcMgTnVhFfGjyHOiGnQeBfqax
200+
5P0VBuAUCihegskKEUCJB8HFPkC4hqbrEk0+psQ2Gm8kjoll/SpltFLS77Xjhrz9
201+
1qaiDHuWnUSifz6SGpWr
202+
-----END CERTIFICATE-----`)
203+
b, _ := pem.Decode(mockCertbytes)
204+
x509cert, _ := x509.ParseCertificate(b.Bytes)
205+
return mockCertbytes, x509cert
206+
}

0 commit comments

Comments
 (0)
Please sign in to comment.