NO-ISSUE: Synchronize From Upstream Repositories#701
Merged
openshift-merge-bot[bot] merged 102 commits intoopenshift:mainfrom Apr 22, 2026
Merged
NO-ISSUE: Synchronize From Upstream Repositories#701openshift-merge-bot[bot] merged 102 commits intoopenshift:mainfrom
openshift-merge-bot[bot] merged 102 commits intoopenshift:mainfrom
Conversation
Bumps [charset-normalizer](https://github.com/jawah/charset_normalizer) from 3.4.6 to 3.4.7. - [Release notes](https://github.com/jawah/charset_normalizer/releases) - [Changelog](https://github.com/jawah/charset_normalizer/blob/master/CHANGELOG.md) - [Commits](jawah/charset_normalizer@3.4.6...3.4.7) --- updated-dependencies: - dependency-name: charset-normalizer dependency-version: 3.4.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
openshift-bot
added
tide/merge-method-merge
openshift-ci-robot
added
the
jira/valid-reference
|
@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. |
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds unit and end-to-end TLS profile tests, new E2E step implementations and deployment patch/restore logic, port-forward/TLS helpers, and bumps to several Python and Go module dependencies. No production API signatures were changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Tester
participant TestHarness as Test Harness
participant KubeAPI as Kubernetes API
participant Kubectl as kubectl (port-forward)
participant Pod as Target Pod
participant TLSClient as Local TLS Client
Tester->>TestHarness: Start TLS scenario (configure deployment)
TestHarness->>KubeAPI: Patch Deployment args (--tls-profile=custom, ciphers, curves, min-version)
KubeAPI-->>TestHarness: Acknowledge rollout / pods restarting
TestHarness->>KubeAPI: Wait for rollout status
KubeAPI-->>TestHarness: Pod ready
TestHarness->>Kubectl: Start port-forward to Service:metricsPort
Kubectl->>Pod: Forward local port to Pod metrics port
TLSClient->>Kubectl: Dial local port (TLS client config)
Kubectl->>Pod: Forward TLS handshake
Pod-->>TLSClient: TLS handshake result (negotiated cipher/curve or failure)
TLSClient-->>TestHarness: Report handshake outcome
TestHarness->>Tester: Assert expected acceptance/rejection
Tester->>TestHarness: Scenario end
TestHarness->>KubeAPI: Restore original Deployment args (from deploymentRestores)
KubeAPI-->>TestHarness: Rollout restore complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 |
1 similar comment
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 |
Bumps [github.com/moby/spdystream](https://github.com/moby/spdystream) from 0.5.0 to 0.5.1. - [Release notes](https://github.com/moby/spdystream/releases) - [Commits](moby/spdystream@v0.5.0...v0.5.1) --- updated-dependencies: - dependency-name: github.com/moby/spdystream dependency-version: 0.5.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Unit tests in tlsprofiles package verify cipher negotiation, cipher rejection, min-version enforcement, and curve acceptance/rejection by starting a local TLS server with a custom profile and connecting to it with a restricted client config. - e2e feature (tls.feature) patches the catalogd deployment with specific custom TLS settings for each scenario, asserts the expected connection behaviour, then restores the original args on cleanup. Covers min-version enforcement (TLSv1.3), cipher negotiation and rejection (TLS 1.2 + ECDHE_ECDSA), and curve enforcement (prime256v1 accepted, secp521r1 rejected). - GODOG_ARGS variable added to the e2e Makefile target so a single feature file can be run with: make test-e2e GODOG_ARGS=features/tls.feature Signed-off-by: Todd Short <tshort@redhat.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Bumps [marocchino/sticky-pull-request-comment](https://github.com/marocchino/sticky-pull-request-comment) from 3 to 3.0.2. - [Release notes](https://github.com/marocchino/sticky-pull-request-comment/releases) - [Commits](marocchino/sticky-pull-request-comment@v3...v3.0.2) --- updated-dependencies: - dependency-name: marocchino/sticky-pull-request-comment dependency-version: 3.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [go.podman.io/image/v5](https://github.com/containers/container-libs) from 5.39.1 to 5.39.2. - [Release notes](https://github.com/containers/container-libs/releases) - [Commits](containers/container-libs@image/v5.39.1...image/v5.39.2) --- updated-dependencies: - dependency-name: go.podman.io/image/v5 dependency-version: 5.39.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [dorny/paths-filter](https://github.com/dorny/paths-filter) from 4 to 4.0.1. - [Release notes](https://github.com/dorny/paths-filter/releases) - [Changelog](https://github.com/dorny/paths-filter/blob/master/CHANGELOG.md) - [Commits](dorny/paths-filter@v4...v4.0.1) --- updated-dependencies: - dependency-name: dorny/paths-filter dependency-version: 4.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
e79a121 to
ac28118
Compare
Contributor
|
I suspect this will fail due to the TLS profiles testing, in which case, we will want to skip them in |
Contributor
|
/lgtm |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go (1)
135-139: PinMinVersionin all TLS 1.2 client configs to ensure tests remain stable across Go versions.These tests set
MaxVersion: tls.VersionTLS12to exercise TLS 1.2-specific behavior, but rely on Go's default minimum version (TLS 1.2 since Go 1.18). Go's documentation explicitly states the default is "currently" TLS 1.2, indicating it may change in future releases. PinningMinVersionensures these tests continue to validate the intended behavior regardless of Go version updates.Add
MinVersion: tls.VersionTLS12to thetls.Configstructs at lines 135–139, 164–168, 189–192, 216–221, and 247–252.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go` around lines 135 - 139, Tests create tls.Config literals that set MaxVersion: tls.VersionTLS12 (for example the clientCfg variable) but don't set MinVersion; add MinVersion: tls.VersionTLS12 to each tls.Config literal that pins MaxVersion to tls.VersionTLS12 so the tests don't depend on Go's changing default minimum TLS version—i.e., find every tls.Config struct in this test file that includes MaxVersion: tls.VersionTLS12 and add MinVersion: tls.VersionTLS12 alongside it.test/e2e/steps/tls_steps.go (1)
153-156: Set explicitMinVersion: tls.VersionTLS12alongsideMaxVersionfor consistent behavior across Go versions.These TLS 1.2-specific tests currently depend on Go's default minimum TLS version, which varies by release: TLS 1.0 before Go 1.18, and TLS 1.2 from Go 1.18 onward. Without an explicit
MinVersion, tests running on Go 1.17 or earlier could unexpectedly negotiate TLS 1.0 or 1.1 instead of 1.2, causing the rejection test to fail and cipher/curve tests to validate incorrect protocol versions.Suggested change
conn, err := tls.Dial("tcp", addr, &tls.Config{ InsecureSkipVerify: true, //nolint:gosec // self-signed cert in e2e + MinVersion: tls.VersionTLS12, MaxVersion: tls.VersionTLS12, CipherSuites: []uint16{cipherID}, })Apply to lines 153–156, 330–334, 356–360, 381–386, and 407–412.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/steps/tls_steps.go` around lines 153 - 156, The TLS tests set MaxVersion: tls.VersionTLS12 but omit MinVersion, making behavior vary across Go versions; update the tls.Config initializations (e.g., the tls.Dial call where conn, err := tls.Dial("tcp", addr, &tls.Config{...}) and the other tls.Config occurrences referenced in this file) to include MinVersion: tls.VersionTLS12 alongside MaxVersion so the client explicitly requires TLS 1.2; apply this change to all mentioned spots (the blocks at the given ranges) to ensure consistent TLS 1.2-only negotiation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go`:
- Around line 135-139: Tests create tls.Config literals that set MaxVersion:
tls.VersionTLS12 (for example the clientCfg variable) but don't set MinVersion;
add MinVersion: tls.VersionTLS12 to each tls.Config literal that pins MaxVersion
to tls.VersionTLS12 so the tests don't depend on Go's changing default minimum
TLS version—i.e., find every tls.Config struct in this test file that includes
MaxVersion: tls.VersionTLS12 and add MinVersion: tls.VersionTLS12 alongside it.
In `@test/e2e/steps/tls_steps.go`:
- Around line 153-156: The TLS tests set MaxVersion: tls.VersionTLS12 but omit
MinVersion, making behavior vary across Go versions; update the tls.Config
initializations (e.g., the tls.Dial call where conn, err := tls.Dial("tcp",
addr, &tls.Config{...}) and the other tls.Config occurrences referenced in this
file) to include MinVersion: tls.VersionTLS12 alongside MaxVersion so the client
explicitly requires TLS 1.2; apply this change to all mentioned spots (the
blocks at the given ranges) to ensure consistent TLS 1.2-only negotiation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7330088-cf6c-42dd-a75a-46e48b840574
⛔ Files ignored due to path filters (15)
go.sumis excluded by!**/*.sumhack/tools/test-profiling/go.sumis excluded by!**/*.sumhack/tools/test-profiling/vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/LICENSEis excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/PATENTSis excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/options.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/modules.txtis excluded by!**/vendor/**vendor/go.podman.io/image/v5/docker/docker_client.gois excluded by!**/vendor/**,!vendor/**vendor/go.podman.io/image/v5/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (8)
go.modhack/tools/test-profiling/go.modinternal/shared/util/tlsprofiles/tlsprofiles_connection_test.gorequirements.txttest/e2e/features/tls.featuretest/e2e/steps/hooks.gotest/e2e/steps/steps.gotest/e2e/steps/tls_steps.go
✅ Files skipped from review due to trivial changes (3)
- hack/tools/test-profiling/go.mod
- requirements.txt
- go.mod
Bumps [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) from 5.17.1 to 5.18.0. - [Release notes](https://github.com/go-git/go-git/releases) - [Commits](go-git/go-git@v5.17.1...v5.18.0) --- updated-dependencies: - dependency-name: github.com/go-git/go-git/v5 dependency-version: 5.18.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
ac28118 to
b16e4d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go (1)
93-101: Serialize global TLS profile mutation in tests.
setCustomProfilemutates package-level globals for the full test duration. If any test in this package runs in parallel later, this becomes racy and can produce nondeterministic failures.♻️ Proposed fix
import ( "crypto/ecdsa" @@ "net" + "sync" "testing" "time" @@ ) +var customProfileMu sync.Mutex + // setCustomProfile configures the package-level custom TLS profile for the duration // of the test and restores the original state via t.Cleanup. func setCustomProfile(t *testing.T, cipherNames []string, curveNames []string, minVersion string) { t.Helper() + customProfileMu.Lock() + t.Cleanup(func() { customProfileMu.Unlock() }) origProfile := configuredProfile origCustom := customTLSProfile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go` around lines 93 - 101, setCustomProfile currently mutates the package-level globals configuredProfile and customTLSProfile for the full test duration, which is racy if tests run in parallel; fix it by serializing access: add a package-level sync.Mutex (e.g., tlsProfileMu) and have setCustomProfile Lock() at the start and Unlock() in the t.Cleanup closure after restoring origProfile and origCustom, so the global mutation is held while the test uses the custom profile and released when cleanup runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/steps/tls_steps.go`:
- Around line 254-273: buildCustomTLSArgs currently only strips combined form
flags like "--tls-profile=..." and leaves split-form flags like "--tls-profile"
followed by "custom" in baseArgs, causing stale/conflicting args; update the
filtering loop in buildCustomTLSArgs to also detect split-form TLS flags
("--tls-profile", "--tls-custom-version", "--tls-custom-ciphers",
"--tls-custom-curves") and skip both the flag and its next argument when present
(ensure you check bounds before skipping the next element), so that all old TLS
flags (both joined and split forms) are removed before appending the new custom
TLS flags.
---
Nitpick comments:
In `@internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go`:
- Around line 93-101: setCustomProfile currently mutates the package-level
globals configuredProfile and customTLSProfile for the full test duration, which
is racy if tests run in parallel; fix it by serializing access: add a
package-level sync.Mutex (e.g., tlsProfileMu) and have setCustomProfile Lock()
at the start and Unlock() in the t.Cleanup closure after restoring origProfile
and origCustom, so the global mutation is held while the test uses the custom
profile and released when cleanup runs.
🪄 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: 109c5bfd-4af3-433e-835c-94ef97876c18
⛔ Files ignored due to path filters (15)
go.sumis excluded by!**/*.sumhack/tools/test-profiling/go.sumis excluded by!**/*.sumhack/tools/test-profiling/vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/LICENSEis excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/PATENTSis excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/options.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**hack/tools/test-profiling/vendor/modules.txtis excluded by!**/vendor/**vendor/go.podman.io/image/v5/docker/docker_client.gois excluded by!**/vendor/**,!vendor/**vendor/go.podman.io/image/v5/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (8)
go.modhack/tools/test-profiling/go.modinternal/shared/util/tlsprofiles/tlsprofiles_connection_test.gorequirements.txttest/e2e/features/tls.featuretest/e2e/steps/hooks.gotest/e2e/steps/steps.gotest/e2e/steps/tls_steps.go
✅ Files skipped from review due to trivial changes (2)
- hack/tools/test-profiling/go.mod
- requirements.txt
🚧 Files skipped from review as they are similar to previous changes (3)
- go.mod
- test/e2e/steps/hooks.go
- test/e2e/steps/steps.go
Comment on lines
+254
to
+273
| func buildCustomTLSArgs(baseArgs []string, version, ciphers, curves string) []string { | ||
| filtered := make([]string, 0, len(baseArgs)+4) | ||
| for _, arg := range baseArgs { | ||
| switch { | ||
| case strings.HasPrefix(arg, "--tls-profile="), | ||
| strings.HasPrefix(arg, "--tls-custom-version="), | ||
| strings.HasPrefix(arg, "--tls-custom-ciphers="), | ||
| strings.HasPrefix(arg, "--tls-custom-curves="): | ||
| // drop — will be replaced below | ||
| default: | ||
| filtered = append(filtered, arg) | ||
| } | ||
| } | ||
| filtered = append(filtered, "--tls-profile=custom", "--tls-custom-version="+version) | ||
| if ciphers != "" { | ||
| filtered = append(filtered, "--tls-custom-ciphers="+ciphers) | ||
| } | ||
| if curves != "" { | ||
| filtered = append(filtered, "--tls-custom-curves="+curves) | ||
| } |
There was a problem hiding this comment.
Handle split-form TLS flags when rebuilding deployment args.
Current filtering only removes --tls-*=... forms. If existing args use split form (--tls-profile, custom), stale flags remain and can produce conflicting CLI input.
🐛 Proposed fix
func buildCustomTLSArgs(baseArgs []string, version, ciphers, curves string) []string {
filtered := make([]string, 0, len(baseArgs)+4)
- for _, arg := range baseArgs {
+ skipNext := false
+ for i, arg := range baseArgs {
+ if skipNext {
+ skipNext = false
+ continue
+ }
switch {
+ case arg == "--tls-profile",
+ arg == "--tls-custom-version",
+ arg == "--tls-custom-ciphers",
+ arg == "--tls-custom-curves":
+ // Drop split-form flag and its following value token (if present).
+ if i+1 < len(baseArgs) && !strings.HasPrefix(baseArgs[i+1], "--") {
+ skipNext = true
+ }
case strings.HasPrefix(arg, "--tls-profile="),
strings.HasPrefix(arg, "--tls-custom-version="),
strings.HasPrefix(arg, "--tls-custom-ciphers="),
strings.HasPrefix(arg, "--tls-custom-curves="):
// drop — will be replaced below
default:
filtered = append(filtered, arg)
}
}📝 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
| func buildCustomTLSArgs(baseArgs []string, version, ciphers, curves string) []string { | |
| filtered := make([]string, 0, len(baseArgs)+4) | |
| for _, arg := range baseArgs { | |
| switch { | |
| case strings.HasPrefix(arg, "--tls-profile="), | |
| strings.HasPrefix(arg, "--tls-custom-version="), | |
| strings.HasPrefix(arg, "--tls-custom-ciphers="), | |
| strings.HasPrefix(arg, "--tls-custom-curves="): | |
| // drop — will be replaced below | |
| default: | |
| filtered = append(filtered, arg) | |
| } | |
| } | |
| filtered = append(filtered, "--tls-profile=custom", "--tls-custom-version="+version) | |
| if ciphers != "" { | |
| filtered = append(filtered, "--tls-custom-ciphers="+ciphers) | |
| } | |
| if curves != "" { | |
| filtered = append(filtered, "--tls-custom-curves="+curves) | |
| } | |
| func buildCustomTLSArgs(baseArgs []string, version, ciphers, curves string) []string { | |
| filtered := make([]string, 0, len(baseArgs)+4) | |
| skipNext := false | |
| for i, arg := range baseArgs { | |
| if skipNext { | |
| skipNext = false | |
| continue | |
| } | |
| switch { | |
| case arg == "--tls-profile", | |
| arg == "--tls-custom-version", | |
| arg == "--tls-custom-ciphers", | |
| arg == "--tls-custom-curves": | |
| // Drop split-form flag and its following value token (if present). | |
| if i+1 < len(baseArgs) && !strings.HasPrefix(baseArgs[i+1], "--") { | |
| skipNext = true | |
| } | |
| case strings.HasPrefix(arg, "--tls-profile="), | |
| strings.HasPrefix(arg, "--tls-custom-version="), | |
| strings.HasPrefix(arg, "--tls-custom-ciphers="), | |
| strings.HasPrefix(arg, "--tls-custom-curves="): | |
| // drop — will be replaced below | |
| default: | |
| filtered = append(filtered, arg) | |
| } | |
| } | |
| filtered = append(filtered, "--tls-profile=custom", "--tls-custom-version="+version) | |
| if ciphers != "" { | |
| filtered = append(filtered, "--tls-custom-ciphers="+ciphers) | |
| } | |
| if curves != "" { | |
| filtered = append(filtered, "--tls-custom-curves="+curves) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/steps/tls_steps.go` around lines 254 - 273, buildCustomTLSArgs
currently only strips combined form flags like "--tls-profile=..." and leaves
split-form flags like "--tls-profile" followed by "custom" in baseArgs, causing
stale/conflicting args; update the filtering loop in buildCustomTLSArgs to also
detect split-form TLS flags ("--tls-profile", "--tls-custom-version",
"--tls-custom-ciphers", "--tls-custom-curves") and skip both the flag and its
next argument when present (ensure you check bounds before skipping the next
element), so that all old TLS flags (both joined and split forms) are removed
before appending the new custom TLS flags.
Contributor
|
/retest-required |
Contributor
|
This requires #702 for this to pass. |
Add 7 Ginkgo tests under [sig-olmv1][OCPFeatureGate:NewOLMDeploymentConfig] covering the spec.config.inline.deploymentConfig feature: Positive tests (verify applied customisations): - environment variables - resource requirements - tolerations - node selector - annotations on deployment and pod template Negative tests (verify terminal validation errors): - invalid deploymentConfig.env type (string instead of array) - unknown field inside deploymentConfig (additionalProperties:false) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
Signed-off-by: Todd Short <tshort@redhat.com>
…64 support Signed-off-by: Daniel Franz <dfranz@redhat.com>
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>
Signed-off-by: Todd Short <todd.short@me.com>
…to run outside of OCP
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
b44108c to
5e6b95e
Compare
Contributor
|
/lgtm |
Contributor
|
@openshift-bot: all tests passed! 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
|
/verified by tmshort No extraneous clusterobjectset messages in upstream-e2e logs. The logs also contain: Which properly disables the TLSProfiles tests for downstream. All other changes were dependency bumps, and since the code builds and passes, I'm considering it verified. |
openshift-ci-robot
added
the
verified
|
@tmshort: 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. |
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
17 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 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