diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go b/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go index 5fc76ab22f..f48e62b771 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go @@ -160,7 +160,7 @@ func (i *StrategyDeploymentInstaller) getCertResources() []certResource { } func (i *StrategyDeploymentInstaller) certResourcesForDeployment(deploymentName string) []certResource { - result := []certResource{} + var result []certResource for _, desc := range i.getCertResources() { if desc.getDeploymentName() == deploymentName { result = append(result, desc) @@ -185,8 +185,8 @@ func (i *StrategyDeploymentInstaller) installCertRequirements(strategy Strategy) } // Create the CA - expiration, _ := CalculateCertExpirationAndRotateAt() - ca, err := certs.GenerateCA(expiration, Organization) + i.certificateExpirationTime = CalculateCertExpiration(time.Now()) + ca, err := certs.GenerateCA(i.certificateExpirationTime, Organization) if err != nil { logger.Debug("failed to generate CA") return nil, err @@ -201,7 +201,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirements(strategy Strategy) } // Update the deployment for each certResource - newDepSpec, caPEM, err := i.installCertRequirementsForDeployment(sddSpec.Name, ca, expiration, sddSpec.Spec, getServicePorts(certResources)) + newDepSpec, caPEM, err := i.installCertRequirementsForDeployment(sddSpec.Name, ca, i.certificateExpirationTime, sddSpec.Spec, getServicePorts(certResources)) if err != nil { return nil, err } @@ -213,6 +213,14 @@ func (i *StrategyDeploymentInstaller) installCertRequirements(strategy Strategy) return strategyDetailsDeployment, nil } +func (i *StrategyDeploymentInstaller) CertsRotateAt() time.Time { + return CalculateCertRotatesAt(i.certificateExpirationTime) +} + +func (i *StrategyDeploymentInstaller) CertsRotated() bool { + return i.certificatesRotated +} + func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool { now := metav1.Now() if !csv.Status.CertsRotateAt.IsZero() && csv.Status.CertsRotateAt.Before(&now) { @@ -222,10 +230,12 @@ func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool { return false } -func CalculateCertExpirationAndRotateAt() (expiration time.Time, rotateAt time.Time) { - expiration = time.Now().Add(DefaultCertValidFor) - rotateAt = expiration.Add(-1 * DefaultCertMinFresh) - return +func CalculateCertExpiration(startingFrom time.Time) time.Time { + return startingFrom.Add(DefaultCertValidFor) +} + +func CalculateCertRotatesAt(certExpirationTime time.Time) time.Time { + return certExpirationTime.Add(-1 * DefaultCertMinFresh) } func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deploymentName string, ca *certs.KeyPair, expiration time.Time, depSpec appsv1.DeploymentSpec, ports []corev1.ServicePort) (*appsv1.DeploymentSpec, []byte, error) { @@ -316,9 +326,12 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo secret = existingSecret caPEM = existingCAPEM caHash = certs.PEMSHA256(caPEM) - } else if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil { - logger.Warnf("could not update secret %s", secret.GetName()) - return nil, nil, err + } else { + if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil { + logger.Warnf("could not update secret %s", secret.GetName()) + return nil, nil, err + } + i.certificatesRotated = true } } else if apierrors.IsNotFound(err) { // Create the secret @@ -335,6 +348,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo return nil, nil, err } } + i.certificatesRotated = true } else { return nil, nil, err } diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/deployment.go b/staging/operator-lifecycle-manager/pkg/controller/install/deployment.go index d763c51306..559520aad7 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/deployment.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/deployment.go @@ -3,6 +3,7 @@ package install import ( "fmt" "hash/fnv" + "time" log "github.com/sirupsen/logrus" appsv1 "k8s.io/api/apps/v1" @@ -22,13 +23,15 @@ import ( const DeploymentSpecHashLabelKey = "olm.deployment-spec-hash" type StrategyDeploymentInstaller struct { - strategyClient wrappers.InstallStrategyDeploymentInterface - owner ownerutil.Owner - previousStrategy Strategy - templateAnnotations map[string]string - initializers DeploymentInitializerFuncChain - apiServiceDescriptions []certResource - webhookDescriptions []certResource + strategyClient wrappers.InstallStrategyDeploymentInterface + owner ownerutil.Owner + previousStrategy Strategy + templateAnnotations map[string]string + initializers DeploymentInitializerFuncChain + apiServiceDescriptions []certResource + webhookDescriptions []certResource + certificateExpirationTime time.Time + certificatesRotated bool } var _ Strategy = &v1alpha1.StrategyDetailsDeployment{} @@ -77,13 +80,15 @@ func NewStrategyDeploymentInstaller(strategyClient wrappers.InstallStrategyDeplo } return &StrategyDeploymentInstaller{ - strategyClient: strategyClient, - owner: owner, - previousStrategy: previousStrategy, - templateAnnotations: templateAnnotations, - initializers: initializers, - apiServiceDescriptions: apiDescs, - webhookDescriptions: webhookDescs, + strategyClient: strategyClient, + owner: owner, + previousStrategy: previousStrategy, + templateAnnotations: templateAnnotations, + initializers: initializers, + apiServiceDescriptions: apiDescs, + webhookDescriptions: webhookDescs, + certificatesRotated: false, + certificateExpirationTime: time.Time{}, } } diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go b/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go index 07693ebb26..206ce9e384 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go @@ -5,6 +5,7 @@ package install import ( "fmt" + "time" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers" @@ -20,6 +21,8 @@ type Strategy interface { type StrategyInstaller interface { Install(strategy Strategy) error CheckInstalled(strategy Strategy) (bool, error) + CertsRotateAt() time.Time + CertsRotated() bool } type StrategyResolverInterface interface { @@ -68,3 +71,11 @@ func (i *NullStrategyInstaller) Install(s Strategy) error { func (i *NullStrategyInstaller) CheckInstalled(s Strategy) (bool, error) { return true, nil } + +func (i *NullStrategyInstaller) CertsRotateAt() time.Time { + return time.Time{} +} + +func (i *NullStrategyInstaller) CertsRotated() bool { + return false +} diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go index fe9d056bff..f00f18a40c 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -1888,20 +1888,9 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v return } - // Only update certificate status if: - // - the CSV has CAResources; and - // - the certificate lastUpdated and rotateAt timestamps have not already been set, or the certificate should be rotated - // Note: the code here is a bit wonky and it wasn't clear how to clean it up without some major refactoring: - // the installer is in charge of generating and rotating the certificates. It detects whether a certificate should be rotated by - // looking at the csv.status.RotateAt value. But, it does not update this value or surface - // the certificate expiry information. So, the rotatedAt value is to be re-calculated here. This is bad because you have - // two different components doing the same thing (installer and operator are both calculating RotateAt). If we're not careful - // there could be skew - // See pkg/controller/install/certresources.go - if shouldUpdateCertificateDates(out) { + if installer.CertsRotated() { now := metav1.Now() - _, rotateAt := install.CalculateCertExpirationAndRotateAt() - rotateTime := metav1.NewTime(rotateAt) + rotateTime := metav1.NewTime(installer.CertsRotateAt()) out.Status.CertsLastUpdated = &now out.Status.CertsRotateAt = &rotateTime } @@ -2531,11 +2520,3 @@ func (a *Operator) ensureLabels(in *v1alpha1.ClusterServiceVersion, labelSets .. out, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(out.GetNamespace()).Update(context.TODO(), out, metav1.UpdateOptions{}) return out, err } - -// shouldUpdateCertificateDates checks the csv status to decide whether -// status.CertsLastUpdated and status.CertsRotateAt should be updated -// returns true if the CSV has CAResources and status.RotatedAt is not set OR its time to rotate the certificates -func shouldUpdateCertificateDates(csv *v1alpha1.ClusterServiceVersion) bool { - isNotSet := csv.Status.CertsRotateAt == nil || csv.Status.CertsRotateAt.IsZero() - return csv.HasCAResources() && (isNotSet || install.ShouldRotateCerts(csv)) -} diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator_test.go index dc04de5cc3..00ebc733c9 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator_test.go @@ -95,6 +95,14 @@ func (i *TestInstaller) CheckInstalled(s install.Strategy) (bool, error) { return true, nil } +func (i *TestInstaller) CertsRotateAt() time.Time { + return time.Time{} +} + +func (i *TestInstaller) CertsRotated() bool { + return false +} + func ownerLabelFromCSV(name, namespace string) map[string]string { return map[string]string{ ownerutil.OwnerKey: name, @@ -5112,9 +5120,6 @@ func TestCARotation(t *testing.T) { logrus.SetLevel(logrus.DebugLevel) namespace := "ns" - //apiHash, err := resolvercache.APIKeyToGVKHash(opregistry.APIKey{Group: "g1", Version: "v1", Kind: "c1"}) - //require.NoError(t, err) - defaultOperatorGroup := &operatorsv1.OperatorGroup{ TypeMeta: metav1.TypeMeta{ Kind: "OperatorGroup", @@ -5137,9 +5142,8 @@ func TestCARotation(t *testing.T) { } // Generate valid and expired CA fixtures - expirationTime, rotationTime := install.CalculateCertExpirationAndRotateAt() - expiresAt := metav1.Time{Time: expirationTime} - rotateAt := metav1.Time{Time: rotationTime} + expiresAt := metav1.NewTime(install.CalculateCertExpiration(time.Now())) + rotateAt := metav1.NewTime(install.CalculateCertRotatesAt(expiresAt.Time)) lastUpdate := metav1.Time{Time: time.Now().UTC()} @@ -5358,18 +5362,17 @@ func TestCARotation(t *testing.T) { require.NoError(t, err) require.NotNil(t, serviceSecret) - // Extract certificate + // Extract certificate validity period start, end, err := GetServiceCertificaValidityPeriod(serviceSecret) require.NoError(t, err) require.NotNil(t, start) require.NotNil(t, end) - // Compare csv status timestamps with certificate timestamps - // NOTE: These values (csv.Status.Certs* and the certificate expiry and rotation are calculated - // with the same method but independently, therefore a second granularity will need to suffice. - // See https://github.com/operator-framework/operator-lifecycle-manager/issues/2764 for more info. rotationTime := end.Add(-1 * install.DefaultCertMinFresh) - require.Equal(t, start.Unix(), outCSV.Status.CertsLastUpdated.Unix()) + // The csv status is updated after the certificate is created/rotated + require.LessOrEqual(t, start.Unix(), outCSV.Status.CertsLastUpdated.Unix()) + + // Rotation time should always be the same between the certificate and the status require.Equal(t, rotationTime.Unix(), outCSV.Status.CertsRotateAt.Unix()) } } diff --git a/staging/operator-lifecycle-manager/pkg/fakes/fake_strategy_installer.go b/staging/operator-lifecycle-manager/pkg/fakes/fake_strategy_installer.go index e3e5ab6571..7d6dcf2cec 100644 --- a/staging/operator-lifecycle-manager/pkg/fakes/fake_strategy_installer.go +++ b/staging/operator-lifecycle-manager/pkg/fakes/fake_strategy_installer.go @@ -3,11 +3,32 @@ package fakes import ( "sync" + "time" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" ) type FakeStrategyInstaller struct { + CertsRotateAtStub func() time.Time + certsRotateAtMutex sync.RWMutex + certsRotateAtArgsForCall []struct { + } + certsRotateAtReturns struct { + result1 time.Time + } + certsRotateAtReturnsOnCall map[int]struct { + result1 time.Time + } + CertsRotatedStub func() bool + certsRotatedMutex sync.RWMutex + certsRotatedArgsForCall []struct { + } + certsRotatedReturns struct { + result1 bool + } + certsRotatedReturnsOnCall map[int]struct { + result1 bool + } CheckInstalledStub func(install.Strategy) (bool, error) checkInstalledMutex sync.RWMutex checkInstalledArgsForCall []struct { @@ -36,6 +57,110 @@ type FakeStrategyInstaller struct { invocationsMutex sync.RWMutex } +func (fake *FakeStrategyInstaller) CertsRotateAt() time.Time { + fake.certsRotateAtMutex.Lock() + ret, specificReturn := fake.certsRotateAtReturnsOnCall[len(fake.certsRotateAtArgsForCall)] + fake.certsRotateAtArgsForCall = append(fake.certsRotateAtArgsForCall, struct { + }{}) + fake.recordInvocation("CertsRotateAt", []interface{}{}) + fake.certsRotateAtMutex.Unlock() + if fake.CertsRotateAtStub != nil { + return fake.CertsRotateAtStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.certsRotateAtReturns + return fakeReturns.result1 +} + +func (fake *FakeStrategyInstaller) CertsRotateAtCallCount() int { + fake.certsRotateAtMutex.RLock() + defer fake.certsRotateAtMutex.RUnlock() + return len(fake.certsRotateAtArgsForCall) +} + +func (fake *FakeStrategyInstaller) CertsRotateAtCalls(stub func() time.Time) { + fake.certsRotateAtMutex.Lock() + defer fake.certsRotateAtMutex.Unlock() + fake.CertsRotateAtStub = stub +} + +func (fake *FakeStrategyInstaller) CertsRotateAtReturns(result1 time.Time) { + fake.certsRotateAtMutex.Lock() + defer fake.certsRotateAtMutex.Unlock() + fake.CertsRotateAtStub = nil + fake.certsRotateAtReturns = struct { + result1 time.Time + }{result1} +} + +func (fake *FakeStrategyInstaller) CertsRotateAtReturnsOnCall(i int, result1 time.Time) { + fake.certsRotateAtMutex.Lock() + defer fake.certsRotateAtMutex.Unlock() + fake.CertsRotateAtStub = nil + if fake.certsRotateAtReturnsOnCall == nil { + fake.certsRotateAtReturnsOnCall = make(map[int]struct { + result1 time.Time + }) + } + fake.certsRotateAtReturnsOnCall[i] = struct { + result1 time.Time + }{result1} +} + +func (fake *FakeStrategyInstaller) CertsRotated() bool { + fake.certsRotatedMutex.Lock() + ret, specificReturn := fake.certsRotatedReturnsOnCall[len(fake.certsRotatedArgsForCall)] + fake.certsRotatedArgsForCall = append(fake.certsRotatedArgsForCall, struct { + }{}) + fake.recordInvocation("CertsRotated", []interface{}{}) + fake.certsRotatedMutex.Unlock() + if fake.CertsRotatedStub != nil { + return fake.CertsRotatedStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.certsRotatedReturns + return fakeReturns.result1 +} + +func (fake *FakeStrategyInstaller) CertsRotatedCallCount() int { + fake.certsRotatedMutex.RLock() + defer fake.certsRotatedMutex.RUnlock() + return len(fake.certsRotatedArgsForCall) +} + +func (fake *FakeStrategyInstaller) CertsRotatedCalls(stub func() bool) { + fake.certsRotatedMutex.Lock() + defer fake.certsRotatedMutex.Unlock() + fake.CertsRotatedStub = stub +} + +func (fake *FakeStrategyInstaller) CertsRotatedReturns(result1 bool) { + fake.certsRotatedMutex.Lock() + defer fake.certsRotatedMutex.Unlock() + fake.CertsRotatedStub = nil + fake.certsRotatedReturns = struct { + result1 bool + }{result1} +} + +func (fake *FakeStrategyInstaller) CertsRotatedReturnsOnCall(i int, result1 bool) { + fake.certsRotatedMutex.Lock() + defer fake.certsRotatedMutex.Unlock() + fake.CertsRotatedStub = nil + if fake.certsRotatedReturnsOnCall == nil { + fake.certsRotatedReturnsOnCall = make(map[int]struct { + result1 bool + }) + } + fake.certsRotatedReturnsOnCall[i] = struct { + result1 bool + }{result1} +} + func (fake *FakeStrategyInstaller) CheckInstalled(arg1 install.Strategy) (bool, error) { fake.checkInstalledMutex.Lock() ret, specificReturn := fake.checkInstalledReturnsOnCall[len(fake.checkInstalledArgsForCall)] @@ -162,6 +287,10 @@ func (fake *FakeStrategyInstaller) InstallReturnsOnCall(i int, result1 error) { func (fake *FakeStrategyInstaller) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() + fake.certsRotateAtMutex.RLock() + defer fake.certsRotateAtMutex.RUnlock() + fake.certsRotatedMutex.RLock() + defer fake.certsRotatedMutex.RUnlock() fake.checkInstalledMutex.RLock() defer fake.checkInstalledMutex.RUnlock() fake.installMutex.RLock() diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go index 5fc76ab22f..f48e62b771 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go @@ -160,7 +160,7 @@ func (i *StrategyDeploymentInstaller) getCertResources() []certResource { } func (i *StrategyDeploymentInstaller) certResourcesForDeployment(deploymentName string) []certResource { - result := []certResource{} + var result []certResource for _, desc := range i.getCertResources() { if desc.getDeploymentName() == deploymentName { result = append(result, desc) @@ -185,8 +185,8 @@ func (i *StrategyDeploymentInstaller) installCertRequirements(strategy Strategy) } // Create the CA - expiration, _ := CalculateCertExpirationAndRotateAt() - ca, err := certs.GenerateCA(expiration, Organization) + i.certificateExpirationTime = CalculateCertExpiration(time.Now()) + ca, err := certs.GenerateCA(i.certificateExpirationTime, Organization) if err != nil { logger.Debug("failed to generate CA") return nil, err @@ -201,7 +201,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirements(strategy Strategy) } // Update the deployment for each certResource - newDepSpec, caPEM, err := i.installCertRequirementsForDeployment(sddSpec.Name, ca, expiration, sddSpec.Spec, getServicePorts(certResources)) + newDepSpec, caPEM, err := i.installCertRequirementsForDeployment(sddSpec.Name, ca, i.certificateExpirationTime, sddSpec.Spec, getServicePorts(certResources)) if err != nil { return nil, err } @@ -213,6 +213,14 @@ func (i *StrategyDeploymentInstaller) installCertRequirements(strategy Strategy) return strategyDetailsDeployment, nil } +func (i *StrategyDeploymentInstaller) CertsRotateAt() time.Time { + return CalculateCertRotatesAt(i.certificateExpirationTime) +} + +func (i *StrategyDeploymentInstaller) CertsRotated() bool { + return i.certificatesRotated +} + func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool { now := metav1.Now() if !csv.Status.CertsRotateAt.IsZero() && csv.Status.CertsRotateAt.Before(&now) { @@ -222,10 +230,12 @@ func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool { return false } -func CalculateCertExpirationAndRotateAt() (expiration time.Time, rotateAt time.Time) { - expiration = time.Now().Add(DefaultCertValidFor) - rotateAt = expiration.Add(-1 * DefaultCertMinFresh) - return +func CalculateCertExpiration(startingFrom time.Time) time.Time { + return startingFrom.Add(DefaultCertValidFor) +} + +func CalculateCertRotatesAt(certExpirationTime time.Time) time.Time { + return certExpirationTime.Add(-1 * DefaultCertMinFresh) } func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deploymentName string, ca *certs.KeyPair, expiration time.Time, depSpec appsv1.DeploymentSpec, ports []corev1.ServicePort) (*appsv1.DeploymentSpec, []byte, error) { @@ -316,9 +326,12 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo secret = existingSecret caPEM = existingCAPEM caHash = certs.PEMSHA256(caPEM) - } else if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil { - logger.Warnf("could not update secret %s", secret.GetName()) - return nil, nil, err + } else { + if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil { + logger.Warnf("could not update secret %s", secret.GetName()) + return nil, nil, err + } + i.certificatesRotated = true } } else if apierrors.IsNotFound(err) { // Create the secret @@ -335,6 +348,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo return nil, nil, err } } + i.certificatesRotated = true } else { return nil, nil, err } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/deployment.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/deployment.go index d763c51306..559520aad7 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/deployment.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/deployment.go @@ -3,6 +3,7 @@ package install import ( "fmt" "hash/fnv" + "time" log "github.com/sirupsen/logrus" appsv1 "k8s.io/api/apps/v1" @@ -22,13 +23,15 @@ import ( const DeploymentSpecHashLabelKey = "olm.deployment-spec-hash" type StrategyDeploymentInstaller struct { - strategyClient wrappers.InstallStrategyDeploymentInterface - owner ownerutil.Owner - previousStrategy Strategy - templateAnnotations map[string]string - initializers DeploymentInitializerFuncChain - apiServiceDescriptions []certResource - webhookDescriptions []certResource + strategyClient wrappers.InstallStrategyDeploymentInterface + owner ownerutil.Owner + previousStrategy Strategy + templateAnnotations map[string]string + initializers DeploymentInitializerFuncChain + apiServiceDescriptions []certResource + webhookDescriptions []certResource + certificateExpirationTime time.Time + certificatesRotated bool } var _ Strategy = &v1alpha1.StrategyDetailsDeployment{} @@ -77,13 +80,15 @@ func NewStrategyDeploymentInstaller(strategyClient wrappers.InstallStrategyDeplo } return &StrategyDeploymentInstaller{ - strategyClient: strategyClient, - owner: owner, - previousStrategy: previousStrategy, - templateAnnotations: templateAnnotations, - initializers: initializers, - apiServiceDescriptions: apiDescs, - webhookDescriptions: webhookDescs, + strategyClient: strategyClient, + owner: owner, + previousStrategy: previousStrategy, + templateAnnotations: templateAnnotations, + initializers: initializers, + apiServiceDescriptions: apiDescs, + webhookDescriptions: webhookDescs, + certificatesRotated: false, + certificateExpirationTime: time.Time{}, } } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/resolver.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/resolver.go index 07693ebb26..206ce9e384 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/resolver.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/resolver.go @@ -5,6 +5,7 @@ package install import ( "fmt" + "time" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers" @@ -20,6 +21,8 @@ type Strategy interface { type StrategyInstaller interface { Install(strategy Strategy) error CheckInstalled(strategy Strategy) (bool, error) + CertsRotateAt() time.Time + CertsRotated() bool } type StrategyResolverInterface interface { @@ -68,3 +71,11 @@ func (i *NullStrategyInstaller) Install(s Strategy) error { func (i *NullStrategyInstaller) CheckInstalled(s Strategy) (bool, error) { return true, nil } + +func (i *NullStrategyInstaller) CertsRotateAt() time.Time { + return time.Time{} +} + +func (i *NullStrategyInstaller) CertsRotated() bool { + return false +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go index fe9d056bff..f00f18a40c 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -1888,20 +1888,9 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v return } - // Only update certificate status if: - // - the CSV has CAResources; and - // - the certificate lastUpdated and rotateAt timestamps have not already been set, or the certificate should be rotated - // Note: the code here is a bit wonky and it wasn't clear how to clean it up without some major refactoring: - // the installer is in charge of generating and rotating the certificates. It detects whether a certificate should be rotated by - // looking at the csv.status.RotateAt value. But, it does not update this value or surface - // the certificate expiry information. So, the rotatedAt value is to be re-calculated here. This is bad because you have - // two different components doing the same thing (installer and operator are both calculating RotateAt). If we're not careful - // there could be skew - // See pkg/controller/install/certresources.go - if shouldUpdateCertificateDates(out) { + if installer.CertsRotated() { now := metav1.Now() - _, rotateAt := install.CalculateCertExpirationAndRotateAt() - rotateTime := metav1.NewTime(rotateAt) + rotateTime := metav1.NewTime(installer.CertsRotateAt()) out.Status.CertsLastUpdated = &now out.Status.CertsRotateAt = &rotateTime } @@ -2531,11 +2520,3 @@ func (a *Operator) ensureLabels(in *v1alpha1.ClusterServiceVersion, labelSets .. out, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(out.GetNamespace()).Update(context.TODO(), out, metav1.UpdateOptions{}) return out, err } - -// shouldUpdateCertificateDates checks the csv status to decide whether -// status.CertsLastUpdated and status.CertsRotateAt should be updated -// returns true if the CSV has CAResources and status.RotatedAt is not set OR its time to rotate the certificates -func shouldUpdateCertificateDates(csv *v1alpha1.ClusterServiceVersion) bool { - isNotSet := csv.Status.CertsRotateAt == nil || csv.Status.CertsRotateAt.IsZero() - return csv.HasCAResources() && (isNotSet || install.ShouldRotateCerts(csv)) -}