NO-ISSUE: Synchronize From Upstream Repositories#1330
Open
openshift-bot wants to merge 7 commits into
Open
Conversation
openshift-bot
added
approved
|
@openshift-bot: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (53)
📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (11)
WalkthroughPackageServer now applies the OpenShift cluster APIServer TLS profile at startup when no TLS minimum version is set. RBAC manifests grant APIServer read access, dependencies are refreshed, and several GitHub Actions workflows switch to checkout v7. ChangesPackageServer TLS fallback and RBAC
Dependency and workflow updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.69.3)Trivy execution failed: 2026-06-25T00:07:00Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: helm scan error: fs filter error: fs filter error: walk error range error: stat .golangci.yml: no such file or directory: range error: stat .golangci.yml: no such file or directory Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
staging/operator-lifecycle-manager/pkg/package-server/server/server.go (1)
391-396: Use non-panicking TLS conversion functions for defensive robustness.While
libcrypto.TLSVersionToNameOrDieandCipherSuitesToNamesOrDiewill panic on invalid input, the values passed to them come fromolmapiserver.GetSecurityProfileConfig, which either returns hardcoded values from predefined profiles or reads from the OpenShift APIServer object (which has upstream validation). In practice, upstream validation prevents invalid TLSSecurityProfile objects from being stored, making a panic unlikely. However, for defensive programming, consider using the non-panicking variants (TLSVersionToName,CipherSuitesToNames) with proper error handling, or add explicit validation that the profile values are supported before conversion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@staging/operator-lifecycle-manager/pkg/package-server/server/server.go` around lines 391 - 396, Replace the panicking versions of the TLS conversion functions with their non-panicking equivalents for better defensive programming. In the block where MinTLSVersion is set, replace the call to libcrypto.TLSVersionToNameOrDie with libcrypto.TLSVersionToName and add proper error handling for the returned error. Similarly, replace libcrypto.CipherSuitesToNamesOrDie with libcrypto.CipherSuitesToNames in the CipherSuites assignment and handle any returned error appropriately, such as logging the error and returning it from the function or using a fallback value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@staging/operator-lifecycle-manager/pkg/package-server/server/server.go`:
- Around line 391-396: Replace the panicking versions of the TLS conversion
functions with their non-panicking equivalents for better defensive programming.
In the block where MinTLSVersion is set, replace the call to
libcrypto.TLSVersionToNameOrDie with libcrypto.TLSVersionToName and add proper
error handling for the returned error. Similarly, replace
libcrypto.CipherSuitesToNamesOrDie with libcrypto.CipherSuitesToNames in the
CipherSuites assignment and handle any returned error appropriately, such as
logging the error and returning it from the function or using a fallback value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9bb5a6f0-3eb8-49c9-bf46-4e397c168c33
⛔ Files ignored due to path filters (5)
go.sumis excluded by!**/*.sumstaging/operator-lifecycle-manager/go.sumis excluded by!**/*.sumvendor/github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/server/server.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/api/validation/objectmeta.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (7)
go.modpkg/manifests/csv.yamlstaging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yamlstaging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yamlstaging/operator-lifecycle-manager/go.modstaging/operator-lifecycle-manager/pkg/package-server/server/server.gostaging/operator-lifecycle-manager/pkg/package-server/server/server_test.go
Contributor
|
/test okd-scos-images |
Contributor
|
/retest |
Contributor
|
/test e2e-upgrade |
Contributor
|
/retest |
5 similar comments
Contributor
|
/retest |
Contributor
|
/retest |
Contributor
|
/retest |
Contributor
|
/retest |
Contributor
|
/retest |
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
04d0e39 to
02b58a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go`:
- Around line 28-35: The NewHashEqualitor function uses panic calls when
hashstructure.Hash errors occur (for both hashA and hashB hash computations),
which will crash the controller in production when reconciling subscriptions.
Replace both panic(err.Error()) calls with a non-panicking fallback behavior
such as returning false, so the equality check can gracefully handle hashing
failures without interrupting controller stability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c29888ed-d6fa-444c-80f7-cc174c3d6850
⛔ Files ignored due to path filters (50)
go.sumis excluded by!**/*.sumstaging/operator-lifecycle-manager/go.sumis excluded by!**/*.sumvendor/github.com/go-viper/mapstructure/v2/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/.envrcis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/decode_hooks.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/devenv.lockis excluded by!**/*.lock,!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/devenv.nixis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/devenv.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/internal/errors/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/internal/errors/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/internal/errors/join_go1_19.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/mapstructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/reflect_go1_19.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-viper/mapstructure/v2/reflect_go1_20.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/hashstructure/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/hashstructure/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/hashstructure/v2/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/hashstructure/v2/hashstructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/hashstructure/v2/include.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/mapstructure/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/mapstructure/decode_hooks.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mitchellh/mapstructure/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/codec/mapstructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/comparison/equal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/server/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/expfmt.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/text_create.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/text_parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/labelset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value_float.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value_histogram.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/api/validation/objectmeta.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (16)
go.modpkg/manifests/csv.yamlstaging/operator-lifecycle-manager/.github/workflows/e2e-tests.ymlstaging/operator-lifecycle-manager/.github/workflows/go-verdiff.yamlstaging/operator-lifecycle-manager/.github/workflows/goreleaser.yamlstaging/operator-lifecycle-manager/.github/workflows/quickstart.ymlstaging/operator-lifecycle-manager/.github/workflows/sanity.yamlstaging/operator-lifecycle-manager/.github/workflows/unit.ymlstaging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yamlstaging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yamlstaging/operator-lifecycle-manager/go.modstaging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.gostaging/operator-lifecycle-manager/pkg/lib/codec/mapstructure.gostaging/operator-lifecycle-manager/pkg/lib/comparison/equal.gostaging/operator-lifecycle-manager/pkg/package-server/server/server.gostaging/operator-lifecycle-manager/pkg/package-server/server/server_test.go
✅ Files skipped from review due to trivial changes (5)
- pkg/manifests/csv.yaml
- staging/operator-lifecycle-manager/.github/workflows/sanity.yaml
- staging/operator-lifecycle-manager/.github/workflows/quickstart.yml
- staging/operator-lifecycle-manager/.github/workflows/unit.yml
- staging/operator-lifecycle-manager/.github/workflows/go-verdiff.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- staging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yaml
- staging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yaml
- staging/operator-lifecycle-manager/pkg/package-server/server/server.go
- staging/operator-lifecycle-manager/pkg/package-server/server/server_test.go
Comment on lines
+28
to
35
| hashA, err := hashstructure.Hash(a, hashstructure.FormatV2, nil) | ||
| if err != nil { | ||
| panic(err.Error()) | ||
| } | ||
|
|
||
| hashB, err := hashstructure.Hash(b, nil) | ||
| hashB, err := hashstructure.Hash(b, hashstructure.FormatV2, nil) | ||
| if err != nil { | ||
| panic(err.Error()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Non-test call sites of NewHashEqualitor:"
rg -nP --type go '\bNewHashEqualitor\s*\(' staging/operator-lifecycle-manager -g '!**/*_test.go'
echo
echo "Test-only call sites of NewHashEqualitor:"
rg -nP --type go '\bNewHashEqualitor\s*\(' staging/operator-lifecycle-manager -g '**/*_test.go'
echo
echo "Current panic usage in equal comparator:"
rg -nP --type go 'panic\s*\(' staging/operator-lifecycle-manager/pkg/lib/comparison/equal.goRepository: openshift/operator-framework-olm
Length of output: 693
🏁 Script executed:
# Examine the usage in state.go line 353
sed -n '340,370p' staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscription/state.go
# Also check if there's any error handling around hashEqual calls
rg -A 5 -B 5 'hashEqual\s*\(' staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscription/state.goRepository: openshift/operator-framework-olm
Length of output: 1770
🏁 Script executed:
# Check what types are being hashed (conditions, which are typically simple structs)
# and verify the impact of hash failures
rg -B 10 'if hashEqual' staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscription/state.go | head -40
# Also check if there are any recent issues or discussions about this
rg -A 3 'NewHashEqualitor' staging/operator-lifecycle-manager/pkg/lib/comparison/equal.goRepository: openshift/operator-framework-olm
Length of output: 1187
🏁 Script executed:
# Get the full NewHashEqualitor function implementation
sed -n '26,45p' staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go
# Check what hashstructure is and where it comes from
rg -B 2 'hashstructure\.Hash' staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go | head -10
# Verify Conditions type to understand what's being hashed
rg 'Status\.Conditions' staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscription/state.go | head -5Repository: openshift/operator-framework-olm
Length of output: 892
🏁 Script executed:
# Check imports to see what hashstructure library is used
head -20 staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go
# Look for any existing error handling patterns in this file
cat staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go
# Check if there are any test cases showing when hashstructure.Hash might fail
rg 'hashstructure\.Hash' staging/operator-lifecycle-manager -A 2 -B 2Repository: openshift/operator-framework-olm
Length of output: 2586
🏁 Script executed:
# Check if there are any comments or documentation about why panic was chosen
rg -B 5 -A 10 'panic' staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go
# Verify this is the only place where NewHashEqualitor is instantiated
rg 'var hashEqual' staging/operator-lifecycle-manager -rRepository: openshift/operator-framework-olm
Length of output: 1191
🏁 Script executed:
# Check if there are any comments about why panic was chosen
sed -n '20,50p' staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go
# Verify this is the only place where NewHashEqualitor is instantiated
rg 'var hashEqual' staging/operator-lifecycle-managerRepository: openshift/operator-framework-olm
Length of output: 913
Remove panics on hash failures in NewHashEqualitor.
This function is used in production code (pkg/controller/operators/catalog/subscription/state.go:353) within subscription reconciliation logic. Panics on hashstructure.Hash errors will crash the controller or interrupt reconciliation. Replace panic calls with a non-panicking fallback (e.g., return false) to preserve controller stability.
Suggested fix
func NewHashEqualitor() EqualFunc {
return func(a, b interface{}) bool {
hashA, err := hashstructure.Hash(a, hashstructure.FormatV2, nil)
if err != nil {
- panic(err.Error())
+ return false
}
hashB, err := hashstructure.Hash(b, hashstructure.FormatV2, nil)
if err != nil {
- panic(err.Error())
+ return false
}
return hashA == hashB
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| hashA, err := hashstructure.Hash(a, hashstructure.FormatV2, nil) | |
| if err != nil { | |
| panic(err.Error()) | |
| } | |
| hashB, err := hashstructure.Hash(b, nil) | |
| hashB, err := hashstructure.Hash(b, hashstructure.FormatV2, nil) | |
| if err != nil { | |
| panic(err.Error()) | |
| func NewHashEqualitor() EqualFunc { | |
| return func(a, b interface{}) bool { | |
| hashA, err := hashstructure.Hash(a, hashstructure.FormatV2, nil) | |
| if err != nil { | |
| return false | |
| } | |
| hashB, err := hashstructure.Hash(b, hashstructure.FormatV2, nil) | |
| if err != nil { | |
| return false | |
| } | |
| return hashA == hashB | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/operator-lifecycle-manager/pkg/lib/comparison/equal.go` around lines
28 - 35, The NewHashEqualitor function uses panic calls when hashstructure.Hash
errors occur (for both hashA and hashB hash computations), which will crash the
controller in production when reconciling subscriptions. Replace both
panic(err.Error()) calls with a non-panicking fallback behavior such as
returning false, so the equality check can gracefully handle hashing failures
without interrupting controller stability.
Contributor
|
/retest |
…#3850) Bumps the k8s-dependencies group with 3 updates in the / directory: [k8s.io/api](https://github.com/kubernetes/api), [k8s.io/apiextensions-apiserver](https://github.com/kubernetes/apiextensions-apiserver) and [k8s.io/kube-aggregator](https://github.com/kubernetes/kube-aggregator). Updates `k8s.io/api` from 0.36.1 to 0.36.2 - [Commits](kubernetes/api@v0.36.1...v0.36.2) Updates `k8s.io/apiextensions-apiserver` from 0.36.1 to 0.36.2 - [Release notes](https://github.com/kubernetes/apiextensions-apiserver/releases) - [Commits](kubernetes/apiextensions-apiserver@v0.36.1...v0.36.2) Updates `k8s.io/apimachinery` from 0.36.1 to 0.36.2 - [Commits](kubernetes/apimachinery@v0.36.1...v0.36.2) Updates `k8s.io/apiserver` from 0.36.1 to 0.36.2 - [Commits](kubernetes/apiserver@v0.36.1...v0.36.2) Updates `k8s.io/client-go` from 0.36.1 to 0.36.2 - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.36.1...v0.36.2) Updates `k8s.io/code-generator` from 0.36.1 to 0.36.2 - [Commits](kubernetes/code-generator@v0.36.1...v0.36.2) Updates `k8s.io/component-base` from 0.36.1 to 0.36.2 - [Commits](kubernetes/component-base@v0.36.1...v0.36.2) Updates `k8s.io/kube-aggregator` from 0.36.1 to 0.36.2 - [Commits](kubernetes/kube-aggregator@v0.36.1...v0.36.2) --- updated-dependencies: - dependency-name: k8s.io/api dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/apiextensions-apiserver dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/apimachinery dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/apiserver dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/client-go dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/code-generator dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/component-base dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies - dependency-name: k8s.io/kube-aggregator dependency-version: 0.36.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-dependencies ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 97cb2b13b52d6cbe6a90e135b9e1bcb2ab95dd1d
…#3849) Read the OpenShift APIServer CR at startup to apply the cluster-wide TLS security profile (min version and cipher suites) to the packageserver's SecureServingOptions when --tls-min-version is not already set via flags. Includes a 30s timeout on the API lookup and fails closed if the profile cannot be applied. Adds RBAC to allow reading apiservers.config.openshift.io. Signed-off-by: Todd Short <tshort@redhat.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 2c667c31ade5ff0a05f009a8529b6d4104b577a7
Bumps [actions/checkout](https://github.com/actions/checkout) from 6 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 35f77a927765fd487b7abcad8c4d1b9e94b536c7
Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.31.0 to 2.32.0. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.31.0...v2.32.0) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-version: 2.32.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 7cdad362a110fad4926917f2b6bee2a70ac5d3a1
Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.68.1 to 0.69.0. - [Release notes](https://github.com/prometheus/common/releases) - [Changelog](https://github.com/prometheus/common/blob/main/CHANGELOG.md) - [Commits](prometheus/common@v0.68.1...v0.69.0) --- updated-dependencies: - dependency-name: github.com/prometheus/common dependency-version: 0.69.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: d5481e01953e2621548c826583d0b476dc5d0554
* chore: upgrade hashstructure and mapstructure to v2 Signed-off-by: Chiman Jain <chimanjain15@gmail.com> * chore: run gofmt Signed-off-by: Chiman Jain <chimanjain15@gmail.com> * chore:tidy the mod file Signed-off-by: Chiman Jain <chimanjain15@gmail.com> --------- Signed-off-by: Chiman Jain <chimanjain15@gmail.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: a2d2380910b1bfb3fa588f3b8de12ca8f1b725dd
Bumps [github.com/containerd/containerd](https://github.com/containerd/containerd) from 1.7.32 to 1.7.33. - [Release notes](https://github.com/containerd/containerd/releases) - [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md) - [Commits](containerd/containerd@v1.7.32...v1.7.33) --- updated-dependencies: - dependency-name: github.com/containerd/containerd dependency-version: 1.7.33 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 27ef3822af8f727c8dc5c6ad2fa85be859cee01e
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
02b58a8 to
80043d4
Compare
Contributor
|
/retest |
Contributor
|
/verified by @perdasilva 1. RBAC:
|
| Check | Result |
|---|---|
| CSV contains new RBAC rule | PASS - apiGroups: ["config.openshift.io"], resources: ["apiservers"], resourceNames: ["cluster"], verbs: ["get"] present in packageserver CSV |
2. Fallback path activation (no --tls-min-version flag)
| Check | Result |
|---|---|
Container args include --tls-min-version |
No - args are ["/bin/package-server", "-v=4", "--secure-port", "5443", "--global-namespace", "openshift-marketplace"] |
| Fallback code path triggered | PASS - confirmed by log message on both pods |
3. Cluster TLS profile applied correctly
The cluster APIServer CR has no explicit tlsSecurityProfile set, which defaults to Intermediate.
Both pods logged the profile application at startup:
Applying cluster TLS security profile:
minVersion=VersionTLS12
cipherSuites=[TLS_AES_128_GCM_SHA256 TLS_AES_256_GCM_SHA384
TLS_CHACHA20_POLY1305_SHA256 TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256]
| Check | Result |
|---|---|
minVersion matches Intermediate profile (TLS 1.2) |
PASS |
| Cipher suites match Intermediate profile | PASS |
| Both replicas applied the profile | PASS |
4. PackageServer health post-change
| Check | Result |
|---|---|
| Pods running | PASS - 2/2 Running, 0 restarts |
APIService v1.packages.operators.coreos.com |
PASS - Available: True (Passed) |
| PackageManifests served | PASS - 513 manifests returned |
| Pod events | PASS - Normal scheduling/pull/start only, no warnings or errors |
5. Code-level observations
| Aspect | Detail |
|---|---|
| Fail-closed behavior | If the APIServer CR is missing or unreadable, the function returns an error that halts startup |
| Non-OpenShift clusters | IsAPIAvailable() check makes this a no-op on vanilla Kubernetes |
| Flag precedence | Explicitly set --tls-min-version / --tls-cipher-suites flags are not overwritten |
| Timeout | 30-second context timeout on the API lookup |
| Unit tests | 5 test cases cover: non-OpenShift, Intermediate profile, Modern profile, flag precedence, missing CR |
Negative Tests
CVO and package-server-manager were scaled to 0 to prevent reconciliation during testing.
Test N1: Modern profile — TLS 1.2 rejection
Set tlsSecurityProfile.type: Modern on the cluster APIServer CR, restarted packageserver pods.
Logs confirmed profile change:
Applying cluster TLS security profile:
minVersion=VersionTLS13
cipherSuites=[TLS_AES_128_GCM_SHA256 TLS_AES_256_GCM_SHA384 TLS_CHACHA20_POLY1305_SHA256]
| Handshake | Result |
|---|---|
TLS 1.2 (--tls-max 1.2) |
REJECTED — tlsv1 alert protocol version (curl exit 35) |
TLS 1.3 (--tlsv1.3) |
ACCEPTED — HTTP 200 ok |
PASS — The packageserver correctly enforced TLS 1.3-only when the Modern profile was set.
Additional observation: an old pod that was not restarted still accepted TLS 1.2 (its Intermediate profile was loaded at boot). This confirms the profile is read once at startup, not dynamically.
Test N2: Bogus cipher — API-level validation
Attempted to set a Custom profile with ciphers: ["TLS_FAKE_CIPHER_256_BOGUS"].
Result: The APIServer CR rejected the patch with validation errors:
spec.tlsSecurityProfile.custom.ciphers: Invalid value: ["TLS_FAKE_CIPHER_256_BOGUS"]:
no supported cipher suite found
PASS — Defense in depth: invalid cipher names cannot reach the packageserver because the OpenShift API validates the APIServer CR schema. The packageserver code's fail-closed behavior is a second layer; the first layer is API admission.
Test N3: Old profile — TLS 1.0/1.1 floor
Set tlsSecurityProfile.type: Old on the cluster APIServer CR, restarted packageserver pods.
Logs confirmed profile change:
Applying cluster TLS security profile:
minVersion=VersionTLS10
cipherSuites=[TLS_AES_128_GCM_SHA256 ... TLS_RSA_WITH_3DES_EDE_CBC_SHA]
(21 cipher suites including legacy CBC and 3DES)
| Handshake | Result |
|---|---|
TLS 1.0 (--tls-max 1.0) |
REJECTED — no protocols available (OpenSSL in UBI9 has compiled out TLS 1.0) |
TLS 1.1 (--tls-max 1.1) |
REJECTED — no protocols available (same reason) |
TLS 1.2 (--tls-max 1.2) |
ACCEPTED — HTTP 200 ok |
TLS 1.3 (--tlsv1.3) |
ACCEPTED — HTTP 200 ok |
PASS — The packageserver correctly applied the Old profile (VersionTLS10 + legacy ciphers), but the underlying Go crypto/OpenSSL runtime on RHEL9/UBI9 provides a safety floor by refusing TLS 1.0/1.1 at the library level. The profile was honoured as far as the runtime allows.
Verdict
All checks passed (positive and negative). The packageserver correctly reads and applies the cluster-wide TLS security profile at startup via the fallback path. Negative tests confirmed:
- Profile enforcement is real — Modern (TLS 1.3 only) actively rejected TLS 1.2 connections
- Invalid ciphers are blocked — OpenShift API validation prevents bogus cipher names from reaching the packageserver
- Runtime safety floor — Even with the Old profile (
VersionTLS10), the Go/OpenSSL runtime on UBI9 refuses TLS 1.0/1.1 connections - Profile is startup-only — Unrestarted pods retain their boot-time profile, confirming the code path is a one-shot read
openshift-ci-robot
added
the
verified
|
@perdasilva: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@perdasilva: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
/lgtm |
Contributor
|
/test e2e-gcp-olm |
Contributor
|
@openshift-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Contributor
|
/retest |
rashmigottipati
approved these changes
Jun 25, 2026
rashmigottipati
left a comment
rashmigottipati
left a comment
Member
There was a problem hiding this comment.
/verified by @rashmigottipati
|
@rashmigottipati: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot, rashmigottipati The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The staging/ and vendor/ directories have been synchronized from the upstream repositories, pulling in the following commits:
This pull request is expected to merge without any human intervention. If tests are failing here, changes must land upstream to fix any issues so that future downstreaming efforts succeed.
/assign @openshift/openshift-team-operator-runtime
Summary by CodeRabbit