wip: no-op

[ingvagabund]

/hold

For the moment just for experimenting

Summary by CodeRabbit

• Tests
  • Refactored TLS observed-config tests for clearer, more reliable validation of TLS settings and accurate cipher-suite comparisons (handles OpenSSL→IANA names), reducing duplication and improving logging.
• Style
  • Minor formatting adjustment in test configuration comments.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[coderabbitai]

Walkthrough

Refactor TLS observed-config test to centralize nested-field validation, extract and verify servingInfo.minTLSVersion and cipherSuites (converting OpenSSL names to IANA), and extend expected-profile helper to return cipher suites alongside minTLSVersion and profile type.

Changes

Observed-config TLS test updates

Layer / File(s)	Summary
Imports and nested-field helper test/extended/tls/tls_observed_config.go	Add github.com/openshift/library-go/pkg/crypto import and introduce validateNestedField to standardize unstructured.Nested* value/found/err checks and logging.
testObservedConfig: extract & verify servingInfo test/extended/tls/tls_observed_config.go	Refactor testObservedConfig to use validateNestedField for spec.observedConfig; explicitly extract servingInfo.minTLSVersion and servingInfo.cipherSuites, convert OpenSSL cipher names to IANA, and compare against expected profile cipher suites without relying on ordering.
verifyObservedConfig, ConfigMap check, expected-profile helper test/extended/tls/tls_observed_config.go	Update verifyObservedConfigForTargets to use validateNestedField; adapt ConfigMap TLS injection check to the new getExpectedMinTLSVersionWithType multi-return signature; change helper to return (minTLSVersion, cipherSuites, profileType) and log cipher-suite counts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'wip: no-op' is vague and generic, using non-descriptive terms that do not convey meaningful information about the actual changes, which involve refactoring TLS observed-config tests.	Update the title to describe the actual changes, such as 'Refactor TLS observed-config tests to use validateNestedField helper' or similar, reflecting the substantive refactoring work performed.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All test names in tls_observed_config.go are stable. Test titles use hardcoded namespace and port values from package-scope arrays, not generated content or dynamic cluster-specific identifiers.
Test Structure And Quality	✅ Passed	Single responsibility per test; BeforeEach with proper skips; DeferCleanup for cleanup; timeouts on cluster ops; assertions have messages via helper function and format strings.
Microshift Test Compatibility	✅ Passed	No new Ginkgo tests added. The PR refactors existing tests in tls_observed_config.go. All existing tests already protected by exutil.IsMicroShiftCluster() skip check in BeforeEach blocks.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests added; PR is purely a refactoring of existing test infrastructure (helpers, error handling, function signatures) with no multi-node assumptions in scope.
Topology-Aware Scheduling Compatibility	✅ Passed	PR modifies only test code (test/extended/tls/tls_observed_config.go). Check applies to deployment manifests, operator code, and controllers—not test code.
Ote Binary Stdout Contract	✅ Passed	File contains no process-level stdout writes: no fmt.Print/Println/Printf, log.*, klog, or os.Stdout calls outside Ginkgo test contexts; all logging uses e2e.Logf (framework-safe).
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR refactors existing tests with no new Ginkgo tests added. Existing checkTLSConnection tests both IPv4/IPv6 with graceful fallback; no IPv4-only assumptions or external connectivity issues.
No-Weak-Crypto	✅ Passed	No weak crypto found. PR uses standard Go crypto/tls and OpenShift library-go. Old TLS versions only in "shouldNotWork" test fixtures to verify rejection.
Container-Privileges	✅ Passed	PR contains only Go test code with no Kubernetes manifests or container configurations. No privileged, hostPID, hostNetwork, or other container security settings found.
No-Sensitive-Data-In-Logs	✅ Passed	PR logs only TLS config data (version strings, cipher suite names, counts) and test metadata. No passwords, tokens, certificates, keys, or PII are exposed in any logging statements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ingvagabund]

/test tls-observed-config

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ingvagabund

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• test/extended/tls/OWNERS [ingvagabund]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ingvagabund]

/test tls-observed-config

[ingvagabund]

/test ci/prow/tls-observed-config

[ingvagabund]

Maybe there's just too many jobs already running? I don't see tls-observed-config triggered yet.

[ingvagabund]

/test pull-ci-openshift-origin-main-tls-observed-config

[coderabbitai]

🧹 Nitpick comments (1)

test/extended/tls/tls_observed_config.go (1)

1749-1758: 💤 Low value

Consider truncating large values in debug logs.

The helper logs the full value via e2e.Logf("Found %v at %s", value, path). For large cipher suite arrays or config maps, this could produce verbose output. Consider truncating or summarizing large values.

♻️ Optional refinement for log verbosity

 func validateNestedField(value interface{}, found bool, err error, fields ...string) {
 	path := strings.Join(fields, ".")
 	o.Expect(err).NotTo(o.HaveOccurred(), "failed to extract %s", path)
 	o.Expect(found).To(o.BeTrue(), "expected %s to exist", path)
 	o.Expect(value).NotTo(o.BeEmpty(), "expected %s to be non-empty", path)
-	e2e.Logf("Found %v at %s", value, path)
+	summary := fmt.Sprintf("%v", value)
+	if len(summary) > 100 {
+		summary = summary[:100] + "..."
+	}
+	e2e.Logf("Found %s at %s", summary, path)
 }

🤖 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 `@test/extended/tls/tls_observed_config.go` around lines 1749 - 1758, The
e2e.Logf in validateNestedField currently prints the entire value which can be
huge; change the logging to produce a summarized/truncated representation:
convert value to a string (e.g., via fmt.Sprintf("%v", value)), and if the
string length exceeds a threshold (e.g., 200 chars) truncate it and append an
indicator like "...(truncated, len=<fullLen>)" so logs remain readable; update
the log call in validateNestedField to use this truncated summary while
preserving the path and full length info.

🤖 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 `@test/extended/tls/tls_observed_config.go`:
- Around line 1749-1758: The e2e.Logf in validateNestedField currently prints
the entire value which can be huge; change the logging to produce a
summarized/truncated representation: convert value to a string (e.g., via
fmt.Sprintf("%v", value)), and if the string length exceeds a threshold (e.g.,
200 chars) truncate it and append an indicator like "...(truncated,
len=<fullLen>)" so logs remain readable; update the log call in
validateNestedField to use this truncated summary while preserving the path and
full length info.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 645ada33-5648-4ada-9eed-a955b7eb3cc7

📥 Commits

Reviewing files that changed from the base of the PR and between f3addfc and 859dec2.

📒 Files selected for processing (1)

• test/extended/tls/tls_observed_config.go

[openshift-merge-bot]

Scheduling required tests:
/test e2e-aws-csi
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-microshift
/test e2e-aws-ovn-microshift-serial
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-gcp-csi
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-upi

[openshift-ci]

@ingvagabund: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/tls-observed-config	f3addfc	link	false	/test tls-observed-config
ci/prow/e2e-metal-ipi-ovn-ipv6	859dec2	link	true	/test e2e-metal-ipi-ovn-ipv6

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.