OCPBUGS-81452: Synchronize From Upstream Repositories#696
Open
openshift-bot wants to merge 98 commits intoopenshift:mainfrom
Open
OCPBUGS-81452: Synchronize From Upstream Repositories#696openshift-bot wants to merge 98 commits intoopenshift:mainfrom
openshift-bot wants to merge 98 commits intoopenshift:mainfrom
Conversation
Bumps [go.opentelemetry.io/otel/sdk](https://github.com/open-telemetry/opentelemetry-go) from 1.40.0 to 1.43.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-go@v1.40.0...v1.43.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/otel/sdk dependency-version: 1.43.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.5.2 to 6.0.0. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v5.5.2...v6.0.0) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: 6.0.0 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>
…d (#2637) Boxcutter v0.13.1 includes the fix from package-operator/boxcutter#501 which ensures collision detection runs before revision linearity checks. This allows us to remove the foreignRevisionController workaround that was manually detecting ActionProgressed objects owned by foreign ClusterExtensions. Assisted-by: Claude
openshift-bot
added
the
tide/merge-method-merge
Contributor
|
@openshift-bot: GitHub didn't allow me to request PR reviews from the following users: openshift/openshift-team-operator-framework. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
openshift-ci-robot
added
jira/severity-low
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-81452, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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-bot
added
approved
WalkthroughUpdated dependencies; moved cert-manager Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
/re-title OCPBUGS-77972, OCPBUGS-81452: Synchronize From Upstream Repositories |
Contributor
|
/test openshift-e2e-aws |
Contributor
|
/retitle OCPBUGS-77972, OCPBUGS-81452: Synchronize From Upstream Repositories |
openshift-ci
bot
changed the title
openshift-ci-robot
added
jira/severity-important
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-77972, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-81452, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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
|
/jira refresh |
|
@camilamacedo86: This pull request references Jira Issue OCPBUGS-77972, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-81452, which is invalid:
Comment 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
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: kuiwang02. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
Contributor
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@camilamacedo86: This pull request references Jira Issue OCPBUGS-77972, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-81452, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (bandrade@redhat.com), skipping review request. 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
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: kuiwang02. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
Signed-off-by: Todd Short <tshort@redhat.com>
…t in OTE tests Update all remaining references to ClusterExtensionRevision in openshift/tests-extension to use ClusterObjectSet, matching the upstream rename in operator-framework/operator-controller#2589. Files updated: - test/qe/specs/olmv1_ce.go: RBAC resource names and comments - test/olmv1-preflight.go: scenario constants, test names, RBAC rules - .openshift-tests-extension/openshift_payload_olmv1.json: test name - pkg/bindata/qe/bindata.go: embedded RBAC templates - test/qe/testdata/olm/sa-nginx-limited-boxcutter.yaml: RBAC resources - test/qe/testdata/olm/sa-nginx-insufficient-operand-rbac-boxcutter.yaml: RBAC resources Signed-off-by: Camila Macedo <cmacedo@redhat.com> Made-with: Cursor
…s ClusterObjectSet The upstream rename of ClusterExtensionRevision to ClusterObjectSet (operator-framework/operator-controller#2589) breaks the incompatible operator detection in cluster-olm-operator. The cluster-olm-operator binary still reads ClusterExtensionRevision resources to find operators with olm.maxOpenShiftVersion, so after the rename it never detects incompatible operators and InstalledOLMOperatorsUpgradeable stays True. Skip this test when NewOLMBoxCutterRuntime feature gate is enabled until cluster-olm-operator is updated to read ClusterObjectSet. Signed-off-by: Camila Macedo <cmacedo@redhat.com> Made-with: Cursor
Signed-off-by: Francesco Giudici <fgiudici@redhat.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
9f00e24 to
8188669
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openshift/tests-extension/go.mod (1)
87-92: Confirm OTel version skew and clarify its source.Lines 87 and 90–92 show core OTel modules at
v1.43.0while OTLP exporter modules (lines 88–89) remain atv1.40.0. This 3-minor-version gap is real, and both core and exporter packages are used in the codebase. However, there are no explicitreplacedirectives that explain or manage this skew—it appears to result from transitive dependency resolution. Verify whether this lag is intentional (e.g., due to an upstream constraint) or accidental, and update exporter versions to match core if unintentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift/tests-extension/go.mod` around lines 87 - 92, The go.mod shows version skew between core OTel modules (go.opentelemetry.io/otel, metric, sdk, trace at v1.43.0) and OTLP exporter modules (go.opentelemetry.io/otel/exporters/otlp/otlptrace and otlptracegrpc at v1.40.0); confirm whether this was intentional due to an upstream constraint or accidental transitive resolution and either update the exporter module versions to v1.43.0 to match the core modules (update the entries for go.opentelemetry.io/otel/exporters/otlp/otlptrace and go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc) or add an explicit replace directive documenting the required skew, then run dependency resolution (go get / go mod tidy) and run tests to ensure compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/shared/util/tlsprofiles/mozilla_data.go`:
- Around line 68-103: parseProfile currently can return a tlsProfile with no
usable ciphers or curves; after building cipherNums, curveNums and determining
minTLSVersion (the tlsVersion variable via version.Set(cfg.TLSVersions[0])), add
a guard that panics when the parsed result would be unusable: if curveNums is
empty OR (minTLSVersion is less than TLS1_3 and cipherNums is empty) then panic
with a clear message including the profile name; this ensures parseProfile (and
the returned tlsProfile, cipherSlice, curveSlice, minTLSVersion) fails fast on
bad embedded data instead of returning a zero-value profile.
---
Nitpick comments:
In `@openshift/tests-extension/go.mod`:
- Around line 87-92: The go.mod shows version skew between core OTel modules
(go.opentelemetry.io/otel, metric, sdk, trace at v1.43.0) and OTLP exporter
modules (go.opentelemetry.io/otel/exporters/otlp/otlptrace and otlptracegrpc at
v1.40.0); confirm whether this was intentional due to an upstream constraint or
accidental transitive resolution and either update the exporter module versions
to v1.43.0 to match the core modules (update the entries for
go.opentelemetry.io/otel/exporters/otlp/otlptrace and
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc) or add an
explicit replace directive documenting the required skew, then run dependency
resolution (go get / go mod tidy) and run tests to ensure compatibility.
🪄 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: Pro Plus
Run ID: 171f4ded-1550-4959-8108-e617657366ed
⛔ Files ignored due to path filters (106)
.bingo/gojq.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumopenshift/tests-extension/go.sumis excluded by!**/*.sumopenshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/.golangci.ymlis excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/README.mdis excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/cache.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/decode.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/decode_map_utils.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/diagnose.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/encode.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/simplevalue.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/stream.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/structfields.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/fxamacker/cbor/v2/valid.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/.golangci.ymlis excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/CHANGELOG.mdis excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/Makefileis excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/RELEASING.mdis excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/attribute/encoder.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/attribute/hash.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/attribute/internal/attribute.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/attribute/kv.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/attribute/type_string.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/attribute/value.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/dependencies.Dockerfileis excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/config.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/semconv/v1.40.0/otelconv/metric.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/trace/trace.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/version.gois excluded by!**/vendor/**openshift/tests-extension/vendor/go.opentelemetry.io/otel/versions.yamlis excluded by!**/vendor/**openshift/tests-extension/vendor/modules.txtis excluded by!**/vendor/**vendor/github.com/fxamacker/cbor/v2/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/decode_map_utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/diagnose.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/simplevalue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/structfields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/valid.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3-binding.cis excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3-binding.his excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3_opt_serialize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/alpha/declcfg/model_to_declcfg.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/alpha/model/model.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/lib/bundle/validate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/registry/conversion.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/Makefileis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/hash.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/internal/attribute.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/kv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/type_string.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/value.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/dependencies.Dockerfileis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/otelconv/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/trace.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/versions.yamlis excluded by!**/vendor/**,!vendor/**vendor/helm.sh/helm/v3/pkg/chart/metadata.gois excluded by!**/vendor/**,!vendor/**vendor/helm.sh/helm/v3/pkg/chartutil/expand.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/pkg.package-operator.run/boxcutter/boxcutter.gois excluded by!**/vendor/**,!vendor/**vendor/pkg.package-operator.run/boxcutter/machinery/objects.gois excluded by!**/vendor/**,!vendor/**vendor/pkg.package-operator.run/boxcutter/managedcache/objectboundaccess.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (17)
.bingo/Variables.mk.bingo/gojq.mod.bingo/variables.envMakefileOWNERS_ALIASESgo.modhack/test/e2e-coverage.shhack/tools/update-tls-profiles.shinternal/operator-controller/applier/phase.gointernal/operator-controller/applier/phase_test.gointernal/operator-controller/controllers/clusterobjectset_controller.gointernal/operator-controller/controllers/clusterobjectset_controller_test.gointernal/operator-controller/controllers/revision_engine_factory.gointernal/shared/util/tlsprofiles/mozilla_data.gointernal/shared/util/tlsprofiles/mozilla_data.jsoninternal/shared/util/tlsprofiles/tlsprofiles_test.goopenshift/tests-extension/go.mod
💤 Files with no reviewable changes (4)
- OWNERS_ALIASES
- .bingo/gojq.mod
- .bingo/Variables.mk
- .bingo/variables.env
✅ Files skipped from review due to trivial changes (2)
- hack/tools/update-tls-profiles.sh
- internal/shared/util/tlsprofiles/mozilla_data.json
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/operator-controller/applier/phase.go
- internal/operator-controller/controllers/revision_engine_factory.go
- Makefile
- internal/operator-controller/applier/phase_test.go
- internal/operator-controller/controllers/clusterobjectset_controller_test.go
- go.mod
- internal/operator-controller/controllers/clusterobjectset_controller.go
Comment on lines
+68
to
+103
| func parseProfile(name string, cfg mozillaConfiguration) (tlsProfile, []string, []string) { | ||
| var skippedC, skippedK []string | ||
| var cipherNums []uint16 | ||
| for _, c := range append(cfg.Ciphersuites, cfg.Ciphers.IANA...) { | ||
| id := cipherSuiteId(c) | ||
| if id == 0 { | ||
| skippedC = append(skippedC, c) | ||
| continue | ||
| } | ||
| cipherNums = append(cipherNums, id) | ||
| } | ||
|
|
||
| var curveNums []tls.CurveID | ||
| for _, c := range cfg.TLSCurves { | ||
| id := curveId(c) | ||
| if id == 0 { | ||
| skippedK = append(skippedK, c) | ||
| continue | ||
| } | ||
| curveNums = append(curveNums, id) | ||
| } | ||
|
|
||
| if len(cfg.TLSVersions) == 0 { | ||
| panic(fmt.Sprintf("tlsprofiles: profile %q has no tls_versions in embedded mozilla_data.json", name)) | ||
| } | ||
|
|
||
| var version tlsVersion | ||
| if err := version.Set(cfg.TLSVersions[0]); err != nil { | ||
| panic(fmt.Sprintf("tlsprofiles: profile %q has unrecognized tls_versions[0] %q: %v", name, cfg.TLSVersions[0], err)) | ||
| } | ||
|
|
||
| return tlsProfile{ | ||
| ciphers: cipherSlice{cipherNums: cipherNums}, | ||
| curves: curveSlice{curveNums: curveNums}, | ||
| minTLSVersion: version, | ||
| }, skippedC, skippedK |
There was a problem hiding this comment.
Guard the “nothing usable was parsed” case.
Keeping partial skips is fine, but parseProfile still accepts a profile that resolves to zero curves or, for pre-TLS-1.3 profiles, zero cipher suites. That leaves modernTLSProfile / intermediateTLSProfile as zero-value data if the embedded JSON shape drifts or a future sync stops matching these fields. Add an explicit empty-result check so bad embedded data fails fast instead of silently publishing an empty profile.
🛡️ Proposed guard
var version tlsVersion
if err := version.Set(cfg.TLSVersions[0]); err != nil {
panic(fmt.Sprintf("tlsprofiles: profile %q has unrecognized tls_versions[0] %q: %v", name, cfg.TLSVersions[0], err))
}
+ if len(curveNums) == 0 {
+ panic(fmt.Sprintf("tlsprofiles: profile %q resolved no supported tls_curves from embedded mozilla_data.json", name))
+ }
+ if version < tlsVersion(tls.VersionTLS13) && len(cipherNums) == 0 {
+ panic(fmt.Sprintf("tlsprofiles: profile %q resolved no supported cipher suites from embedded mozilla_data.json", name))
+ }
+
return tlsProfile{
ciphers: cipherSlice{cipherNums: cipherNums},
curves: curveSlice{curveNums: curveNums},
minTLSVersion: version,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/shared/util/tlsprofiles/mozilla_data.go` around lines 68 - 103,
parseProfile currently can return a tlsProfile with no usable ciphers or curves;
after building cipherNums, curveNums and determining minTLSVersion (the
tlsVersion variable via version.Set(cfg.TLSVersions[0])), add a guard that
panics when the parsed result would be unusable: if curveNums is empty OR
(minTLSVersion is less than TLS1_3 and cipherNums is empty) then panic with a
clear message including the profile name; this ensures parseProfile (and the
returned tlsProfile, cipherSlice, curveSlice, minTLSVersion) fails fast on bad
embedded data instead of returning a zero-value profile.
Contributor
|
/retest-required |
Contributor
|
/retest |
Contributor
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
openshift-ci-robot
added
the
verified
|
@bandrade: 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
|
/retest |
1 similar comment
Contributor
|
/retest |
Contributor
|
/lgtm |
Contributor
Contributor
|
@openshift-bot: The following tests 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 |
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
18 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.
JIRA Tickets:
The downstream repository has been updated with the following following upstream commits:
The
vendor/directory has been updated and the following commits were carried:@catalogd-updateThis 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.
/cc @openshift/openshift-team-operator-framework
Summary by CodeRabbit
New Features
Bug Fixes
Chores