Skip to content

Commit 94005cb

Browse files
ankitathomasankitathomas
committed
perform operator apiService certificate validity checks directly
Signed-off-by: Ankita Thomas <[email protected]>
1 parent 157af32 commit 94005cb

File tree

5 files changed

+177
-21
lines changed

5 files changed

+177
-21
lines changed

Diff for: ‎pkg/controller/install/certresources.go

+47-4
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,58 @@ func (i *StrategyDeploymentInstaller) CertsRotated() bool {
223223
return i.certificatesRotated
224224
}
225225

226-
func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool {
226+
func shouldRotateCerts(certSecret *corev1.Secret) bool {
227227
now := metav1.Now()
228-
if !csv.Status.CertsRotateAt.IsZero() && csv.Status.CertsRotateAt.Before(&now) {
228+
caPEM, ok := certSecret.Data[OLMCAPEMKey]
229+
if !ok {
230+
// missing CA cert in secret
231+
return true
232+
}
233+
certPEM, ok := certSecret.Data["tls.crt"]
234+
if !ok {
235+
// missing cert in secret
236+
return true
237+
}
238+
239+
ca, err := certs.PEMToCert(caPEM)
240+
if err != nil {
241+
// malformed CA cert
242+
return true
243+
}
244+
cert, err := certs.PEMToCert(certPEM)
245+
if err != nil {
246+
// malformed cert
229247
return true
230248
}
231249

250+
if now.Time.Before(ca.NotBefore) || now.Time.After(CalculateCertRotatesAt(ca.NotAfter)) ||
251+
now.Time.Before(cert.NotBefore) || now.Time.After(CalculateCertRotatesAt(cert.NotAfter)) {
252+
return true
253+
}
232254
return false
233255
}
234256

257+
func (i *StrategyDeploymentInstaller) ShouldRotateCerts(s Strategy) (bool, error) {
258+
strategy, ok := s.(*v1alpha1.StrategyDetailsDeployment)
259+
if !ok {
260+
return false, fmt.Errorf("attempted to install %s strategy with deployment installer", strategy.GetStrategyName())
261+
}
262+
263+
for _, sddSpec := range strategy.DeploymentSpecs {
264+
certSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(SecretName(ServiceName(sddSpec.Name)))
265+
if err == nil {
266+
if shouldRotateCerts(certSecret) {
267+
return true, nil
268+
}
269+
} else if apierrors.IsNotFound(err) {
270+
return true, nil
271+
} else {
272+
return false, err
273+
}
274+
}
275+
return false, nil
276+
}
277+
235278
func CalculateCertExpiration(startingFrom time.Time) time.Time {
236279
return startingFrom.Add(DefaultCertValidFor)
237280
}
@@ -314,10 +357,10 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
314357

315358
// Attempt an update
316359
// TODO: Check that the secret was not modified
317-
if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) {
360+
if !shouldRotateCerts(existingSecret) {
318361
logger.Warnf("reusing existing cert %s", secret.GetName())
319362
secret = existingSecret
320-
caPEM = existingCAPEM
363+
caPEM = existingSecret.Data[OLMCAPEMKey]
321364
caHash = certs.PEMSHA256(caPEM)
322365
} else {
323366
if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil {

Diff for: ‎pkg/controller/install/resolver.go

+5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type Strategy interface {
2121
type StrategyInstaller interface {
2222
Install(strategy Strategy) error
2323
CheckInstalled(strategy Strategy) (bool, error)
24+
ShouldRotateCerts(strategy Strategy) (bool, error)
2425
CertsRotateAt() time.Time
2526
CertsRotated() bool
2627
}
@@ -79,3 +80,7 @@ func (i *NullStrategyInstaller) CertsRotateAt() time.Time {
7980
func (i *NullStrategyInstaller) CertsRotated() bool {
8081
return false
8182
}
83+
84+
func (i *NullStrategyInstaller) ShouldRotateCerts(s Strategy) (bool, error) {
85+
return false, nil
86+
}

Diff for: ‎pkg/controller/operators/olm/operator.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -2206,6 +2206,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
22062206
out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsReinstall, "calculated deployment install is bad", now, a.recorder)
22072207
return
22082208
}
2209+
22092210
if installErr := a.updateInstallStatus(out, installer, strategy, v1alpha1.CSVPhaseInstalling, v1alpha1.CSVReasonWaiting); installErr != nil {
22102211
// Re-sync if kube-apiserver was unavailable
22112212
if apierrors.IsServiceUnavailable(installErr) {
@@ -2242,7 +2243,10 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
22422243
}
22432244

22442245
// Check if it's time to refresh owned APIService certs
2245-
if install.ShouldRotateCerts(out) {
2246+
if shouldRotate, err := installer.ShouldRotateCerts(strategy); err != nil {
2247+
logger.WithError(err).Info("cert validity check")
2248+
return
2249+
} else if shouldRotate {
22462250
logger.Debug("CSV owns resources that require a cert refresh")
22472251
out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsCertRotation, "CSV owns resources that require a cert refresh", now, a.recorder)
22482252
return
@@ -2352,7 +2356,10 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
23522356
}
23532357

23542358
// Check if it's time to refresh owned APIService certs
2355-
if install.ShouldRotateCerts(out) {
2359+
if shouldRotate, err := installer.ShouldRotateCerts(strategy); err != nil {
2360+
logger.WithError(err).Info("cert validity check")
2361+
return
2362+
} else if shouldRotate {
23562363
logger.Debug("CSV owns resources that require a cert refresh")
23572364
out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsCertRotation, "owned APIServices need cert refresh", now, a.recorder)
23582365
return

Diff for: ‎pkg/controller/operators/olm/operator_test.go

+37-15
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ func (i *TestInstaller) CheckInstalled(s install.Strategy) (bool, error) {
101101
return true, nil
102102
}
103103

104+
func (i *TestInstaller) ShouldRotateCerts(s install.Strategy) (bool, error) {
105+
return false, nil
106+
}
107+
104108
func (i *TestInstaller) CertsRotateAt() time.Time {
105109
return time.Time{}
106110
}
@@ -543,14 +547,15 @@ func roleBinding(name, namespace, roleName, serviceAccountName, serviceAccountNa
543547
return roleBinding
544548
}
545549

546-
func tlsSecret(name, namespace string, certPEM, privPEM []byte) *corev1.Secret {
550+
func tlsSecret(name, namespace string, caPEM, certPEM, privPEM []byte) *corev1.Secret {
547551
secret := &corev1.Secret{
548552
ObjectMeta: metav1.ObjectMeta{
549553
Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue},
550554
},
551555
Data: map[string][]byte{
552-
"tls.crt": certPEM,
553-
"tls.key": privPEM,
556+
install.OLMCAPEMKey: caPEM,
557+
"tls.crt": certPEM,
558+
"tls.key": privPEM,
554559
},
555560
Type: corev1.SecretTypeTLS,
556561
}
@@ -565,7 +570,7 @@ func withCA(secret *corev1.Secret, caPEM []byte) *corev1.Secret {
565570
return secret
566571
}
567572

568-
func keyPairToTLSSecret(name, namespace string, kp *certs.KeyPair) *corev1.Secret {
573+
func keyPairToTLSSecret(name, namespace string, caPEM []byte, kp *certs.KeyPair) *corev1.Secret {
569574
var privPEM []byte
570575
var certPEM []byte
571576

@@ -577,7 +582,7 @@ func keyPairToTLSSecret(name, namespace string, kp *certs.KeyPair) *corev1.Secre
577582
}
578583
}
579584

580-
return tlsSecret(name, namespace, certPEM, privPEM)
585+
return tlsSecret(name, namespace, caPEM, certPEM, privPEM)
581586
}
582587

583588
func signedServingPair(notAfter time.Time, ca *certs.KeyPair, hosts []string) *certs.KeyPair {
@@ -1322,7 +1327,7 @@ func TestTransitionCSV(t *testing.T) {
13221327
install.OLMCAHashAnnotationKey: validCAHash,
13231328
}))),
13241329
),
1325-
withAnnotations(keyPairToTLSSecret("a1-service-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
1330+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
13261331
install.OLMCAHashAnnotationKey: validCAHash,
13271332
}),
13281333
service("a1", namespace, "a1", 80),
@@ -1388,6 +1393,11 @@ func TestTransitionCSV(t *testing.T) {
13881393
crds: []runtime.Object{
13891394
crd("c1", "v1", "g1"),
13901395
},
1396+
objs: []runtime.Object{
1397+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
1398+
install.OLMCAHashAnnotationKey: validCAHash,
1399+
}),
1400+
},
13911401
},
13921402
expected: expected{
13931403
csvStates: map[string]csvState{
@@ -1548,7 +1558,7 @@ func TestTransitionCSV(t *testing.T) {
15481558
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
15491559
install.OLMCAHashAnnotationKey: validCAHash,
15501560
})),
1551-
withLabels(withAnnotations(keyPairToTLSSecret("a1-service-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
1561+
withLabels(withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
15521562
install.OLMCAHashAnnotationKey: validCAHash,
15531563
}), map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
15541564
service("a1-service", namespace, "a1", 80),
@@ -1617,7 +1627,7 @@ func TestTransitionCSV(t *testing.T) {
16171627
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
16181628
install.OLMCAHashAnnotationKey: "a-pretty-bad-hash",
16191629
})),
1620-
withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
1630+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
16211631
install.OLMCAHashAnnotationKey: validCAHash,
16221632
}),
16231633
service("v1-a1", namespace, "a1", 80),
@@ -1686,7 +1696,7 @@ func TestTransitionCSV(t *testing.T) {
16861696
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
16871697
install.OLMCAHashAnnotationKey: validCAHash,
16881698
})),
1689-
withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
1699+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
16901700
install.OLMCAHashAnnotationKey: "also-a-pretty-bad-hash",
16911701
}),
16921702
service("v1-a1", namespace, "a1", 80),
@@ -1755,7 +1765,7 @@ func TestTransitionCSV(t *testing.T) {
17551765
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
17561766
install.OLMCAHashAnnotationKey: "a-pretty-bad-hash",
17571767
})),
1758-
withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
1768+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
17591769
install.OLMCAHashAnnotationKey: "also-a-pretty-bad-hash",
17601770
}),
17611771
service("v1-a1", namespace, "a1", 80),
@@ -1824,7 +1834,7 @@ func TestTransitionCSV(t *testing.T) {
18241834
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
18251835
install.OLMCAHashAnnotationKey: validCAHash,
18261836
})),
1827-
withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
1837+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
18281838
install.OLMCAHashAnnotationKey: validCAHash,
18291839
}),
18301840
service("v1-a1", namespace, "a1", 80),
@@ -1893,7 +1903,7 @@ func TestTransitionCSV(t *testing.T) {
18931903
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
18941904
install.OLMCAHashAnnotationKey: validCAHash,
18951905
})),
1896-
withAnnotations(tlsSecret("v1.a1-cert", namespace, []byte("bad-cert"), []byte("bad-key")), map[string]string{
1906+
withAnnotations(tlsSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, []byte("bad-cert"), []byte("bad-key")), map[string]string{
18971907
install.OLMCAHashAnnotationKey: validCAHash,
18981908
}),
18991909
service("v1-a1", namespace, "a1", 80),
@@ -1962,7 +1972,7 @@ func TestTransitionCSV(t *testing.T) {
19621972
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
19631973
install.OLMCAHashAnnotationKey: expiredCAHash,
19641974
})),
1965-
withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), expiredCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
1975+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), expiredCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
19661976
install.OLMCAHashAnnotationKey: expiredCAHash,
19671977
}),
19681978
service("v1-a1", namespace, "a1", 80),
@@ -2031,7 +2041,7 @@ func TestTransitionCSV(t *testing.T) {
20312041
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
20322042
install.OLMCAHashAnnotationKey: expiredCAHash,
20332043
})),
2034-
withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), expiredCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
2044+
withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(time.Now().Add(24*time.Hour).Add(install.DefaultCertMinFresh), expiredCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
20352045
install.OLMCAHashAnnotationKey: expiredCAHash,
20362046
}),
20372047
service("v1-a1", namespace, "a1", 80),
@@ -4448,6 +4458,15 @@ func TestSyncOperatorGroups(t *testing.T) {
44484458
}
44494459
roleBinding.Labels[install.OLMManagedLabelKey] = install.OLMManagedLabelValue
44504460

