Skip to content

Commit bf96d68

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

File tree

5 files changed

+150
-6
lines changed

5 files changed

+150
-6
lines changed

pkg/controller/install/certresources.go

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,64 @@ 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
229236
return true
230237
}
231238

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
247+
return true
248+
}
249+
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+
hasCerts := map[string]struct{}{}
264+
for _, c := range i.getCertResources() {
265+
hasCerts[c.getDeploymentName()] = struct{}{}
266+
}
267+
for _, sddSpec := range strategy.DeploymentSpecs {
268+
if _, ok := hasCerts[sddSpec.Name]; ok {
269+
certSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(SecretName(ServiceName(sddSpec.Name)))
270+
if err == nil {
271+
if shouldRotateCerts(certSecret) {
272+
return true, nil
273+
}
274+
} else if apierrors.IsNotFound(err) {
275+
return true, nil
276+
} else {
277+
return false, err
278+
}
279+
}
280+
}
281+
return false, nil
282+
}
283+
235284
func CalculateCertExpiration(startingFrom time.Time) time.Time {
236285
return startingFrom.Add(DefaultCertValidFor)
237286
}
@@ -314,10 +363,10 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
314363

315364
// Attempt an update
316365
// TODO: Check that the secret was not modified
317-
if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) {
366+
if !shouldRotateCerts(existingSecret) {
318367
logger.Warnf("reusing existing cert %s", secret.GetName())
319368
secret = existingSecret
320-
caPEM = existingCAPEM
369+
caPEM = existingSecret.Data[OLMCAPEMKey]
321370
caHash = certs.PEMSHA256(caPEM)
322371
} else {
323372
if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil {

pkg/controller/install/resolver.go

Lines changed: 5 additions & 0 deletions
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+
}

pkg/controller/operators/olm/operator.go

Lines changed: 9 additions & 2 deletions
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

pkg/controller/operators/olm/operator_test.go

Lines changed: 4 additions & 0 deletions
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
}

pkg/fakes/fake_strategy_installer.go

Lines changed: 79 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)