Skip to content

Commit a5f82f2

Browse files
swghoshswghosh
committed
Protect against arbitrary *-Degraded conditions as true
* deliberatly fail the e2e if we encounter any unexpected Degraded in Status.Conditions Signed-off-by: Swarup Ghosh <[email protected]>
1 parent f3377ae commit a5f82f2

File tree

7 files changed

+406
-123
lines changed

7 files changed

+406
-123
lines changed

‎test/e2e/certificates_test.go‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,7 @@ var _ = Describe("ACME Certificate", Ordered, func() {
6767

6868
BeforeEach(func() {
6969
By("waiting for operator status to become available")
70-
err := verifyOperatorStatusCondition(certmanageroperatorclient,
71-
[]string{certManagerControllerDeploymentControllerName,
72-
certManagerWebhookDeploymentControllerName,
73-
certManagerCAInjectorDeploymentControllerName},
74-
validOperatorStatusConditions)
70+
err := VerifyHealthyOperatorConditions(certmanageroperatorclient.OperatorV1alpha1())
7571
Expect(err).NotTo(HaveOccurred(), "Operator is expected to be available")
7672

7773
By("creating a test namespace")
@@ -195,7 +191,7 @@ var _ = Describe("ACME Certificate", Ordered, func() {
195191
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "credentials", "credentialsrequest_aws.yaml"), "")
196192

197193
By("waiting for cloud secret to be available")
198-
err := wait.PollUntilContextTimeout(context.TODO(), PollInterval, TestTimeout, true, func(context.Context) (bool, error) {
194+
err := wait.PollUntilContextTimeout(context.TODO(), slowPollInterval, highTimeout, true, func(context.Context) (bool, error) {
199195
_, err := loader.KubeClient.CoreV1().Secrets("cert-manager").Get(ctx, "aws-creds", metav1.GetOptions{})
200196
if err != nil {
201197
return false, nil
@@ -288,7 +284,7 @@ var _ = Describe("ACME Certificate", Ordered, func() {
288284
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "credentials", "credentialsrequest_aws.yaml"), "")
289285

290286
By("waiting for cloud secret to be available")
291-
err := wait.PollUntilContextTimeout(context.TODO(), PollInterval, TestTimeout, true, func(context.Context) (bool, error) {
287+
err := wait.PollUntilContextTimeout(context.TODO(), slowPollInterval, highTimeout, true, func(context.Context) (bool, error) {
292288
_, err := loader.KubeClient.CoreV1().Secrets("cert-manager").Get(ctx, "aws-creds", metav1.GetOptions{})
293289
if err != nil {
294290
return false, nil
@@ -488,7 +484,7 @@ var _ = Describe("ACME Certificate", Ordered, func() {
488484
By("Waiting for cloud secret to be available")
489485
// The name is defined cloud credential by the testdata YAML file.
490486
credentialSecret := "gcp-credentials"
491-
err := wait.PollUntilContextTimeout(context.TODO(), PollInterval, TestTimeout, true, func(context.Context) (bool, error) {
487+
err := wait.PollUntilContextTimeout(context.TODO(), slowPollInterval, highTimeout, true, func(context.Context) (bool, error) {
492488
_, err := loader.KubeClient.CoreV1().Secrets("cert-manager").Get(ctx, credentialSecret, metav1.GetOptions{})
493489
if err != nil {
494490
return false, nil
@@ -645,7 +641,7 @@ var _ = Describe("ACME Certificate", Ordered, func() {
645641
defer loader.KubeClient.NetworkingV1().Ingresses(ingress.Namespace).Delete(ctx, ingress.Name, metav1.DeleteOptions{})
646642

647643
By("checking TLS certificate contents")
648-
err = wait.PollUntilContextTimeout(context.TODO(), PollInterval, TestTimeout, true, func(context.Context) (bool, error) {
644+
err = wait.PollUntilContextTimeout(context.TODO(), slowPollInterval, highTimeout, true, func(context.Context) (bool, error) {
649645
secret, err := loader.KubeClient.CoreV1().Secrets(ingress.Namespace).Get(ctx, secretName, metav1.GetOptions{})
650646
tlsConfig, isvalid := library.GetTLSConfig(secret)
651647
if !isvalid {
@@ -686,11 +682,7 @@ var _ = Describe("Self-signed Certificate", Ordered, func() {
686682

687683
BeforeEach(func() {
688684
By("waiting for operator status to become available")
689-
err := verifyOperatorStatusCondition(certmanageroperatorclient,
690-
[]string{certManagerControllerDeploymentControllerName,
691-
certManagerWebhookDeploymentControllerName,
692-
certManagerCAInjectorDeploymentControllerName},
693-
validOperatorStatusConditions)
685+
err := VerifyHealthyOperatorConditions(certmanageroperatorclient.OperatorV1alpha1())
694686
Expect(err).NotTo(HaveOccurred(), "Operator is expected to be available")
695687
})
696688

@@ -768,7 +760,7 @@ var _ = Describe("Self-signed Certificate", Ordered, func() {
768760
Expect(err).NotTo(HaveOccurred())
769761

770762
By("certificate was renewed atleast once")
771-
err = verifyCertificateRenewed(ctx, cert.Spec.SecretName, ns.Name, time.Minute+5*time.Second) // using wait period of (1min+jitter)=65s
763+
err = verifyCertificateRenewed(ctx, cert.Spec.SecretName, ns.Name, time.Minute+slowPollInterval) // using wait period of (1min+jitter)=65s
772764
Expect(err).NotTo(HaveOccurred())
773765
})
774766
})

‎test/e2e/condition_matcher_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
//go:build e2e
2+
// +build e2e
3+
4+
package e2e
5+
6+
import (
7+
"regexp"
8+
9+
"context"
10+
"encoding/json"
11+
"fmt"
12+
"log"
13+
14+
apierrors "k8s.io/apimachinery/pkg/api/errors"
15+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
"k8s.io/apimachinery/pkg/util/wait"
17+
18+
opv1 "github.com/openshift/api/operator/v1"
19+
v1alpha1client "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned/typed/operator/v1alpha1"
20+
)
21+
22+
// ConditionMatcher tracks a regex pattern cond.Type with an expected cond.Status
23+
type ConditionMatcher struct {
24+
TypePattern *regexp.Regexp `json:"condition.Type"`
25+
ExpectedStatus opv1.ConditionStatus `json:"condition.Status"`
26+
27+
// Any when true will expect matcher to succeed
28+
// when atleast one conditions match, default is false when
29+
// matcher will succeed only on matching all pattern conditions.
30+
Any bool `json:"matcher.shouldMatchAny"`
31+
}
32+
33+
// MatchesType returns true if the provided Condition.Type satisfies regex pattern
34+
func (m *ConditionMatcher) MatchesType(cond *opv1.OperatorCondition) bool {
35+
return m.TypePattern.MatchString(cond.Type)
36+
}
37+
38+
// MatchesStatus returns true if the provided Condition.Status is same as ExpectedStatus
39+
func (m *ConditionMatcher) MatchesStatus(cond *opv1.OperatorCondition) bool {
40+
return cond.Status == m.ExpectedStatus
41+
}
42+
43+
func (m *ConditionMatcher) Matches(conditions []opv1.OperatorCondition) bool {
44+
matchCount := 0
45+
for _, cond := range conditions {
46+
if m.MatchesType(&cond) && m.MatchesStatus(&cond) {
47+
48+
if m.Any {
49+
return true
50+
}
51+
52+
matchCount += 1
53+
}
54+
55+
if m.MatchesType(&cond) && !m.MatchesStatus(&cond) {
56+
return false
57+
}
58+
}
59+
60+
return matchCount > 0
61+
}
62+
63+
const (
64+
MatchAnyCondition bool = true
65+
MatchAllConditions bool = false
66+
)
67+
68+
// PrefixAndMatchTypeTuple is a tuple containing
69+
// controller name (i.e. the condition prefix)
70+
// and the condition matcher type be ANY or ALL.
71+
type PrefixAndMatchTypeTuple struct {
72+
Prefix string
73+
ShouldMatchAny bool
74+
}
75+
76+
func GenerateConditionMatchers(tuples []PrefixAndMatchTypeTuple, expectedConditions map[string]opv1.ConditionStatus) []ConditionMatcher {
77+
matchers := make([]ConditionMatcher, len(tuples)*len(expectedConditions))
78+
79+
idx := 0
80+
for _, t := range tuples {
81+
for suffix, state := range expectedConditions {
82+
matchers[idx] = ConditionMatcher{
83+
TypePattern: regexp.MustCompile("^" + t.Prefix + ".*" + suffix + "$"),
84+
ExpectedStatus: state,
85+
Any: t.ShouldMatchAny,
86+
}
87+
idx += 1
88+
}
89+
}
90+
return matchers
91+
}
92+
93+
// verifyOperatorStatusCondition polls every few second to check if the status of each of the controllers
94+
// match with the expected conditions. It returns an error if a timeout (few mins) occurs or an error was
95+
// encountered which polls the status.
96+
func verifyOperatorStatusCondition(client v1alpha1client.OperatorV1alpha1Interface, conditionMatchers []ConditionMatcher) error {
97+
var lastFetchedConditions []opv1.OperatorCondition
98+
99+
err := wait.PollUntilContextTimeout(context.TODO(), fastPollInterval, lowTimeout, true, func(context.Context) (bool, error) {
100+
operator, err := client.CertManagers().Get(context.TODO(), "cluster", metav1.GetOptions{})
101+
if err != nil {
102+
if apierrors.IsNotFound(err) {
103+
return false, nil
104+
}
105+
return false, err
106+
}
107+
108+
if operator.DeletionTimestamp != nil {
109+
return false, nil
110+
}
111+
112+
lastFetchedConditions = operator.Status.DeepCopy().Conditions
113+
for _, matcher := range conditionMatchers {
114+
if !matcher.Matches(lastFetchedConditions) {
115+
116+
// [retry] false: NOT match as desired
117+
return false, nil
118+
}
119+
}
120+
121+
// [no-retry] true: when ALL match as desired
122+
return true, nil
123+
})
124+
125+
if err != nil {
126+
prettyConds, _ := json.Marshal(lastFetchedConditions)
127+
log.Printf("found status.conditions: %s", prettyConds)
128+
prettyMatchers, _ := json.Marshal(conditionMatchers)
129+
log.Printf("expected status.conditions to adhere with %s", prettyMatchers)
130+
131+
return fmt.Errorf("could not verify all status conditions as expected : %v", err)
132+
}
133+
134+
return nil
135+
}
136+
137+
func VerifyHealthyOperatorConditions(client v1alpha1client.OperatorV1alpha1Interface) error {
138+
return verifyOperatorStatusCondition(client,
139+
GenerateConditionMatchers(
140+
[]PrefixAndMatchTypeTuple{
141+
{certManagerController, MatchAllConditions},
142+
{certManagerWebhook, MatchAllConditions},
143+
{certManagerCAInjector, MatchAllConditions},
144+
},
145+
validOperatorStatusConditions,
146+
),
147+
)
148+
}

‎test/e2e/condition_matcher_unit_test.go‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
//go:build e2e
2+
// +build e2e
3+
4+
package e2e
5+
6+
import (
7+
"strings"
8+
"testing"
9+
10+
opv1 "github.com/openshift/api/operator/v1"
11+
operatorv1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
12+
fakecertmanclient "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned/fake"
13+
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/runtime"
16+
)
17+
18+
// TestVerifyOperatorStatusCondition unit test for verifyOperatorStatusCondition func
19+
func TestVerifyOperatorStatusCondition(t *testing.T) {
20+
controllerPrefix := "foo-controller"
21+
22+
newCertManagerObjectWithConditions := func(conditions ...opv1.OperatorCondition) *operatorv1alpha1.CertManager {
23+
return &operatorv1alpha1.CertManager{
24+
ObjectMeta: metav1.ObjectMeta{
25+
Name: "cluster",
26+
},
27+
Status: operatorv1alpha1.CertManagerStatus{
28+
OperatorStatus: opv1.OperatorStatus{
29+
Conditions: conditions,
30+
},
31+
},
32+
}
33+
}
34+
35+
tests := []struct {
36+
name string
37+
initialObjects []runtime.Object
38+
expectedConditions map[string]opv1.ConditionStatus
39+
matchAny bool
40+
expectError bool
41+
errorContains string
42+
}{
43+
{
44+
name: "One degraded is true but another is false using Any",
45+
expectedConditions: map[string]opv1.ConditionStatus{
46+
"Available": opv1.ConditionTrue,
47+
"Degraded": opv1.ConditionFalse,
48+
"Progressing": opv1.ConditionFalse,
49+
},
50+
initialObjects: []runtime.Object{
51+
newCertManagerObjectWithConditions(
52+
opv1.OperatorCondition{Type: controllerPrefix + "Available", Status: opv1.ConditionTrue},
53+
54+
opv1.OperatorCondition{Type: controllerPrefix + "Degraded", Status: opv1.ConditionFalse},
55+
opv1.OperatorCondition{Type: controllerPrefix + "-rand-Degraded", Status: opv1.ConditionTrue},
56+
57+
opv1.OperatorCondition{Type: controllerPrefix + "Progressing", Status: opv1.ConditionFalse},
58+
),
59+
},
60+
matchAny: true,
61+
expectError: false,
62+
errorContains: "context deadline exceeded",
63+
},
64+
{
65+
name: "Both degraded is false",
66+
expectedConditions: map[string]opv1.ConditionStatus{
67+
"Available": opv1.ConditionTrue,
68+
"Degraded": opv1.ConditionFalse,
69+
"Progressing": opv1.ConditionFalse,
70+
},
71+
initialObjects: []runtime.Object{
72+
newCertManagerObjectWithConditions(
73+
opv1.OperatorCondition{Type: controllerPrefix + "Available", Status: opv1.ConditionTrue},
74+
75+
opv1.OperatorCondition{Type: controllerPrefix + "Degraded", Status: opv1.ConditionFalse},
76+
opv1.OperatorCondition{Type: controllerPrefix + "-bar-Degraded", Status: opv1.ConditionFalse},
77+
78+
opv1.OperatorCondition{Type: controllerPrefix + "Progressing", Status: opv1.ConditionFalse},
79+
),
80+
},
81+
expectError: false,
82+
},
83+
{
84+
name: "One available is true but another is false using All",
85+
expectedConditions: map[string]opv1.ConditionStatus{
86+
"Available": opv1.ConditionTrue,
87+
"Degraded": opv1.ConditionFalse,
88+
"Progressing": opv1.ConditionFalse,
89+
},
90+
initialObjects: []runtime.Object{
91+
newCertManagerObjectWithConditions(
92+
opv1.OperatorCondition{Type: controllerPrefix + "Available", Status: opv1.ConditionTrue},
93+
opv1.OperatorCondition{Type: controllerPrefix + "-bar-Available", Status: opv1.ConditionFalse},
94+
95+
opv1.OperatorCondition{Type: controllerPrefix + "Degraded", Status: opv1.ConditionFalse},
96+
opv1.OperatorCondition{Type: controllerPrefix + "-bar-Degraded", Status: opv1.ConditionFalse},
97+
98+
opv1.OperatorCondition{Type: controllerPrefix + "Progressing", Status: opv1.ConditionFalse},
99+
),
100+
},
101+
expectError: true,
102+
errorContains: "context deadline exceeded",
103+
},
104+
{
105+
name: "A missing condition for Progressing",
106+
expectedConditions: map[string]opv1.ConditionStatus{
107+
"Available": opv1.ConditionTrue,
108+
"Degraded": opv1.ConditionFalse,
109+
"Progressing": opv1.ConditionFalse,
110+
},
111+
initialObjects: []runtime.Object{
112+
newCertManagerObjectWithConditions(
113+
opv1.OperatorCondition{Type: controllerPrefix + "Available", Status: opv1.ConditionTrue},
114+
opv1.OperatorCondition{Type: controllerPrefix + "-bar-Available", Status: opv1.ConditionFalse},
115+
116+
opv1.OperatorCondition{Type: controllerPrefix + "Degraded", Status: opv1.ConditionFalse},
117+
opv1.OperatorCondition{Type: controllerPrefix + "-bar-Degraded", Status: opv1.ConditionFalse},
118+
),
119+
},
120+
expectError: true,
121+
errorContains: "could not verify all status conditions as expected",
122+
},
123+
}
124+
125+
t.Parallel()
126+
for _, tt := range tests {
127+
t.Run(tt.name, func(t *testing.T) {
128+
fakeClient := fakecertmanclient.NewSimpleClientset(tt.initialObjects...)
129+
130+
matchers := GenerateConditionMatchers([]PrefixAndMatchTypeTuple{
131+
{controllerPrefix, tt.matchAny},
132+
}, tt.expectedConditions)
133+
134+
err := verifyOperatorStatusCondition(fakeClient.OperatorV1alpha1(), matchers)
135+
136+
if tt.expectError && err == nil {
137+
t.Errorf("Expected an error, but got nil")
138+
}
139+
140+
if !tt.expectError && err != nil {
141+
t.Errorf("Expected no error, but got: %v", err)
142+
}
143+
144+
if tt.expectError && err != nil {
145+
if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) {
146+
t.Errorf("Expected error to contain '%s', but got: %v", tt.errorContains, err)
147+
}
148+
}
149+
})
150+
}
151+
}

‎test/e2e/istio_csr_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,7 @@ var _ = Describe("Istio-CSR", Ordered, Label("TechPreview", "Feature:IstioCSR"),
5858

5959
BeforeEach(func() {
6060
By("waiting for operator status to become available")
61-
err := verifyOperatorStatusCondition(certmanageroperatorclient, []string{
62-
certManagerControllerDeploymentControllerName,
63-
certManagerWebhookDeploymentControllerName,
64-
certManagerCAInjectorDeploymentControllerName,
65-
}, validOperatorStatusConditions)
61+
err := VerifyHealthyOperatorConditions(certmanageroperatorclient.OperatorV1alpha1())
6662
Expect(err).NotTo(HaveOccurred(), "Operator is expected to be available")
6763

6864
By("creating a test namespace")

0 commit comments

Comments
 (0)