4461+
// Generate valid and expired CA fixtures
4462+
validCA, err := generateCA(time.Now().Add(10*365*24*time.Hour), install.Organization)
4463+
require.NoError(t, err)
4464+
validCAPEM, _, err := validCA.ToPEM()
4465+
require.NoError(t, err)
4466+
certSecret := withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName(deploymentName)), operatorNamespace, validCAPEM, signedServingPair(time.Now().Add(48*time.Hour), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
4467+
install.OLMCAHashAnnotationKey: certs.PEMSHA256(validCAPEM),
4468+
})
4469+
44514470
type initial struct {
44524471
operatorGroup *operatorsv1.OperatorGroup
44534472
csvs []*v1alpha1.ClusterServiceVersion
@@ -5032,6 +5051,7 @@ func TestSyncOperatorGroups(t *testing.T) {
50325051
serviceAccount,
50335052
role,
50345053
roleBinding,
5054+
certSecret,
50355055
},
50365056
crds: []*apiextensionsv1.CustomResourceDefinition{crd},
50375057
},
@@ -5137,6 +5157,7 @@ func TestSyncOperatorGroups(t *testing.T) {
51375157
serviceAccount,
51385158
role,
51395159
roleBinding,
5160+
certSecret,
51405161
},
51415162
crds: []*apiextensionsv1.CustomResourceDefinition{crd},
51425163
},
@@ -5245,6 +5266,7 @@ func TestSyncOperatorGroups(t *testing.T) {
52455266
serviceAccount,
52465267
role,
52475268
roleBinding,
5269+
certSecret,
52485270
},
52495271
crds: []*apiextensionsv1.CustomResourceDefinition{crd},
52505272
},
@@ -6029,7 +6051,7 @@ func TestCARotation(t *testing.T) {
60296051
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
60306052
install.OLMCAHashAnnotationKey: validCAHash,
60316053
})),
6032-
withLabels(withAnnotations(withCA(keyPairToTLSSecret("a1-service-cert", namespace, signedServingPair(expiresAt.Time, validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), validCAPEM), map[string]string{
6054+
withLabels(withAnnotations(withCA(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, validCAPEM, signedServingPair(expiresAt.Time, validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), validCAPEM), map[string]string{
60336055
install.OLMCAHashAnnotationKey: validCAHash,
60346056
}), map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
60356057
service("a1-service", namespace, "a1", 80, ownerReference),

0 commit comments

Comments
 (0)