Skip to content
Permalink

Comparing changes

This is a direct comparison between two commits made in this repository or its related repositories. View the default comparison for this range or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: operator-framework/operator-lifecycle-manager
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 7df68ff85bbc687c43ed0dc1b51618483a9456ef
Choose a base ref
..
head repository: operator-framework/operator-lifecycle-manager
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 2ecbf6c7a202108a83481f6d5b7d188b782f41a8
Choose a head ref
Showing with 494 additions and 147 deletions.
  1. +5 −10 .github/workflows/goreleaser.yaml
  2. +1 −2 Makefile
  3. +4 −4 go.mod
  4. +8 −8 go.sum
  5. +13 −1 pkg/controller/bundle/bundle_unpacker.go
  6. +27 −0 pkg/controller/bundle/bundle_unpacker_test.go
  7. +73 −8 pkg/controller/install/certresources.go
  8. +5 −0 pkg/controller/install/resolver.go
  9. +2 −24 pkg/controller/operators/olm/apiservices.go
  10. +8 −2 pkg/controller/operators/olm/operator.go
  11. +22 −18 pkg/controller/operators/olm/operator_test.go
  12. +79 −0 pkg/fakes/fake_strategy_installer.go
  13. +11 −7 test/e2e/csv_e2e_test.go
  14. +84 −5 test/e2e/webhook_e2e_test.go
  15. +4 −2 vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go
  16. +2 −1 vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go
  17. +36 −28 vendor/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go
  18. +3 −1 vendor/k8s.io/apiserver/pkg/admission/plugin/cel/interface.go
  19. +21 −6 vendor/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go
  20. +4 −1 vendor/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking.go
  21. +12 −0 vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go
  22. +4 −2 vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go
  23. +4 −2 vendor/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go
  24. +36 −11 vendor/k8s.io/apiserver/pkg/cel/environment/base.go
  25. +22 −0 vendor/k8s.io/apiserver/pkg/features/kube_features.go
  26. +4 −4 vendor/modules.txt
15 changes: 5 additions & 10 deletions .github/workflows/goreleaser.yaml
Original file line number Diff line number Diff line change
@@ -19,18 +19,16 @@ jobs:
go-version-file: "go.mod"

