diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go b/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go index f48e62b771..2822b43063 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go @@ -12,6 +12,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/certs" @@ -155,6 +156,14 @@ func ServiceName(deploymentName string) string { return deploymentName + "-service" } +func HostnamesForService(serviceName, namespace string) []string { + return []string{ + fmt.Sprintf("%s.%s", serviceName, namespace), + fmt.Sprintf("%s.%s.svc", serviceName, namespace), + fmt.Sprintf("%s.%s.svc.cluster.local", serviceName, namespace), + } +} + func (i *StrategyDeploymentInstaller) getCertResources() []certResource { return append(i.apiServiceDescriptions, i.webhookDescriptions...) } @@ -221,15 +230,74 @@ func (i *StrategyDeploymentInstaller) CertsRotated() bool { return i.certificatesRotated } -func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool { +// shouldRotateCerts indicates whether an apiService cert should be rotated due to being +// malformed, invalid, expired, inactive or within a specific freshness interval (DefaultCertMinFresh) before expiry. +func shouldRotateCerts(certSecret *corev1.Secret, hosts []string) bool { now := metav1.Now() - if !csv.Status.CertsRotateAt.IsZero() && csv.Status.CertsRotateAt.Before(&now) { + caPEM, ok := certSecret.Data[OLMCAPEMKey] + if !ok { + // missing CA cert in secret + return true + } + certPEM, ok := certSecret.Data["tls.crt"] + if !ok { + // missing cert in secret + return true + } + + ca, err := certs.PEMToCert(caPEM) + if err != nil { + // malformed CA cert + return true + } + cert, err := certs.PEMToCert(certPEM) + if err != nil { + // malformed cert return true } + // check for cert freshness + if !certs.Active(ca) || now.Time.After(CalculateCertRotatesAt(ca.NotAfter)) || + !certs.Active(cert) || now.Time.After(CalculateCertRotatesAt(cert.NotAfter)) { + return true + } + + // Check validity of serving cert and if serving cert is trusted by the CA + for _, host := range hosts { + if err := certs.VerifyCert(ca, cert, host); err != nil { + return true + } + } return false } +func (i *StrategyDeploymentInstaller) ShouldRotateCerts(s Strategy) (bool, error) { + strategy, ok := s.(*v1alpha1.StrategyDetailsDeployment) + if !ok { + return false, fmt.Errorf("failed to install %s strategy with deployment installer: unsupported deployment install strategy", strategy.GetStrategyName()) + } + + hasCerts := sets.NewString() + for _, c := range i.getCertResources() { + hasCerts.Insert(c.getDeploymentName()) + } + for _, sddSpec := range strategy.DeploymentSpecs { + if hasCerts.Has(sddSpec.Name) { + certSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(SecretName(ServiceName(sddSpec.Name))) + if err == nil { + if shouldRotateCerts(certSecret, HostnamesForService(ServiceName(sddSpec.Name), i.owner.GetNamespace())) { + return true, nil + } + } else if apierrors.IsNotFound(err) { + return true, nil + } else { + return false, err + } + } + } + return false, nil +} + func CalculateCertExpiration(startingFrom time.Time) time.Time { return startingFrom.Add(DefaultCertValidFor) } @@ -248,7 +316,8 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo Selector: depSpec.Selector.MatchLabels, }, } - service.SetName(ServiceName(deploymentName)) + serviceName := ServiceName(deploymentName) + service.SetName(serviceName) service.SetNamespace(i.owner.GetNamespace()) ownerutil.AddNonBlockingOwner(service, i.owner) @@ -274,10 +343,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo } // Create signed serving cert - hosts := []string{ - fmt.Sprintf("%s.%s", service.GetName(), i.owner.GetNamespace()), - fmt.Sprintf("%s.%s.svc", service.GetName(), i.owner.GetNamespace()), - } + hosts := HostnamesForService(serviceName, i.owner.GetNamespace()) servingPair, err := certGenerator.Generate(expiration, Organization, ca, hosts) if err != nil { logger.Warnf("could not generate signed certs for hosts %v", hosts) @@ -321,10 +387,10 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo // Attempt an update // TODO: Check that the secret was not modified - if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) { + if !shouldRotateCerts(existingSecret, HostnamesForService(serviceName, i.owner.GetNamespace())) { logger.Warnf("reusing existing cert %s", secret.GetName()) secret = existingSecret - caPEM = existingCAPEM + caPEM = existingSecret.Data[OLMCAPEMKey] caHash = certs.PEMSHA256(caPEM) } else { if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil { diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go b/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go index 206ce9e384..3e762fefe4 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/resolver.go @@ -21,6 +21,7 @@ type Strategy interface { type StrategyInstaller interface { Install(strategy Strategy) error CheckInstalled(strategy Strategy) (bool, error) + ShouldRotateCerts(strategy Strategy) (bool, error) CertsRotateAt() time.Time CertsRotated() bool } @@ -79,3 +80,7 @@ func (i *NullStrategyInstaller) CertsRotateAt() time.Time { func (i *NullStrategyInstaller) CertsRotated() bool { return false } + +func (i *NullStrategyInstaller) ShouldRotateCerts(s Strategy) (bool, error) { + return false, nil +} diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go index c71b5594f3..89bc4a4a00 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go @@ -78,7 +78,7 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, } serviceName := install.ServiceName(desc.DeploymentName) - service, err := a.lister.CoreV1().ServiceLister().Services(csv.GetNamespace()).Get(serviceName) + _, err = a.lister.CoreV1().ServiceLister().Services(csv.GetNamespace()).Get(serviceName) if err != nil { logger.WithField("service", serviceName).Warnf("could not retrieve generated Service") errs = append(errs, err) @@ -94,17 +94,12 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, // Check if CA is Active caBundle := apiService.Spec.CABundle - ca, err := certs.PEMToCert(caBundle) + _, err = certs.PEMToCert(caBundle) if err != nil { logger.Warnf("could not convert APIService CA bundle to x509 cert") errs = append(errs, err) continue } - if !certs.Active(ca) { - logger.Warnf("CA cert not active") - errs = append(errs, fmt.Errorf("found the CA cert is not active")) - continue - } // Check if serving cert is active secretName := install.SecretName(serviceName) @@ -114,17 +109,12 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, errs = append(errs, err) continue } - cert, err := certs.PEMToCert(secret.Data["tls.crt"]) + _, err = certs.PEMToCert(secret.Data["tls.crt"]) if err != nil { logger.Warnf("could not convert serving cert to x509 cert") errs = append(errs, err) continue } - if !certs.Active(cert) { - logger.Warnf("serving cert not active") - errs = append(errs, fmt.Errorf("found the serving cert not active")) - continue - } // Check if CA hash matches expected caHash := hashFunc(caBundle) @@ -134,18 +124,6 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, continue } - // Check if serving cert is trusted by the CA - hosts := []string{ - fmt.Sprintf("%s.%s", service.GetName(), csv.GetNamespace()), - fmt.Sprintf("%s.%s.svc", service.GetName(), csv.GetNamespace()), - } - for _, host := range hosts { - if err := certs.VerifyCert(ca, cert, host); err != nil { - errs = append(errs, fmt.Errorf("could not verify cert: %s", err.Error())) - continue - } - } - // Ensure the existing Deployment has a matching CA hash annotation deployment, err := a.lister.AppsV1().DeploymentLister().Deployments(csv.GetNamespace()).Get(desc.DeploymentName) if apierrors.IsNotFound(err) || err != nil { 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 6964fe9917..414c755920 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -2104,7 +2104,10 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v } // Check if it's time to refresh owned APIService certs - if install.ShouldRotateCerts(out) { + if shouldRotate, err := installer.ShouldRotateCerts(strategy); err != nil { + logger.WithError(err).Info("cert validity check") + return + } else if shouldRotate { logger.Debug("CSV owns resources that require a cert refresh") out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsCertRotation, "CSV owns resources that require a cert refresh", now, a.recorder) return @@ -2214,7 +2217,10 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v } // Check if it's time to refresh owned APIService certs - if install.ShouldRotateCerts(out) { + if shouldRotate, err := installer.ShouldRotateCerts(strategy); err != nil { + logger.WithError(err).Info("cert validity check") + return + } else if shouldRotate { logger.Debug("CSV owns resources that require a cert refresh") out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsCertRotation, "owned APIServices need cert refresh", now, a.recorder) return 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 0c49a95b60..b2e8b3ee26 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 @@ -97,6 +97,10 @@ func (i *TestInstaller) CheckInstalled(s install.Strategy) (bool, error) { return true, nil } +func (i *TestInstaller) ShouldRotateCerts(s install.Strategy) (bool, error) { + return false, nil +} + func (i *TestInstaller) CertsRotateAt() time.Time { return time.Time{} } @@ -489,6 +493,7 @@ func tlsSecret(name, namespace string, certPEM, privPEM []byte) *corev1.Secret { } secret.SetName(name) secret.SetNamespace(namespace) + secret.SetLabels(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}) return secret } @@ -1880,26 +1885,26 @@ func TestTransitionCSV(t *testing.T) { }, clientObjs: []runtime.Object{defaultOperatorGroup}, apis: []runtime.Object{ - apiService("a1", "v1", "v1-a1", namespace, "a1", expiredCAPEM, apiregistrationv1.ConditionTrue, ownerLabelFromCSV("csv1", namespace)), + apiService("a1", "v1", install.ServiceName("a1"), namespace, "a1", expiredCAPEM, apiregistrationv1.ConditionTrue, ownerLabelFromCSV("csv1", namespace)), }, objs: []runtime.Object{ deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{ install.OLMCAHashAnnotationKey: expiredCAHash, })), - 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{ + withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, signedServingPair(time.Now().Add(24*time.Hour), expiredCA, install.HostnamesForService(install.ServiceName("a1"), "ns"))), map[string]string{ install.OLMCAHashAnnotationKey: expiredCAHash, }), - service("v1-a1", namespace, "a1", 80), + service(install.ServiceName("a1"), namespace, "a1", 80), serviceAccount("sa", namespace), - role("v1.a1-cert", namespace, []rbacv1.PolicyRule{ + role(install.SecretName(install.ServiceName("a1")), namespace, []rbacv1.PolicyRule{ { Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"secrets"}, - ResourceNames: []string{"v1.a1-cert"}, + ResourceNames: []string{install.SecretName(install.ServiceName("a1"))}, }, }), - roleBinding("v1.a1-cert", namespace, "v1.a1-cert", "sa", namespace), + roleBinding(install.SecretName(install.ServiceName("a1")), namespace, install.SecretName(install.ServiceName("a1")), "sa", namespace), role("extension-apiserver-authentication-reader", "kube-system", []rbacv1.PolicyRule{ { Verbs: []string{"get"}, @@ -1908,7 +1913,7 @@ func TestTransitionCSV(t *testing.T) { ResourceNames: []string{"extension-apiserver-authentication"}, }, }), - roleBinding("v1.a1-auth-reader", "kube-system", "extension-apiserver-authentication-reader", "sa", namespace), + roleBinding(fmt.Sprintf("%s-auth-reader", install.ServiceName("a1")), "kube-system", "extension-apiserver-authentication-reader", "sa", namespace), clusterRole("system:auth-delegator", []rbacv1.PolicyRule{ { Verbs: []string{"create"}, @@ -1921,7 +1926,7 @@ func TestTransitionCSV(t *testing.T) { Resources: []string{"subjectaccessreviews"}, }, }), - clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), + clusterRoleBinding(fmt.Sprintf("%s-system:auth-delegator", install.ServiceName("a1")), "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ crd("c1", "v1", "g1"), @@ -1929,7 +1934,7 @@ func TestTransitionCSV(t *testing.T) { }, expected: expected{ csvStates: map[string]csvState{ - "csv1": {exists: true, phase: v1alpha1.CSVPhaseFailed, reason: v1alpha1.CSVReasonAPIServiceResourceIssue}, + "csv1": {exists: true, phase: v1alpha1.CSVPhaseFailed, reason: v1alpha1.CSVReasonNeedsCertRotation}, }, }, }, @@ -1949,26 +1954,26 @@ func TestTransitionCSV(t *testing.T) { }, clientObjs: []runtime.Object{defaultOperatorGroup}, apis: []runtime.Object{ - apiService("a1", "v1", "v1-a1", namespace, "a1", expiredCAPEM, apiregistrationv1.ConditionTrue, ownerLabelFromCSV("csv1", namespace)), + apiService("a1", "v1", install.ServiceName("a1"), namespace, "a1", expiredCAPEM, apiregistrationv1.ConditionTrue, ownerLabelFromCSV("csv1", namespace)), }, objs: []runtime.Object{ deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{ install.OLMCAHashAnnotationKey: expiredCAHash, })), - 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{ + withAnnotations(keyPairToTLSSecret(install.SecretName(install.ServiceName("a1")), namespace, signedServingPair(time.Now().Add(24*time.Hour), expiredCA, install.HostnamesForService(install.ServiceName("a1"), "ns"))), map[string]string{ install.OLMCAHashAnnotationKey: expiredCAHash, }), - service("v1-a1", namespace, "a1", 80), + service(install.ServiceName("a1"), namespace, "a1", 80), serviceAccount("sa", namespace), - role("v1.a1-cert", namespace, []rbacv1.PolicyRule{ + role(install.SecretName(install.ServiceName("a1")), namespace, []rbacv1.PolicyRule{ { Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"secrets"}, - ResourceNames: []string{"v1.a1-cert"}, + ResourceNames: []string{install.SecretName(install.ServiceName("a1"))}, }, }), - roleBinding("v1.a1-cert", namespace, "v1.a1-cert", "sa", namespace), + roleBinding(install.SecretName(install.ServiceName("a1")), namespace, install.SecretName(install.ServiceName("a1")), "sa", namespace), role("extension-apiserver-authentication-reader", "kube-system", []rbacv1.PolicyRule{ { Verbs: []string{"get"}, @@ -1977,7 +1982,7 @@ func TestTransitionCSV(t *testing.T) { ResourceNames: []string{"extension-apiserver-authentication"}, }, }), - roleBinding("v1.a1-auth-reader", "kube-system", "extension-apiserver-authentication-reader", "sa", namespace), + roleBinding(fmt.Sprintf("%s-auth-reader", install.ServiceName("a1")), "kube-system", "extension-apiserver-authentication-reader", "sa", namespace), clusterRole("system:auth-delegator", []rbacv1.PolicyRule{ { Verbs: []string{"create"}, @@ -1990,7 +1995,7 @@ func TestTransitionCSV(t *testing.T) { Resources: []string{"subjectaccessreviews"}, }, }), - clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), + clusterRoleBinding(fmt.Sprintf("%s-system:auth-delegator", install.ServiceName("a1")), "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ crd("c1", "v1", "g1"), @@ -1998,7 +2003,7 @@ func TestTransitionCSV(t *testing.T) { }, expected: expected{ csvStates: map[string]csvState{ - "csv1": {exists: true, phase: v1alpha1.CSVPhasePending, reason: v1alpha1.CSVReasonAPIServiceResourcesNeedReinstall}, + "csv1": {exists: true, phase: v1alpha1.CSVPhasePending, reason: v1alpha1.CSVReasonNeedsCertRotation}, }, }, }, diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/suite_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/suite_test.go index da10439784..e639720b5c 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/suite_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/suite_test.go @@ -108,5 +108,5 @@ var _ = BeforeSuite(func() { var _ = AfterSuite(func() { By("tearing down the test environment") close(syncCh) - Expect(testEnv.Stop()).To(Succeed()) + testEnv.Stop() }) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/suite_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/suite_test.go index 2ddfb18ae8..c75cd69b9f 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/suite_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/suite_test.go @@ -162,8 +162,7 @@ var _ = AfterSuite(func() { ctx.Done() By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).ToNot(HaveOccurred()) + testEnv.Stop() }) func newOperator(name string) *decorators.Operator { 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 7d6dcf2cec..492666d771 100644 --- a/staging/operator-lifecycle-manager/pkg/fakes/fake_strategy_installer.go +++ b/staging/operator-lifecycle-manager/pkg/fakes/fake_strategy_installer.go @@ -53,6 +53,19 @@ type FakeStrategyInstaller struct { installReturnsOnCall map[int]struct { result1 error } + ShouldRotateCertsStub func(install.Strategy) (bool, error) + shouldRotateCertsMutex sync.RWMutex + shouldRotateCertsArgsForCall []struct { + arg1 install.Strategy + } + shouldRotateCertsReturns struct { + result1 bool + result2 error + } + shouldRotateCertsReturnsOnCall map[int]struct { + result1 bool + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -284,6 +297,70 @@ func (fake *FakeStrategyInstaller) InstallReturnsOnCall(i int, result1 error) { }{result1} } +func (fake *FakeStrategyInstaller) ShouldRotateCerts(arg1 install.Strategy) (bool, error) { + fake.shouldRotateCertsMutex.Lock() + ret, specificReturn := fake.shouldRotateCertsReturnsOnCall[len(fake.shouldRotateCertsArgsForCall)] + fake.shouldRotateCertsArgsForCall = append(fake.shouldRotateCertsArgsForCall, struct { + arg1 install.Strategy + }{arg1}) + stub := fake.ShouldRotateCertsStub + fakeReturns := fake.shouldRotateCertsReturns + fake.recordInvocation("ShouldRotateCerts", []interface{}{arg1}) + fake.shouldRotateCertsMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStrategyInstaller) ShouldRotateCertsCallCount() int { + fake.shouldRotateCertsMutex.RLock() + defer fake.shouldRotateCertsMutex.RUnlock() + return len(fake.shouldRotateCertsArgsForCall) +} + +func (fake *FakeStrategyInstaller) ShouldRotateCertsCalls(stub func(install.Strategy) (bool, error)) { + fake.shouldRotateCertsMutex.Lock() + defer fake.shouldRotateCertsMutex.Unlock() + fake.ShouldRotateCertsStub = stub +} + +func (fake *FakeStrategyInstaller) ShouldRotateCertsArgsForCall(i int) install.Strategy { + fake.shouldRotateCertsMutex.RLock() + defer fake.shouldRotateCertsMutex.RUnlock() + argsForCall := fake.shouldRotateCertsArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeStrategyInstaller) ShouldRotateCertsReturns(result1 bool, result2 error) { + fake.shouldRotateCertsMutex.Lock() + defer fake.shouldRotateCertsMutex.Unlock() + fake.ShouldRotateCertsStub = nil + fake.shouldRotateCertsReturns = struct { + result1 bool + result2 error + }{result1, result2} +} + +func (fake *FakeStrategyInstaller) ShouldRotateCertsReturnsOnCall(i int, result1 bool, result2 error) { + fake.shouldRotateCertsMutex.Lock() + defer fake.shouldRotateCertsMutex.Unlock() + fake.ShouldRotateCertsStub = nil + if fake.shouldRotateCertsReturnsOnCall == nil { + fake.shouldRotateCertsReturnsOnCall = make(map[int]struct { + result1 bool + result2 error + }) + } + fake.shouldRotateCertsReturnsOnCall[i] = struct { + result1 bool + result2 error + }{result1, result2} +} + func (fake *FakeStrategyInstaller) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -295,6 +372,8 @@ func (fake *FakeStrategyInstaller) Invocations() map[string][][]interface{} { defer fake.checkInstalledMutex.RUnlock() fake.installMutex.RLock() defer fake.installMutex.RUnlock() + fake.shouldRotateCertsMutex.RLock() + defer fake.shouldRotateCertsMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go index da38d37bfb..a5de230eaa 100644 --- a/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go @@ -210,7 +210,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { return err }).Should(Succeed()) - _, err = awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Update manifest @@ -360,7 +360,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { subscription, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionStateAtLatestChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(subscription).ShouldNot(BeNil()) - _, err = fetchCSV(crc, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(v1alpha1.CSVPhaseSucceeded)) + _, err = fetchCSV(crc, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(v1alpha1.CSVPhaseSucceeded)) Expect(err).ShouldNot(HaveOccurred()) ipList, err := crc.OperatorsV1alpha1().InstallPlans(generatedNamespace.GetName()).List(context.Background(), metav1.ListOptions{}) @@ -451,7 +451,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { subscription, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionStateAtLatestChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(subscription).ToNot(BeNil()) - _, err = fetchCSV(crc, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(v1alpha1.CSVPhaseSucceeded)) + _, err = fetchCSV(crc, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(v1alpha1.CSVPhaseSucceeded)) Expect(err).ShouldNot(HaveOccurred()) }) @@ -593,7 +593,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { subscription, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionStateAtLatestChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(subscription).ShouldNot(BeNil()) - _, err = fetchCSV(crc, subscription.Status.CurrentCSV, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), subscription.Status.CurrentCSV, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Update the catalog's address to point at the other registry pod's cluster ip @@ -609,7 +609,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { }).Should(Succeed()) // Wait for the replacement CSV to be installed - _, err = awaitCSV(crc, generatedNamespace.GetName(), replacementCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), replacementCSV.GetName(), csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) }) @@ -876,7 +876,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { Expect(subscription).ShouldNot(BeNil()) // Wait for csv to succeed - _, err = fetchCSV(crc, subscription.Status.CurrentCSV, subscription.GetNamespace(), csvSucceededChecker) + _, err = fetchCSV(crc, subscription.GetNamespace(), subscription.Status.CurrentCSV, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) registryCheckFunc := func(podList *corev1.PodList) bool { @@ -981,7 +981,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { Expect(subscription).ShouldNot(BeNil()) // Wait for csv to succeed - csv, err := fetchCSV(crc, subscription.Status.CurrentCSV, subscription.GetNamespace(), csvSucceededChecker) + csv, err := fetchCSV(crc, subscription.GetNamespace(), subscription.Status.CurrentCSV, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // check version of running csv to ensure the latest version (0.9.2) was installed onto the cluster @@ -1092,8 +1092,8 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(subscription).ShouldNot(BeNil()) - // Wait for busybox v2 csv to succeed and check the replaces field - csv, err := fetchCSV(crc, subscription.Status.CurrentCSV, subscription.GetNamespace(), csvSucceededChecker) + By("waiting for busybox v2 csv to succeed and check the replaces field") + csv, err := fetchCSV(crc, subscription.GetNamespace(), subscription.Status.CurrentCSV, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(csv.Spec.Replaces).To(Equal("busybox.v1.0.0")) @@ -1105,8 +1105,8 @@ var _ = Describe("Starting CatalogSource e2e tests", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(subscription).ShouldNot(BeNil()) - // Wait for busybox-dependency v2 csv to succeed and check the replaces field - csv, err = fetchCSV(crc, subscription.Status.CurrentCSV, subscription.GetNamespace(), csvSucceededChecker) + By("waiting for busybox-dependency v2 csv to succeed and check the replaces field") + csv, err = fetchCSV(crc, subscription.GetNamespace(), subscription.Status.CurrentCSV, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(csv.Spec.Replaces).To(Equal("busybox-dependency.v1.0.0")) }) diff --git a/staging/operator-lifecycle-manager/test/e2e/crd_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/crd_e2e_test.go index 755581564d..ac5526a729 100644 --- a/staging/operator-lifecycle-manager/test/e2e/crd_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/crd_e2e_test.go @@ -39,7 +39,7 @@ var _ = Describe("CRD Versions", func() { }) // issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2640 - It("[FLAKE] creates v1 CRDs with a v1 schema successfully", func() { + XIt("[FLAKE] creates v1 CRDs with a v1 schema successfully", func() { By("v1 crds with a valid openapiv3 schema should be created successfully by OLM") mainPackageName := genName("nginx-update2-") @@ -265,14 +265,12 @@ var _ = Describe("CRD Versions", func() { return subscriptionStateAtLatestChecker(v) && v.Status.InstallPlanRef != nil && v.Status.InstallPlanRef.Name != fetchedInstallPlan.Name } - // fetch new subscription - s, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionAtLatestWithDifferentInstallPlan) - Expect(err).ToNot(HaveOccurred()) - Expect(s).ToNot(BeNil()) - Expect(s.Status.InstallPlanRef).ToNot(Equal(nil)) - // Check the error on the installplan - should be related to data loss and the CRD upgrade missing a stored version Eventually(func() (*operatorsv1alpha1.InstallPlan, error) { + s, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionAtLatestWithDifferentInstallPlan) + if err != nil || s.Status.InstallPlanRef == nil { + return nil, err + } return crc.OperatorsV1alpha1().InstallPlans(generatedNamespace.GetName()).Get(context.TODO(), s.Status.InstallPlanRef.Name, metav1.GetOptions{}) }).Should(And( WithTransform( @@ -298,7 +296,7 @@ var _ = Describe("CRD Versions", func() { // Update the CRD status to remove the v1alpha1 // Now the installplan should succeed - It("allows a CRD upgrade that doesn't cause data loss", func() { + XIt("allows a CRD upgrade that doesn't cause data loss", func() { By("manually editing the storage versions in the existing CRD status") crdPlural := genName("ins-v1-") diff --git a/staging/operator-lifecycle-manager/test/e2e/csv_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/csv_e2e_test.go index f2bd7b7876..2b0ffcab01 100644 --- a/staging/operator-lifecycle-manager/test/e2e/csv_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/csv_e2e_test.go @@ -9,7 +9,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" @@ -31,6 +30,7 @@ import ( apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "sigs.k8s.io/controller-runtime/pkg/client" + operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx" ) @@ -746,7 +746,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Shouldn't create deployment @@ -807,7 +807,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Shouldn't create deployment @@ -926,7 +926,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Shouldn't create deployment @@ -985,7 +985,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Shouldn't create deployment @@ -1072,7 +1072,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Shouldn't create deployment @@ -1121,7 +1121,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Shouldn't create deployment @@ -1215,7 +1215,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) sa := corev1.ServiceAccount{} @@ -1387,14 +1387,14 @@ var _ = Describe("ClusterServiceVersion", func() { return false, nil }).Should(BeTrue()) - fetchedCSV, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Delete CRD cleanupCRD() // Wait for CSV failure - fetchedCSV, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvPendingChecker) + fetchedCSV, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvPendingChecker) Expect(err).ShouldNot(HaveOccurred()) // Recreate the CRD @@ -1403,7 +1403,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupCRD() // Wait for CSV success again - fetchedCSV, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) }) It("create requirements met API service", func() { @@ -1556,11 +1556,11 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Fetch cluster service version again to check for unnecessary control loops - sameCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + sameCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(equality.Semantic.DeepEqual(fetchedCSV, sameCSV)).Should(BeTrue(), diff.ObjectDiff(fetchedCSV, sameCSV)) }) @@ -1658,7 +1658,7 @@ var _ = Describe("ClusterServiceVersion", func() { <-deleted }() - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should create Deployment @@ -1699,15 +1699,19 @@ var _ = Describe("ClusterServiceVersion", func() { oldCAAnnotation, ok := dep.Spec.Template.GetAnnotations()[install.OLMCAHashAnnotationKey] Expect(ok).Should(BeTrue(), "expected olm sha annotation not present on existing pod template") - // Induce a cert rotation - Eventually(Apply(fetchedCSV, func(csv *operatorsv1alpha1.ClusterServiceVersion) error { - now := metav1.Now() - csv.Status.CertsLastUpdated = &now - csv.Status.CertsRotateAt = &now + caSecret, err := c.KubernetesInterface().CoreV1().Secrets(generatedNamespace.GetName()).Get(context.TODO(), secretName, metav1.GetOptions{}) + Expect(err).Should(BeNil()) + + caPEM, certPEM, privPEM := generateExpiredCerts(install.HostnamesForService(serviceName, generatedNamespace.GetName())) + By(`Induce a cert rotation`) + Eventually(Apply(caSecret, func(caSecret *corev1.Secret) error { + caSecret.Data[install.OLMCAPEMKey] = caPEM + caSecret.Data["tls.crt"] = certPEM + caSecret.Data["tls.key"] = privPEM return nil })).Should(Succeed()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { // Should create deployment dep, err = c.GetDeployment(generatedNamespace.GetName(), depName) if err != nil { @@ -1737,9 +1741,9 @@ var _ = Describe("ClusterServiceVersion", func() { err = c.DeleteAPIService(apiServiceName, &metav1.DeleteOptions{}) Expect(err).ShouldNot(HaveOccurred()) - // Wait for CSV success - fetchedCSV, err = fetchCSV(crc, csv.GetName(), generatedNamespace.GetName(), func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { - // Should create an APIService + By("Wait for CSV success") + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.GetName(), func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { + By("Should create an APIService") apiService, err := c.GetAPIService(apiServiceName) if err != nil { return false @@ -1822,7 +1826,7 @@ var _ = Describe("ClusterServiceVersion", func() { _, err := createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).ShouldNot(HaveOccurred()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should create Deployment @@ -1898,7 +1902,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV2() - _, err = fetchCSV(crc, csv2.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv2.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should create Deployment @@ -1949,9 +1953,8 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred(), "error getting expected extension-apiserver-authentication-reader RoleBinding") // Should eventually GC the CSV - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csv.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csv.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) // Rename the initial CSV csv.SetName("csv-hat-3") @@ -1961,7 +1964,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - fetched, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), buildCSVReasonChecker(operatorsv1alpha1.CSVReasonOwnerConflict)) + fetched, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, buildCSVReasonChecker(operatorsv1alpha1.CSVReasonOwnerConflict)) Expect(err).ShouldNot(HaveOccurred()) Expect(fetched.Status.Phase).Should(Equal(operatorsv1alpha1.CSVPhaseFailed)) }) @@ -2080,7 +2083,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should create Deployment @@ -2154,7 +2157,7 @@ var _ = Describe("ClusterServiceVersion", func() { _, err = createCSV(c, crc, csv2, secondNamespaceName, false, true) Expect(err).ShouldNot(HaveOccurred()) - _, err = fetchCSV(crc, csv2.Name, secondNamespaceName, csvFailedChecker) + _, err = fetchCSV(crc, secondNamespaceName, csv2.Name, csvFailedChecker) Expect(err).ShouldNot(HaveOccurred()) }) It("orphaned API service clean up", func() { @@ -2282,7 +2285,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupCSV() // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -2353,7 +2356,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupCSV() // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -2468,7 +2471,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -2541,7 +2544,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupNewCSV() // Wait for updated CSV to succeed - fetchedCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have updated existing deployment @@ -2551,12 +2554,11 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(strategyNew.DeploymentSpecs[0].Spec.Template.Spec.Containers[0].Name).Should(Equal(depUpdated.Spec.Template.Spec.Containers[0].Name)) // Should eventually GC the CSV - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csv.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csv.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) // Fetch cluster service version again to check for unnecessary control loops - sameCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + sameCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(equality.Semantic.DeepEqual(fetchedCSV, sameCSV)).Should(BeTrue(), diff.ObjectDiff(fetchedCSV, sameCSV)) }) @@ -2659,7 +2661,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -2730,11 +2732,11 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupNewCSV() // Wait for updated CSV to succeed - fetchedCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Fetch cluster service version again to check for unnecessary control loops - sameCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + sameCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(equality.Semantic.DeepEqual(fetchedCSV, sameCSV)).Should(BeTrue(), diff.ObjectDiff(fetchedCSV, sameCSV)) @@ -2742,13 +2744,12 @@ var _ = Describe("ClusterServiceVersion", func() { depNew, err := c.GetDeployment(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name) Expect(err).ShouldNot(HaveOccurred()) Expect(depNew).ShouldNot(BeNil()) - err = waitForDeploymentToDelete(generatedNamespace.GetName(), c, strategy.DeploymentSpecs[0].Name) + err = waitForDeploymentToDelete(generatedNamespace.GetName(), strategy.DeploymentSpecs[0].Name, c) Expect(err).ShouldNot(HaveOccurred()) // Should eventually GC the CSV - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csv.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csv.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) }) It("update multiple intermediates", func() { @@ -2849,7 +2850,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -2920,11 +2921,11 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupNewCSV() // Wait for updated CSV to succeed - fetchedCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Fetch cluster service version again to check for unnecessary control loops - sameCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + sameCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(equality.Semantic.DeepEqual(fetchedCSV, sameCSV)).Should(BeTrue(), diff.ObjectDiff(fetchedCSV, sameCSV)) @@ -2932,13 +2933,12 @@ var _ = Describe("ClusterServiceVersion", func() { depNew, err := c.GetDeployment(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name) Expect(err).ShouldNot(HaveOccurred()) Expect(depNew).ShouldNot(BeNil()) - err = waitForDeploymentToDelete(generatedNamespace.GetName(), c, strategy.DeploymentSpecs[0].Name) + err = waitForDeploymentToDelete(generatedNamespace.GetName(), strategy.DeploymentSpecs[0].Name, c) Expect(err).ShouldNot(HaveOccurred()) // Should eventually GC the CSV - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csv.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csv.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) }) It("update in place", func() { @@ -3040,7 +3040,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupCSV() // Wait for current CSV to succeed - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -3086,7 +3086,7 @@ var _ = Describe("ClusterServiceVersion", func() { }).Should(Equal(strategyNew.DeploymentSpecs[0].Spec.Template.Spec.Containers[0].Name)) // Wait for updated CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) depUpdated, err := c.GetDeployment(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name) @@ -3211,7 +3211,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -3290,11 +3290,11 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Wait for updated CSV to succeed - fetchedCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Fetch cluster service version again to check for unnecessary control loops - sameCSV, err := fetchCSV(crc, csvNew.Name, generatedNamespace.GetName(), csvSucceededChecker) + sameCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvNew.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(equality.Semantic.DeepEqual(fetchedCSV, sameCSV)).Should(BeTrue(), diff.ObjectDiff(fetchedCSV, sameCSV)) @@ -3302,7 +3302,7 @@ var _ = Describe("ClusterServiceVersion", func() { depNew, err := c.GetDeployment(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name) Expect(err).ShouldNot(HaveOccurred()) Expect(depNew).ShouldNot(BeNil()) - err = waitForDeploymentToDelete(generatedNamespace.GetName(), c, strategy.DeploymentSpecs[0].Name) + err = waitForDeploymentToDelete(generatedNamespace.GetName(), strategy.DeploymentSpecs[0].Name, c) Expect(err).ShouldNot(HaveOccurred()) // Create updated deployment strategy @@ -3369,11 +3369,11 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupNewCSV() // Wait for updated CSV to succeed - fetchedCSV, err = fetchCSV(crc, csvNew2.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err = fetchCSV(crc, generatedNamespace.GetName(), csvNew2.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Fetch cluster service version again to check for unnecessary control loops - sameCSV, err = fetchCSV(crc, csvNew2.Name, generatedNamespace.GetName(), csvSucceededChecker) + sameCSV, err = fetchCSV(crc, generatedNamespace.GetName(), csvNew2.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) Expect(equality.Semantic.DeepEqual(fetchedCSV, sameCSV)).Should(BeTrue(), diff.ObjectDiff(fetchedCSV, sameCSV)) @@ -3381,13 +3381,12 @@ var _ = Describe("ClusterServiceVersion", func() { depNew, err = c.GetDeployment(generatedNamespace.GetName(), strategyNew2.DeploymentSpecs[0].Name) Expect(err).ShouldNot(HaveOccurred()) Expect(depNew).ShouldNot(BeNil()) - err = waitForDeploymentToDelete(generatedNamespace.GetName(), c, strategyNew.DeploymentSpecs[0].Name) + err = waitForDeploymentToDelete(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name, c) Expect(err).ShouldNot(HaveOccurred()) // Should clean up the CSV - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csvNew.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csvNew.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) }) It("update modify deployment name", func() { @@ -3492,7 +3491,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupCSV() // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployments @@ -3520,7 +3519,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Fetch the current csv - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Update csv with same strategy with different deployment's name @@ -3531,11 +3530,11 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) // Wait for new deployment to exist - err = waitForDeployment(generatedNamespace.GetName(), c, strategyNew.DeploymentSpecs[0].Name) + _, err = waitForDeployment(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name, c) Expect(err).ShouldNot(HaveOccurred()) // Wait for updated CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created new deployment and deleted old @@ -3548,7 +3547,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(depNew2).ShouldNot(BeNil()) - err = waitForDeploymentToDelete(generatedNamespace.GetName(), c, strategy.DeploymentSpecs[0].Name) + err = waitForDeploymentToDelete(generatedNamespace.GetName(), strategy.DeploymentSpecs[0].Name, c) Expect(err).ShouldNot(HaveOccurred()) }) It("update deployment spec in an existing CSV for a hotfix", func() { @@ -3652,7 +3651,7 @@ var _ = Describe("ClusterServiceVersion", func() { defer cleanupCSV() // Wait for current CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Should have created deployment @@ -3673,27 +3672,40 @@ var _ = Describe("ClusterServiceVersion", func() { } // Fetch the current csv - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Update csv with modified deployment spec fetchedCSV.Spec.InstallStrategy.StrategySpec = strategyNew Eventually(func() error { + // Fetch the current csv + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) + if err != nil { + return err + } + + // Update csv with modified deployment spec + fetchedCSV.Spec.InstallStrategy.StrategySpec = strategyNew + // Update the current csv _, err = crc.OperatorsV1alpha1().ClusterServiceVersions(generatedNamespace.GetName()).Update(context.TODO(), fetchedCSV, metav1.UpdateOptions{}) return err }).Should(Succeed()) // Wait for updated CSV to succeed - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { // Should have updated existing deployment depUpdated, err := c.GetDeployment(generatedNamespace.GetName(), strategyNew.DeploymentSpecs[0].Name) - Expect(err).ShouldNot(HaveOccurred()) - Expect(depUpdated).ShouldNot(BeNil()) + if err != nil || depUpdated == nil { + return false + } + // container name has been updated and differs from initial CSV spec and updated CSV spec - Expect(depUpdated.Spec.Template.Spec.Containers[0].Name).ShouldNot(Equal(strategyNew.DeploymentSpecs[0].Spec.Template.Spec.Containers[0].Name)) + if depUpdated.Spec.Template.Spec.Containers[0].Name != strategyNew.DeploymentSpecs[0].Spec.Template.Spec.Containers[0].Name { + return false + } // Check for success return csvSucceededChecker(csv) @@ -3892,7 +3904,7 @@ var _ = Describe("ClusterServiceVersion", func() { return false } - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvCheckPhaseAndRequirementStatus) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvCheckPhaseAndRequirementStatus) Expect(err).ShouldNot(HaveOccurred()) Expect(fetchedCSV.Status.RequirementStatus).Should(ContainElement(notServedStatus)) @@ -3972,7 +3984,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) checkLegacyAPIResources(generatedNamespace.GetName(), owned[0], true, c) @@ -4052,7 +4064,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) checkLegacyAPIResources(generatedNamespace.GetName(), owned[0], false, c) @@ -4167,7 +4179,7 @@ var _ = Describe("ClusterServiceVersion", func() { Expect(err).ShouldNot(HaveOccurred()) defer cleanupCSV() - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ShouldNot(HaveOccurred()) // Check that the APIService caBundles are equal @@ -4427,18 +4439,19 @@ var csvFailedChecker = buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseFailed var csvAnyChecker = buildCSVConditionChecker(operatorsv1alpha1.CSVPhasePending, operatorsv1alpha1.CSVPhaseSucceeded, operatorsv1alpha1.CSVPhaseReplacing, operatorsv1alpha1.CSVPhaseDeleting, operatorsv1alpha1.CSVPhaseFailed) var csvCopiedChecker = buildCSVReasonChecker(operatorsv1alpha1.CSVReasonCopied) -func fetchCSV(c versioned.Interface, name, namespace string, checker csvConditionChecker) (*operatorsv1alpha1.ClusterServiceVersion, error) { +func fetchCSV(c versioned.Interface, namespace, name string, checker csvConditionChecker) (*operatorsv1alpha1.ClusterServiceVersion, error) { var fetchedCSV *operatorsv1alpha1.ClusterServiceVersion - var err error - - Eventually(func() (bool, error) { + ctx.Ctx().Logf("waiting for CSV %s/%s to reach condition", namespace, name) + err := wait.Poll(pollInterval, pollDuration, func() (bool, error) { + var err error fetchedCSV, err = c.OperatorsV1alpha1().ClusterServiceVersions(namespace).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { - return false, err + ctx.Ctx().Logf("error getting csv %s/%s: %v", namespace, name, err) + return false, nil } ctx.Ctx().Logf("%s (%s): %s", fetchedCSV.Status.Phase, fetchedCSV.Status.Reason, fetchedCSV.Status.Message) return checker(fetchedCSV), nil - }).Should(BeTrue()) + }) if err != nil { ctx.Ctx().Logf("never got correct status: %#v", fetchedCSV.Status) @@ -4446,70 +4459,79 @@ func fetchCSV(c versioned.Interface, name, namespace string, checker csvConditio return fetchedCSV, err } -func awaitCSV(c versioned.Interface, namespace, name string, checker csvConditionChecker) (*operatorsv1alpha1.ClusterServiceVersion, error) { - var fetched *operatorsv1alpha1.ClusterServiceVersion - var err error +func waitForDeployment(namespace, name string, c operatorclient.ClientInterface) (*appsv1.Deployment, error) { + var fetched *appsv1.Deployment - Eventually(func() (bool, error) { - fetched, err = c.OperatorsV1alpha1().ClusterServiceVersions(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + ctx.Ctx().Logf("waiting for deployment %s/%s to be created", namespace, name) + err := wait.Poll(pollInterval, pollDuration, func() (bool, error) { + var err error + fetched, err = c.GetDeployment(namespace, name) if err != nil { - if apierrors.IsNotFound(err) { - return false, nil - } - return false, err + ctx.Ctx().Logf("error getting deployment %s/%s: %v", namespace, name, err) + return false, nil } - ctx.Ctx().Logf("%s - %s (%s): %s", name, fetched.Status.Phase, fetched.Status.Reason, fetched.Status.Message) - return checker(fetched), nil - }).Should(BeTrue()) + return true, nil + }) - if err != nil { - ctx.Ctx().Logf("never got correct status: %#v", fetched.Status) - } return fetched, err } -func waitForDeployment(namespace string, c operatorclient.ClientInterface, name string) error { - return wait.Poll(pollInterval, pollDuration, func() (bool, error) { - _, err := c.GetDeployment(namespace, name) +func waitForDeploymentToDelete(namespace, name string, c operatorclient.ClientInterface) error { + var lastReplicas, lastUpdated, lastReady, lastAvailable, lastUnavailable int32 + lastTime := time.Now() + + ctx.Ctx().Logf("waiting for deployment %s/%s to delete", namespace, name) + err := wait.Poll(pollInterval, pollDuration, func() (bool, error) { + dep, err := c.GetDeployment(namespace, name) + if apierrors.IsNotFound(err) { + ctx.Ctx().Logf("deployment %s/%s deleted", namespace, name) + return true, nil + } if err != nil { - if apierrors.IsNotFound(err) { - return false, nil + ctx.Ctx().Logf("error getting deployment %s/%s: %v", namespace, name, err) + } + if dep != nil { + replicas, updated, ready, available, unavailable := dep.Status.Replicas, dep.Status.UpdatedReplicas, dep.Status.ReadyReplicas, dep.Status.AvailableReplicas, dep.Status.UnavailableReplicas + if replicas != lastReplicas || updated != lastUpdated || ready != lastReady || available != lastAvailable || unavailable != lastUnavailable { + ctx.Ctx().Logf("waited %s for deployment %s/%s status: rep: %v upd: %v rdy: %v ava: %v una: %v", time.Since(lastTime), replicas, updated, ready, available, unavailable) + lastReplicas, lastUpdated, lastReady, lastAvailable, lastUnavailable = replicas, updated, ready, available, unavailable + lastTime = time.Now() } - return false, err } - return true, nil + return false, nil }) + + return err } -func waitForDeploymentToDelete(namespace string, c operatorclient.ClientInterface, name string) error { - var err error +func waitForCsvToDelete(namespace, name string, c versioned.Interface) error { + var lastPhase operatorsv1alpha1.ClusterServiceVersionPhase + var lastReason operatorsv1alpha1.ConditionReason + var lastMessage string + lastTime := time.Now() - Eventually(func() (bool, error) { - ctx.Ctx().Logf("waiting for deployment %s to delete", name) - _, err := c.GetDeployment(namespace, name) + ctx.Ctx().Logf("waiting for csv %s/%s to delete", namespace, name) + err := wait.Poll(pollInterval, pollDuration, func() (bool, error) { + csv, err := c.OperatorsV1alpha1().ClusterServiceVersions(namespace).Get(context.TODO(), name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { - ctx.Ctx().Logf("deleted %s", name) + ctx.Ctx().Logf("csv %s/%s deleted", namespace, name) return true, nil } if err != nil { - ctx.Ctx().Logf("err trying to delete %s: %s", name, err) - return false, err + ctx.Ctx().Logf("error getting csv %s/%s: %v", namespace, name, err) + } + if csv != nil { + phase, reason, message := csv.Status.Phase, csv.Status.Reason, csv.Status.Message + if phase != lastPhase || reason != lastReason || message != lastMessage { + ctx.Ctx().Logf("waited %s for csv %s/%s status: %s (%s): %s", time.Since(lastTime), namespace, name, phase, reason, message) + lastPhase, lastReason, lastMessage = phase, reason, message + lastTime = time.Now() + } } return false, nil - }).Should(BeTrue()) - return err -} + }) -func csvExists(namespace string, c versioned.Interface, name string) bool { - fetched, err := c.OperatorsV1alpha1().ClusterServiceVersions(namespace).Get(context.TODO(), name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - return false - } - ctx.Ctx().Logf("%s (%s): %s", fetched.Status.Phase, fetched.Status.Reason, fetched.Status.Message) - if err != nil { - return true - } - return true + return err } func deleteLegacyAPIResources(namespace string, desc operatorsv1alpha1.APIServiceDescription, c operatorclient.ClientInterface) { diff --git a/staging/operator-lifecycle-manager/test/e2e/fail_forward_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/fail_forward_e2e_test.go index a5f3c217d6..852cf8ac17 100644 --- a/staging/operator-lifecycle-manager/test/e2e/fail_forward_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/fail_forward_e2e_test.go @@ -90,7 +90,7 @@ var _ = Describe("Fail Forward Upgrades", func() { originalInstallPlanRef := subscription.Status.InstallPlanRef By("waiting for the v0.1.0 CSV to report a succeeded phase") - _, err = fetchCSV(crclient, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) + _, err = fetchCSV(crclient, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) Expect(err).ShouldNot(HaveOccurred()) By("updating the catalog with a broken v0.2.0 bundle image") @@ -227,7 +227,7 @@ var _ = Describe("Fail Forward Upgrades", func() { Expect(err).Should(BeNil()) By("waiting for the v0.1.0 CSV to report a succeeded phase") - _, err = fetchCSV(crclient, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) + _, err = fetchCSV(crclient, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) Expect(err).ShouldNot(HaveOccurred()) By("updating the catalog with a broken v0.2.0 csv") @@ -243,7 +243,7 @@ var _ = Describe("Fail Forward Upgrades", func() { Expect(err).Should(BeNil()) By("waiting for the bad CSV to report a failed state") - _, err = fetchCSV(crclient, subscription.Status.CurrentCSV, generatedNamespace.GetName(), csvFailedChecker) + _, err = fetchCSV(crclient, generatedNamespace.GetName(), subscription.Status.CurrentCSV, csvFailedChecker) Expect(err).To(BeNil()) }) diff --git a/staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go index 121fe070ca..5aeeeb110a 100644 --- a/staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go @@ -814,7 +814,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), dependentCSV.GetName(), dependentSubscription.Status.CurrentCSV) // Verify CSV is created - _, err = awaitCSV(crc, generatedNamespace.GetName(), dependentCSV.GetName(), csvAnyChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), dependentCSV.GetName(), csvAnyChecker) require.NoError(GinkgoT(), err) // Update dependent subscription in catalog and wait for csv to update @@ -842,7 +842,7 @@ var _ = Describe("Install Plan", func() { require.NotEqual(GinkgoT(), fetchedInstallPlan.GetName(), fetchedUpdatedDepInstallPlan.GetName()) // Wait for csv to update - _, err = awaitCSV(crc, generatedNamespace.GetName(), updatedDependentCSV.GetName(), csvAnyChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), updatedDependentCSV.GetName(), csvAnyChecker) require.NoError(GinkgoT(), err) }) @@ -1416,7 +1416,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), tt.expectedPhase, fetchedInstallPlan.Status.Phase) // Ensure correct in-cluster resource(s) - fetchedCSV, err := fetchCSV(crc, mainBetaCSV.GetName(), generatedNamespace.GetName(), csvAnyChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), mainBetaCSV.GetName(), csvAnyChecker) require.NoError(GinkgoT(), err) GinkgoT().Logf("All expected resources resolved %s", fetchedCSV.Status.Phase) @@ -1640,7 +1640,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), tt.expectedPhase, fetchedInstallPlan.Status.Phase) // Ensure correct in-cluster resource(s) - fetchedCSV, err := fetchCSV(crc, mainBetaCSV.GetName(), generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), mainBetaCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure CRD versions are accurate @@ -1680,7 +1680,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), tt.expectedPhase, fetchedInstallPlan.Status.Phase) // Ensure correct in-cluster resource(s) - fetchedCSV, err = fetchCSV(crc, mainDeltaCSV.GetName(), generatedNamespace.GetName(), csvSucceededChecker) + fetchedCSV, err = fetchCSV(crc, generatedNamespace.GetName(), mainDeltaCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure CRD versions are accurate @@ -1820,7 +1820,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedInstallPlan.Status.Phase) // Verify CSV is created - _, err = awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Update CatalogSource with a new CSV with more permissions @@ -1886,7 +1886,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedUpdatedInstallPlan.Status.Phase) // Wait for csv to update - _, err = awaitCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // If the CSV is succeeded, we successfully rolled out the RBAC changes @@ -2018,7 +2018,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedInstallPlan.Status.Phase) // Verify CSV is created - _, err = awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Update CatalogSource with a new CSV with more permissions @@ -2078,7 +2078,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedUpdatedInstallPlan.Status.Phase) // Wait for csv to update - _, err = awaitCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) newSecrets, err := c.KubernetesInterface().CoreV1().Secrets(generatedNamespace.GetName()).List(context.Background(), metav1.ListOptions{}) @@ -2234,7 +2234,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedInstallPlan.Status.Phase) // Verify CSV is created - csv, err := awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) + csv, err := fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) addedEnvVar := corev1.EnvVar{Name: "EXAMPLE", Value: "value"} @@ -2275,7 +2275,7 @@ var _ = Describe("Install Plan", func() { require.NoError(GinkgoT(), err) // Wait for csv to update - _, err = awaitCSV(crc, generatedNamespace.GetName(), csv.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Should have the updated env var @@ -2323,7 +2323,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedUpdatedInstallPlan.Status.Phase) // Wait for csv to update - _, err = awaitCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Should have created deployment and stomped on the env changes @@ -2480,7 +2480,7 @@ var _ = Describe("Install Plan", func() { require.NoError(GinkgoT(), err) // Verify CSV is created - _, err = awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvAnyChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvAnyChecker) require.NoError(GinkgoT(), err) mainManifests = []registry.PackageManifest{ @@ -2504,7 +2504,7 @@ var _ = Describe("Install Plan", func() { require.NotEqual(GinkgoT(), fetchedInstallPlan.GetName(), fetchedUpdatedInstallPlan.GetName()) // Wait for csv to update - _, err = awaitCSV(crc, generatedNamespace.GetName(), betaCSV.GetName(), csvAnyChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), betaCSV.GetName(), csvAnyChecker) require.NoError(GinkgoT(), err) // Get the CRD to see if it is updated @@ -2680,7 +2680,7 @@ var _ = Describe("Install Plan", func() { require.NoError(GinkgoT(), err) // Verify CSV is created - _, err = awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvAnyChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvAnyChecker) require.NoError(GinkgoT(), err) // Get the CRD to see if it is updated @@ -3276,7 +3276,7 @@ var _ = Describe("Install Plan", func() { require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedInstallPlan.Status.Phase) // Verify CSV is created - _, err = awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Make sure to clean up the installed CRD diff --git a/staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go index 0d0d119c10..711be269b9 100644 --- a/staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go @@ -80,7 +80,7 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() { cleanupCSV, err = createCSV(c, crc, failingCSV, generatedNamespace.GetName(), false, false) Expect(err).ToNot(HaveOccurred()) - _, err = fetchCSV(crc, failingCSV.Name, generatedNamespace.GetName(), csvFailedChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), failingCSV.Name, csvFailedChecker) Expect(err).ToNot(HaveOccurred()) }) @@ -136,7 +136,7 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() { var err error _, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).ToNot(HaveOccurred()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).ToNot(HaveOccurred()) }) diff --git a/staging/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go index c10ba615d4..1bf2a4ffc1 100644 --- a/staging/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go @@ -78,7 +78,7 @@ var _ = Describe("Operator Condition", func() { defer cleanupSub() // Await csvA's success - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Get the OperatorCondition for csvA and report that it is not upgradeable @@ -121,7 +121,7 @@ var _ = Describe("Operator Condition", func() { require.NoError(GinkgoT(), err) // csvB will be in Pending phase due to csvA reports Upgradeable=False condition - fetchedCSV, err := fetchCSV(crc, csvB.GetName(), generatedNamespace.GetName(), buildCSVReasonChecker(operatorsv1alpha1.CSVReasonOperatorConditionNotUpgradeable)) + fetchedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csvB.GetName(), buildCSVReasonChecker(operatorsv1alpha1.CSVReasonOperatorConditionNotUpgradeable)) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), fetchedCSV.Status.Phase, operatorsv1alpha1.CSVPhasePending) @@ -146,7 +146,7 @@ var _ = Describe("Operator Condition", func() { }, pollInterval, pollDuration).Should(Succeed()) // Await csvB's success - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Get the OperatorCondition for csvB and purposedly change ObservedGeneration @@ -180,7 +180,7 @@ var _ = Describe("Operator Condition", func() { require.NoError(GinkgoT(), err) // CSVD will be in Pending status due to overrides in csvB's condition - fetchedCSV, err = fetchCSV(crc, csvD.GetName(), generatedNamespace.GetName(), buildCSVReasonChecker(operatorsv1alpha1.CSVReasonOperatorConditionNotUpgradeable)) + fetchedCSV, err = fetchCSV(crc, generatedNamespace.GetName(), csvD.GetName(), buildCSVReasonChecker(operatorsv1alpha1.CSVReasonOperatorConditionNotUpgradeable)) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), fetchedCSV.Status.Phase, operatorsv1alpha1.CSVPhasePending) @@ -198,7 +198,7 @@ var _ = Describe("Operator Condition", func() { require.NoError(GinkgoT(), err) require.NoError(GinkgoT(), err) - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvD.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvD.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) }) }) diff --git a/staging/operator-lifecycle-manager/test/e2e/operator_groups_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/operator_groups_e2e_test.go index fc0b5e743a..9768b1d3ff 100644 --- a/staging/operator-lifecycle-manager/test/e2e/operator_groups_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/operator_groups_e2e_test.go @@ -375,7 +375,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err, "could not update csv installmodes") // Ensure CSV fails - _, err = fetchCSV(crc, csvName, opGroupNamespace, csvFailedChecker) + _, err = fetchCSV(crc, opGroupNamespace, csvName, csvFailedChecker) require.NoError(GinkgoT(), err, "csv did not transition to failed as expected") // ensure deletion cleans up copied CSV @@ -469,7 +469,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) defer cleanupCRD() - _, err = fetchCSV(crc, csvA.GetName(), nsA, csvSucceededChecker) + _, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Create a csv for an apiserver @@ -540,7 +540,8 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) defer cleanupCSV() - _, err = fetchCSV(crc, csvB.GetName(), nsA, csvSucceededChecker) + GinkgoT().Logf("Fetch the APIService CSV %s/%s", nsA, depName) + _, err = fetchCSV(crc, nsA, csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure clusterroles created and aggregated for access provided APIs @@ -678,7 +679,7 @@ var _ = Describe("Operator Group", func() { failedWithUnsupportedOperatorGroup := func(csv *v1alpha1.ClusterServiceVersion) bool { return csvFailedChecker(csv) && csv.Status.Reason == v1alpha1.CSVReasonUnsupportedOperatorGroup } - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, failedWithUnsupportedOperatorGroup) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), failedWithUnsupportedOperatorGroup) require.NoError(GinkgoT(), err) // Update csvA to have OwnNamespace supported=true @@ -709,7 +710,7 @@ var _ = Describe("Operator Group", func() { defer cleanupCRD() // Ensure csvA transitions to Succeeded - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, csvSucceededChecker) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Update operatorGroupA's target namespaces to select namespaceB @@ -720,7 +721,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA transitions to Failed with reason "UnsupportedOperatorGroup" - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, failedWithUnsupportedOperatorGroup) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), failedWithUnsupportedOperatorGroup) require.NoError(GinkgoT(), err) // Update csvA to have SingleNamespace supported=true @@ -746,7 +747,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA transitions to Succeeded - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, csvSucceededChecker) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Update operatorGroupA's target namespaces to select namespaceA and namespaceB @@ -757,7 +758,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA transitions to Failed with reason "UnsupportedOperatorGroup" - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, failedWithUnsupportedOperatorGroup) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), failedWithUnsupportedOperatorGroup) require.NoError(GinkgoT(), err) // Update csvA to have MultiNamespace supported=true @@ -783,7 +784,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA transitions to Succeeded - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, csvSucceededChecker) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Update operatorGroupA's target namespaces to select all namespaces @@ -794,7 +795,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA transitions to Failed with reason "UnsupportedOperatorGroup" - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, failedWithUnsupportedOperatorGroup) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), failedWithUnsupportedOperatorGroup) require.NoError(GinkgoT(), err) // Update csvA to have AllNamespaces supported=true @@ -820,7 +821,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA transitions to Pending - csvA, err = fetchCSV(crc, csvA.GetName(), nsA, csvSucceededChecker) + csvA, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) }) It("intersection", func() { @@ -953,11 +954,11 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subD) // Await csvD's success - _, err = awaitCSV(crc, nsD, csvD.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, nsD, csvD.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Await csvD's copy in namespaceE - _, err = awaitCSV(crc, nsE, csvD.GetName(), csvCopiedChecker) + _, err = fetchCSV(crc, nsE, csvD.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) // Await annotation on groupD @@ -976,7 +977,7 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subD2) // Await csvD2's failure - csvD2, err := awaitCSV(crc, nsA, csvD.GetName(), csvFailedChecker) + csvD2, err := fetchCSV(crc, nsA, csvD.GetName(), csvFailedChecker) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, csvD2.Status.Reason) @@ -988,7 +989,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), awaitAnnotations(GinkgoT(), q, map[string]string{})) // Ensure csvD is still successful - _, err = awaitCSV(crc, nsD, csvD.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, nsD, csvD.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Create subscription for csvA in namespaceA @@ -1000,7 +1001,7 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subA) // Await csvA's success - _, err = awaitCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure clusterroles created and aggregated for access provided APIs @@ -1039,7 +1040,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), awaitAnnotations(GinkgoT(), q, map[string]string{v1.OperatorGroupProvidedAPIsAnnotationKey: kvgA})) // Wait for csvA to have a CSV with copied status in namespace D - csvAinNsD, err := awaitCSV(crc, nsD, csvA.GetName(), csvCopiedChecker) + csvAinNsD, err := fetchCSV(crc, nsD, csvA.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) // trigger a resync of operatorgropuD @@ -1051,7 +1052,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Ensure csvA retains the operatorgroup annotations for operatorgroupA - csvAinNsD, err = awaitCSV(crc, nsD, csvA.GetName(), csvCopiedChecker) + csvAinNsD, err = fetchCSV(crc, nsD, csvA.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), groupA.GetName(), csvAinNsD.Annotations[v1.OperatorGroupAnnotationKey]) @@ -1059,7 +1060,7 @@ var _ = Describe("Operator Group", func() { require.Equal(GinkgoT(), nsA, csvAinNsD.Labels[v1alpha1.CopiedLabelKey]) // Await csvA's copy in namespaceC - _, err = awaitCSV(crc, nsC, csvA.GetName(), csvCopiedChecker) + _, err = fetchCSV(crc, nsC, csvA.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) // Create subscription for csvB in namespaceB @@ -1071,7 +1072,7 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subB) // Await csvB's failure - fetchedB, err := awaitCSV(crc, nsB, csvB.GetName(), csvFailedChecker) + fetchedB, err := fetchCSV(crc, nsB, csvB.GetName(), csvFailedChecker) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, fetchedB.Status.Reason) @@ -1093,14 +1094,14 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), awaitAnnotations(GinkgoT(), q, map[string]string{v1.OperatorGroupProvidedAPIsAnnotationKey: ""})) // Ensure csvA's deployment is deleted - require.NoError(GinkgoT(), waitForDeploymentToDelete(generatedNamespace.GetName(), c, pkgAStable)) + require.NoError(GinkgoT(), waitForDeploymentToDelete(generatedNamespace.GetName(), pkgAStable, c)) // Await csvB's success - _, err = awaitCSV(crc, nsB, csvB.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, nsB, csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Await csvB's copy in namespace C - _, err = awaitCSV(crc, nsC, csvB.GetName(), csvCopiedChecker) + _, err = fetchCSV(crc, nsC, csvB.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) // Ensure annotations exist on group B @@ -1216,7 +1217,7 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subA) // Await csvA's failure - fetchedCSVA, err := awaitCSV(crc, nsB, csvA.GetName(), csvFailedChecker) + fetchedCSVA, err := fetchCSV(crc, nsB, csvA.GetName(), csvFailedChecker) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, fetchedCSVA.Status.Reason) @@ -1242,7 +1243,7 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subAC) // Await csvA's success - _, err = awaitCSV(crc, nsC, csvA.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, nsC, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure operatorGroupC has KindA.version.group in its providedAPIs annotation @@ -1268,15 +1269,15 @@ var _ = Describe("Operator Group", func() { require.NotNil(GinkgoT(), subB) // Await csvB's success - _, err = awaitCSV(crc, nsB, csvB.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, nsB, csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Await copied csvBs - _, err = awaitCSV(crc, nsA, csvB.GetName(), csvCopiedChecker) + _, err = fetchCSV(crc, nsA, csvB.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) - _, err = awaitCSV(crc, nsC, csvB.GetName(), csvCopiedChecker) + _, err = fetchCSV(crc, nsC, csvB.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) - _, err = awaitCSV(crc, nsD, csvB.GetName(), csvCopiedChecker) + _, err = fetchCSV(crc, nsD, csvB.GetName(), csvCopiedChecker) require.NoError(GinkgoT(), err) // Ensure operatorGroupB has KindB.version.group in its providedAPIs annotation @@ -1301,7 +1302,7 @@ var _ = Describe("Operator Group", func() { require.NoError(GinkgoT(), err) // Wait for csvA in namespaceC to fail with status "InterOperatorGroupOwnerConflict" - fetchedCSVA, err = awaitCSV(crc, nsC, csvA.GetName(), csvFailedChecker) + fetchedCSVA, err = fetchCSV(crc, nsC, csvA.GetName(), csvFailedChecker) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, fetchedCSVA.Status.Reason) @@ -1438,7 +1439,7 @@ var _ = Describe("Operator Group", func() { _, err = c.UpdateRoleBinding(createdRoleBinding) require.NoError(GinkgoT(), err) GinkgoT().Log("wait for CSV to succeed") - _, err = fetchCSV(crc, createdCSV.GetName(), opGroupNamespace, csvSucceededChecker) + _, err = fetchCSV(crc, opGroupNamespace, createdCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) GinkgoT().Log("wait for roles to be promoted to clusterroles") var fetchedRole *rbacv1.ClusterRole @@ -1917,7 +1918,7 @@ var _ = Describe("Operator Group", func() { _, err = c.UpdateRoleBinding(createdRoleBinding) require.NoError(GinkgoT(), err) GinkgoT().Log("wait for CSV to succeed") - _, err = fetchCSV(crc, createdCSV.GetName(), opGroupNamespace, csvSucceededChecker) + _, err = fetchCSV(crc, opGroupNamespace, createdCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) GinkgoT().Log("wait for roles to be promoted to clusterroles") var fetchedRole *rbacv1.ClusterRole diff --git a/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go index 766c0a5317..e954a37cee 100644 --- a/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go @@ -32,7 +32,6 @@ import ( "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection" - "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/comparison" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" "github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx" registryapi "github.com/operator-framework/operator-registry/pkg/api" @@ -160,7 +159,7 @@ var _ = Describe("Subscription", func() { return false }, 5*time.Minute, 10*time.Second).Should(BeTrue()) - csv, err := fetchCSV(crc, currentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) + csv, err := fetchCSV(crc, generatedNamespace.GetName(), currentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) require.NoError(GinkgoT(), err) // Check for the olm.package property as a proxy for @@ -192,7 +191,7 @@ var _ = Describe("Subscription", func() { subscription, err := fetchSubscription(crc, generatedNamespace.GetName(), testSubscriptionName, subscriptionStateAtLatestChecker) require.NoError(GinkgoT(), err) require.NotNil(GinkgoT(), subscription) - _, err = fetchCSV(crc, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) + _, err = fetchCSV(crc, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) require.NoError(GinkgoT(), err) }) It("skip range", func() { @@ -277,14 +276,14 @@ var _ = Describe("Subscription", func() { defer subscriptionCleanup() // Wait for csv to install - firstCSV, err := awaitCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) + firstCSV, err := fetchCSV(crc, generatedNamespace.GetName(), mainCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Update catalog with a new csv in the channel with a skip range updateInternalCatalog(GinkgoT(), c, crc, mainCatalogName, generatedNamespace.GetName(), []apiextensions.CustomResourceDefinition{crd}, []operatorsv1alpha1.ClusterServiceVersion{updatedCSV}, updatedManifests) // Wait for csv to update - finalCSV, err := awaitCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) + finalCSV, err := fetchCSV(crc, generatedNamespace.GetName(), updatedCSV.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure we set the replacement field based on the registry data @@ -349,10 +348,9 @@ var _ = Describe("Subscription", func() { require.NoError(GinkgoT(), err) require.NotNil(GinkgoT(), subscription) - _, err = fetchCSV(crc, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) + _, err = fetchCSV(crc, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) require.NoError(GinkgoT(), err) }) - It("with starting CSV", func() { crdPlural := genName("ins") @@ -475,7 +473,7 @@ var _ = Describe("Subscription", func() { _, err = crc.OperatorsV1alpha1().InstallPlans(generatedNamespace.GetName()).Update(context.Background(), fetchedInstallPlan, metav1.UpdateOptions{}) require.NoError(GinkgoT(), err) - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Wait for the subscription to begin upgrading to csvB @@ -485,15 +483,21 @@ var _ = Describe("Subscription", func() { return subscription != nil && subscription.Status.InstallPlanRef.Name != fetchedInstallPlan.GetName() && subscription.Status.State == operatorsv1alpha1.SubscriptionStateUpgradePending, err }, 5*time.Minute, 1*time.Second).Should(BeTrue(), "expected new installplan for upgraded csv") - upgradeInstallPlan, err := fetchInstallPlanWithNamespace(GinkgoT(), crc, subscription.Status.InstallPlanRef.Name, generatedNamespace.GetName(), requiresApprovalChecker) - require.NoError(GinkgoT(), err) - - // Approve the upgrade installplan and wait for - upgradeInstallPlan.Spec.Approved = true - _, err = crc.OperatorsV1alpha1().InstallPlans(generatedNamespace.GetName()).Update(context.Background(), upgradeInstallPlan, metav1.UpdateOptions{}) - require.NoError(GinkgoT(), err) + // Approve install plan + Eventually(func() (bool, error) { + upgradeInstallPlan, err := fetchInstallPlanWithNamespace(GinkgoT(), crc, subscription.Status.InstallPlanRef.Name, generatedNamespace.GetName(), requiresApprovalChecker) + if err != nil { + return false, nil + } - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) + // Approve the upgrade installplan and wait for + upgradeInstallPlan.Spec.Approved = true + if _, err = crc.OperatorsV1alpha1().InstallPlans(generatedNamespace.GetName()).Update(context.Background(), upgradeInstallPlan, metav1.UpdateOptions{}); err != nil { + return false, nil + } + return true, nil + }, 1*time.Minute, 5*time.Second).Should(BeTrue(), "expected new installplan for upgraded csv") + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure that 2 installplans were created @@ -544,7 +548,7 @@ var _ = Describe("Subscription", func() { require.NotNil(GinkgoT(), subscription) // Wait for csvA to be installed - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Set up async watches that will fail the test if csvB doesn't get created in between csvA and csvC @@ -553,7 +557,7 @@ var _ = Describe("Subscription", func() { defer GinkgoRecover() wg.Add(1) defer wg.Done() - _, err := awaitCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvReplacingChecker) + _, err := fetchCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvReplacingChecker) require.NoError(GinkgoT(), err) }(GinkgoT()) // Update the catalog to include multiple updates @@ -573,17 +577,15 @@ var _ = Describe("Subscription", func() { wg.Wait() // Wait for csvC to be installed - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvC.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvC.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Should eventually GC the CSVs - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csvA.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csvA.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csvB.Name) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csvB.Name, crc) + Expect(err).ShouldNot(HaveOccurred()) // TODO: check installplans, subscription status, etc }) @@ -626,7 +628,7 @@ var _ = Describe("Subscription", func() { createSubscriptionForCatalog(crc, generatedNamespace.GetName(), subscriptionName, catalogSourceName, packageName, stableChannel, csvB.GetName(), operatorsv1alpha1.ApprovalAutomatic) // Wait for csvB to be installed - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) subscription, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionHasInstallPlanChecker) @@ -682,7 +684,7 @@ var _ = Describe("Subscription", func() { require.NoError(GinkgoT(), err) // Wait for csvA to be installed - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Wait for the subscription to begin upgrading to csvB @@ -700,7 +702,7 @@ var _ = Describe("Subscription", func() { require.NoError(GinkgoT(), err) // Wait for csvB to be installed - _, err = awaitCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csvB.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) }) @@ -1022,6 +1024,7 @@ var _ = Describe("Subscription", func() { // - Wait for sub to have status condition SubscriptionInstallPlanMissing true // - Ensure original non-InstallPlan status conditions remain after InstallPlan transitions It("can reconcile InstallPlan status", func() { + By(`TestSubscriptionInstallPlanStatus ensures that a Subscription has the appropriate status conditions for possible referenced InstallPlan states.`) c := newKubeClient() crc := newCRClient() @@ -1068,6 +1071,7 @@ var _ = Describe("Subscription", func() { ref := sub.Status.InstallPlanRef Expect(ref).ToNot(BeNil()) + By(`Get the InstallPlan`) plan := &operatorsv1alpha1.InstallPlan{} plan.SetNamespace(ref.Namespace) plan.SetName(ref.Name) @@ -1179,16 +1183,13 @@ var _ = Describe("Subscription", func() { Expect(err).ToNot(HaveOccurred()) Expect(sub).ToNot(BeNil()) - // Ensure original non-InstallPlan status conditions remain after InstallPlan transitions - hashEqual := comparison.NewHashEqualitor() + By(`Ensure InstallPlan-related status conditions match what we're expecting`) for _, cond := range conds { switch condType := cond.Type; condType { case operatorsv1alpha1.SubscriptionInstallPlanPending, operatorsv1alpha1.SubscriptionInstallPlanFailed: require.FailNowf(GinkgoT(), "failed", "subscription contains unexpected installplan condition: %v", cond) case operatorsv1alpha1.SubscriptionInstallPlanMissing: require.Equal(GinkgoT(), operatorsv1alpha1.ReferencedInstallPlanNotFound, cond.Reason) - default: - require.True(GinkgoT(), hashEqual(cond, sub.Status.GetCondition(condType)), "non-installplan status condition changed") } } }) @@ -1337,7 +1338,7 @@ var _ = Describe("Subscription", func() { expected = append(expected, proxyEnv...) Eventually(func() error { - csv, err := fetchCSV(crClient, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) + csv, err := fetchCSV(crClient, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseSucceeded)) if err != nil { return err } @@ -1391,7 +1392,7 @@ var _ = Describe("Subscription", func() { require.NoError(GinkgoT(), err) require.NotNil(GinkgoT(), subscription) - csv, err := fetchCSV(crClient, subscription.Status.CurrentCSV, generatedNamespace.GetName(), buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseInstalling, operatorsv1alpha1.CSVPhaseSucceeded)) + csv, err := fetchCSV(crClient, generatedNamespace.GetName(), subscription.Status.CurrentCSV, buildCSVConditionChecker(operatorsv1alpha1.CSVPhaseInstalling, operatorsv1alpha1.CSVPhaseSucceeded)) require.NoError(GinkgoT(), err) Eventually(func() error { @@ -1676,7 +1677,7 @@ var _ = Describe("Subscription", func() { Expect(err).ToNot(HaveOccurred()) Expect(subscription).ToNot(BeNil()) - _, err = fetchCSV(crClient, mainCSVName, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), mainCSVName, csvSucceededChecker) Expect(err).ToNot(HaveOccurred()) }) @@ -1764,7 +1765,7 @@ var _ = Describe("Subscription", func() { Expect(err).ToNot(HaveOccurred()) Expect(subscription).ToNot(BeNil()) - _, err = fetchCSV(crClient, mainCSVName, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), mainCSVName, csvSucceededChecker) Expect(err).ToNot(HaveOccurred()) }) @@ -1858,7 +1859,7 @@ var _ = Describe("Subscription", func() { Expect(err).ToNot(HaveOccurred()) Expect(subscription).ToNot(BeNil()) - _, err = fetchCSV(crClient, mainCSVName, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), mainCSVName, csvSucceededChecker) Expect(err).ToNot(HaveOccurred()) }) @@ -1952,7 +1953,7 @@ var _ = Describe("Subscription", func() { Expect(err).ToNot(HaveOccurred()) Expect(subscription).ToNot(BeNil()) - _, err = fetchCSV(crClient, mainCSVName, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), mainCSVName, csvSucceededChecker) Expect(err).ToNot(HaveOccurred()) }) @@ -2066,9 +2067,9 @@ var _ = Describe("Subscription", func() { _, err = fetchInstallPlanWithNamespace(GinkgoT(), crClient, subscription.Status.InstallPlanRef.Name, generatedNamespace.GetName(), buildInstallPlanPhaseCheckFunc(operatorsv1alpha1.InstallPlanPhaseComplete)) require.NoError(GinkgoT(), err) // Fetch CSVs A and B - _, err = fetchCSV(crClient, csvA.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), csvA.Name, csvSucceededChecker) require.NoError(GinkgoT(), err) - _, err = fetchCSV(crClient, csvB.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), csvB.Name, csvSucceededChecker) require.NoError(GinkgoT(), err) // Update PackageManifest @@ -2096,7 +2097,7 @@ var _ = Describe("Subscription", func() { _, err = crClient.OperatorsV1alpha1().ClusterServiceVersions(generatedNamespace.GetName()).Get(context.Background(), csvNewA.Name, metav1.GetOptions{}) require.Error(GinkgoT(), err) // Ensure csvA still exists - _, err = fetchCSV(crClient, csvA.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), csvA.Name, csvSucceededChecker) require.NoError(GinkgoT(), err) // Update packagemanifest again @@ -2121,10 +2122,10 @@ var _ = Describe("Subscription", func() { _, err = fetchSubscription(crClient, generatedNamespace.GetName(), subscriptionName, subscriptionHasInstallPlanDifferentChecker(subscription.Status.InstallPlanRef.Name)) require.NoError(GinkgoT(), err) // Ensure csvNewA is installed - _, err = fetchCSV(crClient, csvNewA.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), csvNewA.Name, csvSucceededChecker) require.NoError(GinkgoT(), err) // Ensure csvNewB is installed - _, err = fetchCSV(crClient, csvNewB.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crClient, generatedNamespace.GetName(), csvNewB.Name, csvSucceededChecker) require.NoError(GinkgoT(), err) }) @@ -2632,14 +2633,27 @@ func subscriptionHasCurrentCSV(currentCSV string) subscriptionStateChecker { } func subscriptionHasCondition(condType operatorsv1alpha1.SubscriptionConditionType, status corev1.ConditionStatus, reason, message string) subscriptionStateChecker { + var lastCond operatorsv1alpha1.SubscriptionCondition + lastTime := time.Now() + // if status/reason/message meet expectations, then subscription state is considered met/true + // IFF this is the result of a recent change of status/reason/message + // else, cache the current status/reason/message for next loop/comparison return func(subscription *operatorsv1alpha1.Subscription) bool { cond := subscription.Status.GetCondition(condType) if cond.Status == status && cond.Reason == reason && cond.Message == message { - fmt.Printf("subscription condition met %v\n", cond) + if lastCond.Status != cond.Status && lastCond.Reason != cond.Reason && lastCond.Message == cond.Message { + GinkgoT().Logf("waited %s subscription condition met %v\n", time.Since(lastTime), cond) + lastTime = time.Now() + lastCond = cond + } return true } - fmt.Printf("subscription condition not met: %v\n", cond) + if lastCond.Status != cond.Status && lastCond.Reason != cond.Reason && lastCond.Message == cond.Message { + GinkgoT().Logf("waited %s subscription condition not met: %v\n", time.Since(lastTime), cond) + lastTime = time.Now() + lastCond = cond + } return false } } @@ -2932,6 +2946,8 @@ func updateInternalCatalog(t GinkgoTInterface, c operatorclient.ClientInterface, require.NoError(t, err) // wait for catalog to update + var lastState string + lastTime := time.Now() _, err = fetchCatalogSourceOnStatus(crc, catalogSourceName, namespace, func(catalog *operatorsv1alpha1.CatalogSource) bool { before := fetchedInitialCatalog.Status.ConfigMapResource after := catalog.Status.ConfigMapResource @@ -2940,7 +2956,11 @@ func updateInternalCatalog(t GinkgoTInterface, c operatorclient.ClientInterface, fmt.Println("catalog updated") return true } - fmt.Printf("waiting for catalog pod %v to be available (after catalog update) - %s\n", catalog.GetName(), catalog.Status.GRPCConnectionState.LastObservedState) + if catalog.Status.GRPCConnectionState.LastObservedState != lastState { + fmt.Printf("waited %s for catalog pod %v to be available (after catalog update) - %s\n", time.Since(lastTime), catalog.GetName(), catalog.Status.GRPCConnectionState.LastObservedState) + lastState = catalog.Status.GRPCConnectionState.LastObservedState + lastTime = time.Now() + } return false }) require.NoError(t, err) diff --git a/staging/operator-lifecycle-manager/test/e2e/webhook_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/webhook_e2e_test.go index df2793cdea..71423796ca 100644 --- a/staging/operator-lifecycle-manager/test/e2e/webhook_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/webhook_e2e_test.go @@ -2,7 +2,14 @@ package e2e import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" "fmt" + "math" + "math/big" "strings" "time" @@ -21,6 +28,7 @@ import ( v1 "github.com/operator-framework/api/pkg/operators/v1" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned" + "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/certs" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" ) @@ -97,7 +105,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) actualWebhook, err := getWebhookWithGenerateName(c, webhook.GenerateName) @@ -139,7 +147,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) actualWebhook, err := getWebhookWithGenerateName(c, webhook.GenerateName) @@ -194,7 +202,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) // Get the existing secret @@ -216,7 +224,7 @@ var _ = Describe("CSVs with a Webhook", func() { }).Should(BeTrue(), "Unable to set CSV phase to Pending") // Wait for webhook-operator to succeed - _, err = awaitCSV(crc, generatedNamespace.GetName(), csv.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) // Get the updated secret @@ -243,7 +251,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvFailedChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvFailedChecker) Expect(err).Should(BeNil()) }) It("Fails if the webhooks intercepts all resources", func() { @@ -273,7 +281,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - failedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvFailedChecker) + failedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvFailedChecker) Expect(err).Should(BeNil()) Expect(failedCSV.Status.Message).Should(Equal("webhook rules cannot include all groups")) }) @@ -304,7 +312,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - failedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvFailedChecker) + failedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvFailedChecker) Expect(err).Should(BeNil()) Expect(failedCSV.Status.Message).Should(Equal("webhook rules cannot include the OLM group")) }) @@ -335,7 +343,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - failedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvFailedChecker) + failedCSV, err := fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvFailedChecker) Expect(err).Should(BeNil()) Expect(failedCSV.Status.Message).Should(Equal("webhook rules cannot include MutatingWebhookConfiguration or ValidatingWebhookConfiguration resources")) }) @@ -368,7 +376,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) }) It("Can be installed and upgraded successfully", func() { @@ -400,7 +408,7 @@ var _ = Describe("CSVs with a Webhook", func() { Expect(err).Should(BeNil()) // cleanup by upgrade - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) _, err = getWebhookWithGenerateName(c, webhook.GenerateName) @@ -415,16 +423,15 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.GetName(), generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.GetName(), csvSucceededChecker) Expect(err).Should(BeNil()) _, err = getWebhookWithGenerateName(c, webhook.GenerateName) Expect(err).Should(BeNil()) // Make sure old resources are cleaned up. - Eventually(func() bool { - return csvExists(generatedNamespace.GetName(), crc, csv.Spec.Replaces) - }).Should(BeFalse()) + err = waitForCsvToDelete(generatedNamespace.GetName(), csv.Spec.Replaces, crc) + Expect(err).ShouldNot(HaveOccurred()) // Wait until previous webhook is cleaned up Eventually(func() (bool, error) { @@ -456,7 +463,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - fetchedCSV, err := fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) actualWebhook, err := getWebhookWithGenerateName(c, webhook.GenerateName) @@ -472,15 +479,19 @@ var _ = Describe("CSVs with a Webhook", func() { oldCAAnnotation, ok := dep.Spec.Template.GetAnnotations()[install.OLMCAHashAnnotationKey] Expect(ok).Should(BeTrue()) - // Induce a cert rotation - Eventually(Apply(fetchedCSV, func(csv *operatorsv1alpha1.ClusterServiceVersion) error { - now := metav1.Now() - csv.Status.CertsLastUpdated = &now - csv.Status.CertsRotateAt = &now + caSecret, err := c.KubernetesInterface().CoreV1().Secrets(generatedNamespace.GetName()).Get(context.TODO(), install.SecretName(install.ServiceName(dep.Name)), metav1.GetOptions{}) + Expect(err).Should(BeNil()) + + caPEM, certPEM, privPEM := generateExpiredCerts(install.HostnamesForService(install.ServiceName(dep.Name), generatedNamespace.GetName())) + By(`Induce a cert rotation`) + Eventually(Apply(caSecret, func(caSecret *corev1.Secret) error { + caSecret.Data[install.OLMCAPEMKey] = caPEM + caSecret.Data["tls.crt"] = certPEM + caSecret.Data["tls.key"] = privPEM return nil })).Should(Succeed()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, func(csv *operatorsv1alpha1.ClusterServiceVersion) bool { // Should create deployment dep, err = c.GetDeployment(generatedNamespace.GetName(), csv.Spec.WebhookDefinitions[0].DeploymentName) if err != nil { @@ -542,7 +553,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) actualWebhook, err := getWebhookWithGenerateName(c, webhook.GenerateName) Expect(err).Should(BeNil()) @@ -606,7 +617,7 @@ var _ = Describe("CSVs with a Webhook", func() { defer cleanupCSV() Eventually(func() (err error) { - _, err = fetchCSV(crc, csv.Name, namespace1.Name, csvSucceededChecker) + _, err = fetchCSV(crc, namespace1.Name, csv.Name, csvSucceededChecker) return }).Should(Succeed()) @@ -618,7 +629,7 @@ var _ = Describe("CSVs with a Webhook", func() { defer cleanupCSV() Eventually(func() (err error) { - _, err = fetchCSV(crc, csv.Name, namespace2.Name, csvSucceededChecker) + _, err = fetchCSV(crc, namespace2.Name, csv.Name, csvSucceededChecker) return }).Should(Succeed()) @@ -690,8 +701,11 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupSubscription := createSubscriptionForCatalog(crc, source.GetNamespace(), subscriptionName, source.GetName(), packageName, channelName, "", operatorsv1alpha1.ApprovalAutomatic) defer cleanupSubscription() + _, err = fetchSubscription(crc, source.GetNamespace(), subscriptionName, subscriptionHasInstallPlanChecker) + require.NoError(GinkgoT(), err) + // Wait for webhook-operator v2 csv to succeed - csv, err := awaitCSV(crc, source.GetNamespace(), csvName, csvSucceededChecker) + csv, err := fetchCSV(crc, source.GetNamespace(), csvName, csvSucceededChecker) require.NoError(GinkgoT(), err) cleanupCSV = buildCSVCleanupFunc(c, crc, *csv, source.GetNamespace(), true, true) @@ -852,7 +866,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) actualWebhook, err := getWebhookWithGenerateName(c, webhook.GenerateName) Expect(err).Should(BeNil()) @@ -915,7 +929,7 @@ var _ = Describe("CSVs with a Webhook", func() { cleanupCSV, err = createCSV(c, crc, csv, generatedNamespace.GetName(), false, false) Expect(err).Should(BeNil()) - _, err = fetchCSV(crc, csv.Name, generatedNamespace.GetName(), csvSucceededChecker) + _, err = fetchCSV(crc, generatedNamespace.GetName(), csv.Name, csvSucceededChecker) Expect(err).Should(BeNil()) actualWebhook, err := getWebhookWithGenerateName(c, webhook.GenerateName) Expect(err).Should(BeNil()) @@ -1167,3 +1181,70 @@ func newV1CRD(plural string) apiextensionsv1.CustomResourceDefinition { return crd } + +func generateExpiredCerts(hosts []string) ([]byte, []byte, []byte) { + caSerial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64)) + Expect(err).Should(BeNil()) + + caDetails := &x509.Certificate{ + SerialNumber: caSerial, + Subject: pkix.Name{ + CommonName: fmt.Sprintf("olm-selfsigned-%x", caSerial), + Organization: []string{install.Organization}, + }, + NotBefore: time.Now().Add(-5 * time.Minute), + NotAfter: time.Now().Add(-5 * time.Minute), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign, + BasicConstraintsValid: true, + } + + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).Should(BeNil()) + + caRaw, err := x509.CreateCertificate(rand.Reader, caDetails, caDetails, &caKey.PublicKey, caKey) + Expect(err).Should(BeNil()) + + caCert, err := x509.ParseCertificate(caRaw) + Expect(err).Should(BeNil()) + + caPEM, _, err := (&certs.KeyPair{ + Cert: caCert, + Priv: caKey, + }).ToPEM() + Expect(err).Should(BeNil()) + + serial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64)) + Expect(err).Should(BeNil()) + + certDetails := &x509.Certificate{ + SerialNumber: serial, + Subject: pkix.Name{ + CommonName: hosts[0], + Organization: []string{install.Organization}, + }, + NotBefore: time.Now(), + NotAfter: time.Now(), + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + DNSNames: hosts, + } + + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).Should(BeNil()) + + publicKey := &privateKey.PublicKey + certRaw, err := x509.CreateCertificate(rand.Reader, certDetails, caCert, publicKey, caKey) + Expect(err).Should(BeNil()) + + cert, err := x509.ParseCertificate(certRaw) + Expect(err).Should(BeNil()) + + certPEM, privPEM, err := (&certs.KeyPair{ + Cert: cert, + Priv: privateKey, + }).ToPEM() + Expect(err).Should(BeNil()) + + return caPEM, certPEM, privPEM +} 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 f48e62b771..2822b43063 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 @@ -12,6 +12,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/certs" @@ -155,6 +156,14 @@ func ServiceName(deploymentName string) string { return deploymentName + "-service" } +func HostnamesForService(serviceName, namespace string) []string { + return []string{ + fmt.Sprintf("%s.%s", serviceName, namespace), + fmt.Sprintf("%s.%s.svc", serviceName, namespace), + fmt.Sprintf("%s.%s.svc.cluster.local", serviceName, namespace), + } +} + func (i *StrategyDeploymentInstaller) getCertResources() []certResource { return append(i.apiServiceDescriptions, i.webhookDescriptions...) } @@ -221,15 +230,74 @@ func (i *StrategyDeploymentInstaller) CertsRotated() bool { return i.certificatesRotated } -func ShouldRotateCerts(csv *v1alpha1.ClusterServiceVersion) bool { +// shouldRotateCerts indicates whether an apiService cert should be rotated due to being +// malformed, invalid, expired, inactive or within a specific freshness interval (DefaultCertMinFresh) before expiry. +func shouldRotateCerts(certSecret *corev1.Secret, hosts []string) bool { now := metav1.Now() - if !csv.Status.CertsRotateAt.IsZero() && csv.Status.CertsRotateAt.Before(&now) { + caPEM, ok := certSecret.Data[OLMCAPEMKey] + if !ok { + // missing CA cert in secret + return true + } + certPEM, ok := certSecret.Data["tls.crt"] + if !ok { + // missing cert in secret + return true + } + + ca, err := certs.PEMToCert(caPEM) + if err != nil { + // malformed CA cert + return true + } + cert, err := certs.PEMToCert(certPEM) + if err != nil { + // malformed cert return true } + // check for cert freshness + if !certs.Active(ca) || now.Time.After(CalculateCertRotatesAt(ca.NotAfter)) || + !certs.Active(cert) || now.Time.After(CalculateCertRotatesAt(cert.NotAfter)) { + return true + } + + // Check validity of serving cert and if serving cert is trusted by the CA + for _, host := range hosts { + if err := certs.VerifyCert(ca, cert, host); err != nil { + return true + } + } return false } +func (i *StrategyDeploymentInstaller) ShouldRotateCerts(s Strategy) (bool, error) { + strategy, ok := s.(*v1alpha1.StrategyDetailsDeployment) + if !ok { + return false, fmt.Errorf("failed to install %s strategy with deployment installer: unsupported deployment install strategy", strategy.GetStrategyName()) + } + + hasCerts := sets.NewString() + for _, c := range i.getCertResources() { + hasCerts.Insert(c.getDeploymentName()) + } + for _, sddSpec := range strategy.DeploymentSpecs { + if hasCerts.Has(sddSpec.Name) { + certSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(SecretName(ServiceName(sddSpec.Name))) + if err == nil { + if shouldRotateCerts(certSecret, HostnamesForService(ServiceName(sddSpec.Name), i.owner.GetNamespace())) { + return true, nil + } + } else if apierrors.IsNotFound(err) { + return true, nil + } else { + return false, err + } + } + } + return false, nil +} + func CalculateCertExpiration(startingFrom time.Time) time.Time { return startingFrom.Add(DefaultCertValidFor) } @@ -248,7 +316,8 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo Selector: depSpec.Selector.MatchLabels, }, } - service.SetName(ServiceName(deploymentName)) + serviceName := ServiceName(deploymentName) + service.SetName(serviceName) service.SetNamespace(i.owner.GetNamespace()) ownerutil.AddNonBlockingOwner(service, i.owner) @@ -274,10 +343,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo } // Create signed serving cert - hosts := []string{ - fmt.Sprintf("%s.%s", service.GetName(), i.owner.GetNamespace()), - fmt.Sprintf("%s.%s.svc", service.GetName(), i.owner.GetNamespace()), - } + hosts := HostnamesForService(serviceName, i.owner.GetNamespace()) servingPair, err := certGenerator.Generate(expiration, Organization, ca, hosts) if err != nil { logger.Warnf("could not generate signed certs for hosts %v", hosts) @@ -321,10 +387,10 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo // Attempt an update // TODO: Check that the secret was not modified - if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) { + if !shouldRotateCerts(existingSecret, HostnamesForService(serviceName, i.owner.GetNamespace())) { logger.Warnf("reusing existing cert %s", secret.GetName()) secret = existingSecret - caPEM = existingCAPEM + caPEM = existingSecret.Data[OLMCAPEMKey] caHash = certs.PEMSHA256(caPEM) } else { if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil { 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 206ce9e384..3e762fefe4 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 @@ -21,6 +21,7 @@ type Strategy interface { type StrategyInstaller interface { Install(strategy Strategy) error CheckInstalled(strategy Strategy) (bool, error) + ShouldRotateCerts(strategy Strategy) (bool, error) CertsRotateAt() time.Time CertsRotated() bool } @@ -79,3 +80,7 @@ func (i *NullStrategyInstaller) CertsRotateAt() time.Time { func (i *NullStrategyInstaller) CertsRotated() bool { return false } + +func (i *NullStrategyInstaller) ShouldRotateCerts(s Strategy) (bool, error) { + return false, nil +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go index c71b5594f3..89bc4a4a00 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/apiservices.go @@ -78,7 +78,7 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, } serviceName := install.ServiceName(desc.DeploymentName) - service, err := a.lister.CoreV1().ServiceLister().Services(csv.GetNamespace()).Get(serviceName) + _, err = a.lister.CoreV1().ServiceLister().Services(csv.GetNamespace()).Get(serviceName) if err != nil { logger.WithField("service", serviceName).Warnf("could not retrieve generated Service") errs = append(errs, err) @@ -94,17 +94,12 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, // Check if CA is Active caBundle := apiService.Spec.CABundle - ca, err := certs.PEMToCert(caBundle) + _, err = certs.PEMToCert(caBundle) if err != nil { logger.Warnf("could not convert APIService CA bundle to x509 cert") errs = append(errs, err) continue } - if !certs.Active(ca) { - logger.Warnf("CA cert not active") - errs = append(errs, fmt.Errorf("found the CA cert is not active")) - continue - } // Check if serving cert is active secretName := install.SecretName(serviceName) @@ -114,17 +109,12 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, errs = append(errs, err) continue } - cert, err := certs.PEMToCert(secret.Data["tls.crt"]) + _, err = certs.PEMToCert(secret.Data["tls.crt"]) if err != nil { logger.Warnf("could not convert serving cert to x509 cert") errs = append(errs, err) continue } - if !certs.Active(cert) { - logger.Warnf("serving cert not active") - errs = append(errs, fmt.Errorf("found the serving cert not active")) - continue - } // Check if CA hash matches expected caHash := hashFunc(caBundle) @@ -134,18 +124,6 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion, continue } - // Check if serving cert is trusted by the CA - hosts := []string{ - fmt.Sprintf("%s.%s", service.GetName(), csv.GetNamespace()), - fmt.Sprintf("%s.%s.svc", service.GetName(), csv.GetNamespace()), - } - for _, host := range hosts { - if err := certs.VerifyCert(ca, cert, host); err != nil { - errs = append(errs, fmt.Errorf("could not verify cert: %s", err.Error())) - continue - } - } - // Ensure the existing Deployment has a matching CA hash annotation deployment, err := a.lister.AppsV1().DeploymentLister().Deployments(csv.GetNamespace()).Get(desc.DeploymentName) if apierrors.IsNotFound(err) || err != nil { 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 6964fe9917..414c755920 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 @@ -2104,7 +2104,10 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v } // Check if it's time to refresh owned APIService certs - if install.ShouldRotateCerts(out) { + if shouldRotate, err := installer.ShouldRotateCerts(strategy); err != nil { + logger.WithError(err).Info("cert validity check") + return + } else if shouldRotate { logger.Debug("CSV owns resources that require a cert refresh") out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsCertRotation, "CSV owns resources that require a cert refresh", now, a.recorder) return @@ -2214,7 +2217,10 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v } // Check if it's time to refresh owned APIService certs - if install.ShouldRotateCerts(out) { + if shouldRotate, err := installer.ShouldRotateCerts(strategy); err != nil { + logger.WithError(err).Info("cert validity check") + return + } else if shouldRotate { logger.Debug("CSV owns resources that require a cert refresh") out.SetPhaseWithEvent(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonNeedsCertRotation, "owned APIServices need cert refresh", now, a.recorder) return