- name: Get the image tag
if: startsWith(github.ref, 'refs/tags')
run: |
# Source: https://github.community/t/how-to-get-just-the-tag-name/16241/32
if [[ $GITHUB_REF == refs/tags/* ]]; then
echo IMAGE_TAG="${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV
else
echo IMAGE_TAG="snapshot" >> $GITHUB_ENV
fi
# TODO dtfranz
# This action uses node12 and the source repo is archived;
# we should remove it or find a suitable replacement before it becomes a blocker.
- name: Create a draft release
uses: actions/create-release@v1
uses: softprops/action-gh-release@v2
id: release
if: startsWith(github.ref, 'refs/tags')
env:
@@ -42,7 +40,6 @@ jobs:

- name: Set up QEMU
uses: docker/setup-qemu-action@v3
if: startsWith(github.ref, 'refs/tags')

- name: Docker Login
uses: docker/login-action@v3
@@ -54,17 +51,15 @@ jobs:

- name: Run GoReleaser
uses: goreleaser/goreleaser-action@v6
if: startsWith(github.ref, 'refs/tags')
with:
version: 0.177.0
args: release --rm-dist
args: release --rm-dist ${{ github.event_name == 'pull_request' && '--snapshot' || '' }}
env:
GITHUB_TOKEN: ${{ github.token }}
IMAGE_REPO: quay.io/operator-framework/olm
PKG: github.com/operator-framework/operator-lifecycle-manager

- name: Generate quickstart release manifests
if: startsWith(github.ref, 'refs/tags')
run: make release ver=${{ env.IMAGE_TAG }} IMAGE_REPO=quay.io/operator-framework/olm

- name: Update release artifacts with rendered Kubernetes release manifests
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -263,8 +263,7 @@ E2E_TIMEOUT ?= 90m
E2E_TEST_NS ?= operators
E2E_INSTALL_NS ?= operator-lifecycle-manager
E2E_CATALOG_NS ?= $(E2E_INSTALL_NS)
E2E_FLAKE_ATTEMPTS ?= 1
GINKGO_OPTS += -v -randomize-suites -race -trace --show-node-events -flake-attempts='$(E2E_FLAKE_ATTEMPTS)' $(if $(E2E_SEED),-seed '$(E2E_SEED)' ) $(if $(TEST),-focus '$(TEST)', ) $(if $(SKIP), -skip '$(SKIP)')
GINKGO_OPTS += -v -randomize-suites -race -trace $(if $(E2E_FLAKE_ATTEMPTS),--flake-attempts='$(E2E_FLAKE_ATTEMPTS)') $(if $(E2E_SEED),-seed '$(E2E_SEED)') $(if $(TEST),-focus '$(TEST)',) $(if $(SKIP), -skip '$(SKIP)')

.PHONY: e2e
e2e: #HELP Run e2e tests against a cluster running OLM (params: $E2E_TEST_NS (operator), $E2E_INSTALL_NS (operator-lifecycle-manager), $E2E_CATALOG_NS (operator-lifecycle-manager), $E2E_TIMEOUT (90m), $E2E_FLAKE_ATTEMPTS (1), $TEST(undefined))
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
@@ -40,14 +40,14 @@ require (
google.golang.org/grpc v1.64.0
gopkg.in/yaml.v2 v2.4.0
k8s.io/api v0.30.2
k8s.io/apiextensions-apiserver v0.30.1
k8s.io/apiextensions-apiserver v0.30.2
k8s.io/apimachinery v0.30.2
k8s.io/apiserver v0.30.1
k8s.io/apiserver v0.30.2
k8s.io/client-go v0.30.2
k8s.io/code-generator v0.30.2
k8s.io/component-base v0.30.2
k8s.io/klog v1.0.0
k8s.io/kube-aggregator v0.30.1
k8s.io/kube-aggregator v0.30.2
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340
k8s.io/utils v0.0.0-20240102154912-e7106e64919e
sigs.k8s.io/controller-runtime v0.18.4
@@ -168,7 +168,7 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/gengo/v2 v2.0.0-20240228010128-51d4e06bde70 // indirect
k8s.io/klog/v2 v2.120.1 // indirect
k8s.io/kms v0.30.1 // indirect
k8s.io/kms v0.30.2 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
16 changes: 8 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
@@ -2818,12 +2818,12 @@ honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
honnef.co/go/tools v0.1.3/go.mod h1:NgwopIslSNH47DimFoV78dnkksY2EFtX0ajyb3K/las=
k8s.io/api v0.30.2 h1:+ZhRj+28QT4UOH+BKznu4CBgPWgkXO7XAvMcMl0qKvI=
k8s.io/api v0.30.2/go.mod h1:ULg5g9JvOev2dG0u2hig4Z7tQ2hHIuS+m8MNZ+X6EmI=
k8s.io/apiextensions-apiserver v0.30.1 h1:4fAJZ9985BmpJG6PkoxVRpXv9vmPUOVzl614xarePws=
k8s.io/apiextensions-apiserver v0.30.1/go.mod h1:R4GuSrlhgq43oRY9sF2IToFh7PVlF1JjfWdoG3pixk4=
k8s.io/apiextensions-apiserver v0.30.2 h1:l7Eue2t6QiLHErfn2vwK4KgF4NeDgjQkCXtEbOocKIE=
k8s.io/apiextensions-apiserver v0.30.2/go.mod h1:lsJFLYyK40iguuinsb3nt+Sj6CmodSI4ACDLep1rgjw=
k8s.io/apimachinery v0.30.2 h1:fEMcnBj6qkzzPGSVsAZtQThU62SmQ4ZymlXRC5yFSCg=
k8s.io/apimachinery v0.30.2/go.mod h1:iexa2somDaxdnj7bha06bhb43Zpa6eWH8N8dbqVjTUc=
k8s.io/apiserver v0.30.1 h1:BEWEe8bzS12nMtDKXzCF5Q5ovp6LjjYkSp8qOPk8LZ8=
k8s.io/apiserver v0.30.1/go.mod h1:i87ZnQ+/PGAmSbD/iEKM68bm1D5reX8fO4Ito4B01mo=
k8s.io/apiserver v0.30.2 h1:ACouHiYl1yFI2VFI3YGM+lvxgy6ir4yK2oLOsLI1/tw=
k8s.io/apiserver v0.30.2/go.mod h1:BOTdFBIch9Sv0ypSEcUR6ew/NUFGocRFNl72Ra7wTm8=
k8s.io/client-go v0.30.2 h1:sBIVJdojUNPDU/jObC+18tXWcTJVcwyqS9diGdWHk50=
k8s.io/client-go v0.30.2/go.mod h1:JglKSWULm9xlJLx4KCkfLLQ7XwtlbflV6uFFSHTMgVs=
k8s.io/code-generator v0.30.2 h1:ZY1+aGkqZVwKIyGsOzquaeZ5rSfE6wZHur8z3jQAaiw=
@@ -2836,10 +2836,10 @@ k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8=
k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw=
k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kms v0.30.1 h1:gEIbEeCbFiaN2tNfp/EUhFdGr5/CSj8Eyq6Mkr7cCiY=
k8s.io/kms v0.30.1/go.mod h1:GrMurD0qk3G4yNgGcsCEmepqf9KyyIrTXYR2lyUOJC4=
k8s.io/kube-aggregator v0.30.1 h1:ymR2BsxDacTKwzKTuNhGZttuk009c+oZbSeD+IPX5q4=
k8s.io/kube-aggregator v0.30.1/go.mod h1:SFbqWsM6ea8dHd3mPLsZFzJHbjBOS5ykIgJh4znZ5iQ=
k8s.io/kms v0.30.2 h1:VSZILO/tkzrz5Tu2j+yFQZ2Dc5JerQZX2GqhFJbQrfw=
k8s.io/kms v0.30.2/go.mod h1:GrMurD0qk3G4yNgGcsCEmepqf9KyyIrTXYR2lyUOJC4=
k8s.io/kube-aggregator v0.30.2 h1:0+yk/ED6foCprY8VmkDPUhngjaAPKsNTXB/UrtvbIz0=
k8s.io/kube-aggregator v0.30.2/go.mod h1:EhqCfDdxysNWXk1wRL9SEHAdo1DKl6EULQagztkBcXE=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCfRziVtos3ofG/sQ=
14 changes: 13 additions & 1 deletion pkg/controller/bundle/bundle_unpacker.go
Original file line number Diff line number Diff line change
@@ -863,14 +863,26 @@ func sortUnpackJobs(jobs []*batchv1.Job, maxRetainedJobs int) (latest *batchv1.J
// sort jobs so that latest job is first
// with preference for non-failed jobs
sort.Slice(jobs, func(i, j int) bool {
if jobs[i] == nil || jobs[j] == nil {
return jobs[i] != nil
}
condI, failedI := getCondition(jobs[i], batchv1.JobFailed)
condJ, failedJ := getCondition(jobs[j], batchv1.JobFailed)
if failedI != failedJ {
return !failedI // non-failed job goes first
}
return condI.LastTransitionTime.After(condJ.LastTransitionTime.Time)
})
if jobs[0] == nil {
// all nil jobs
return
}
latest = jobs[0]
nilJobsIndex := len(jobs) - 1
for ; nilJobsIndex >= 0 && jobs[nilJobsIndex] == nil; nilJobsIndex-- {
}

jobs = jobs[:nilJobsIndex+1] // exclude nil jobs from list of jobs to delete
if len(jobs) <= maxRetainedJobs {
return
}
@@ -880,7 +892,7 @@ func sortUnpackJobs(jobs []*batchv1.Job, maxRetainedJobs int) (latest *batchv1.J
}

// cleanup old failed jobs, n-1 recent jobs and the oldest job
for i := 0; i < maxRetainedJobs && i+maxRetainedJobs < len(jobs); i++ {
for i := 0; i < maxRetainedJobs && i+maxRetainedJobs < len(jobs)-1; i++ {
toDelete = append(toDelete, jobs[maxRetainedJobs+i])
}

27 changes: 27 additions & 0 deletions pkg/controller/bundle/bundle_unpacker_test.go
Original file line number Diff line number Diff line change
@@ -1997,6 +1997,15 @@ func TestSortUnpackJobs(t *testing.T) {
},
}
}
nilConditionJob := &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "nc",
Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue, bundleUnpackRefLabel: "test"},
},
Status: batchv1.JobStatus{
Conditions: nil,
},
}
failedJobs := []*batchv1.Job{
testJob("f-1", true, 1),
testJob("f-2", true, 2),
@@ -2028,6 +2037,24 @@ func TestSortUnpackJobs(t *testing.T) {
}, {
name: "empty job list",
maxRetained: 1,
}, {
name: "nil job in list",
maxRetained: 1,
jobs: []*batchv1.Job{
failedJobs[2],
nil,
failedJobs[1],
},
expectedLatest: failedJobs[2],
}, {
name: "nil condition",
maxRetained: 3,
jobs: []*batchv1.Job{
failedJobs[2],
nilConditionJob,
failedJobs[1],
},
expectedLatest: nilConditionJob,
}, {
name: "retain oldest",
maxRetained: 1,
81 changes: 73 additions & 8 deletions pkg/controller/install/certresources.go
Original file line number Diff line number Diff line change
@@ -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"
corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
rbacv1ac "k8s.io/client-go/applyconfigurations/rbac/v1"

@@ -157,6 +158,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...)
}
@@ -223,15 +232,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.New[string]()
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)
}
@@ -267,10 +335,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
}

// Create signed serving cert
hosts := []string{
fmt.Sprintf("%s.%s", serviceName, i.owner.GetNamespace()),
fmt.Sprintf("%s.%s.svc", serviceName, 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)
@@ -314,10 +379,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 {
5 changes: 5 additions & 0 deletions pkg/controller/install/resolver.go
Original file line number Diff line number Diff line change
@@ -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
}
26 changes: 2 additions & 24 deletions pkg/controller/operators/olm/apiservices.go
Original file line number Diff line number Diff line change
@@ -93,17 +93,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)
@@ -113,17 +108,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)
@@ -133,18 +123,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 {
Loading