Fix cross-provider variant cache contamination

[mstaeble]

Summary

• Moves variant caching inside the BigQuery provider with a provider-specific cache key (BQJobVariants~), eliminating the shared TestAllVariants~ key that caused cross-provider cache contamination
• Removes allJobVariants parameter from the DataProvider interface and middleware since it was a BigQuery implementation detail the Postgres provider always ignored
• Simplifies GetJobVariants from a cache-wrapping generator to a direct pass-through to provider.QueryJobVariants

Test plan

• go vet ./pkg/... ./test/... passes
• go test ./pkg/... passes
• make e2e (the originally failing TestRegressionCacheLoader requires GCS_SA_JSON_PATH; verified it is no longer affected by the shared cache key)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Performance

  • Improved component readiness report queries by reducing redundant data retrieval.
  • Added caching for job variant lookups to improve response times.

• Reliability

  • Enhanced error aggregation and handling when retrieving job data.
  • Simplified concurrent status and test-detail processing for more consistent results.

[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

[openshift-ci-robot]

@mstaeble: This pull request references TRT-2779 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• Moves variant caching inside the BigQuery provider with a provider-specific cache key (BQJobVariants~), eliminating the shared TestAllVariants~ key that caused cross-provider cache contamination
• Removes allJobVariants parameter from the DataProvider interface and middleware since it was a BigQuery implementation detail the Postgres provider always ignored
• Simplifies GetJobVariants from a cache-wrapping generator to a direct pass-through to provider.QueryJobVariants

Test plan

• go vet ./pkg/... ./test/... passes
• go test ./pkg/... passes
• make e2e (the originally failing TestRegressionCacheLoader requires GCS_SA_JSON_PATH; verified it is no longer affected by the shared cache key)

🤖 Generated with Claude Code

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]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

Walkthrough

Job variant retrieval is centralized in data providers, with BigQuery adding cached access. Report, test-detail, middleware, fallback, and provider interfaces no longer pass pre-fetched variants through query paths.

Changes

Job variant query flow

Layer / File(s)	Summary
Update query contracts pkg/api/componentreadiness/dataprovider/interface.go, pkg/api/componentreadiness/middleware/interface.go	Status, job-run, and middleware query interfaces no longer accept job variants.
Centralize provider retrieval pkg/api/componentreadiness/dataprovider/bigquery/provider.go, pkg/api/componentreadiness/dataprovider/postgres/provider.go	BigQuery retrieves and caches variants internally; status queries use provider-owned variant retrieval, while Postgres removes unused parameters.
Propagate middleware signatures pkg/api/componentreadiness/middleware/*	Middleware implementations and release fallback remove job variant arguments from query flows.
Simplify report orchestration pkg/api/componentreadiness/component_report.go, pkg/api/componentreadiness/test_details.go	Report and test-detail paths stop fetching variants upfront and call revised provider and middleware methods.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: ready-for-human-review

Suggested reviewers: openshift-merge-bot[bot], stbenjam

🚥 Pre-merge checks | ✅ 19 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Coverage For New Features	⚠️ Warning	The PR adds new BigQuery cache-key logic and cache-wrapped QueryJobVariants, but no *_test.go files changed and pkg/api/componentreadiness/dataprovider/bigquery/provider_test.go is empty.	Add unit/regression tests for BQ QueryJobVariants cache-key isolation and the new provider-internal lookup path; cover the removed allJobVariants plumbing if needed.

✅ Passed checks (19 passed)

Check name	Status	Explanation
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.
Go Error Handling	✅ Passed	Changed paths wrap BigQuery variant-fetch failures with %w, keep nil checks for release times, and add no new panic or ignored-error sites.
Sql Injection Prevention	✅ Passed	Changed SQL paths use placeholders for release/time/job/variant filters; the only fmt.Sprintf inputs are config/hardcoded identifiers, not user input.
Excessive Css In React Should Use Styles	✅ Passed	PR only touches Go backend componentreadiness files; no React/JSX/CSS changes or inline style objects are present.
Single Responsibility And Clear Naming	✅ Passed	The refactor removes variant plumbing, centralizes BigQuery caching with a clear provider-specific key, and keeps the touched methods narrowly focused.
Feature Documentation	✅ Passed	docs/features has only an unrelated job-analysis-symptoms doc; no component-readiness feature doc exists to update for these API/cache refactors.
Stable And Deterministic Test Names	✅ Passed	The PR only changes production code; no test files or Ginkgo titles were modified, so there are no unstable test names to flag.
Test Structure And Quality	✅ Passed	No changed Ginkgo tests were present; the modified tests are standard Go unit tests, so this checklist is not applicable.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the patch only changes component-readiness library code, and no changed file contains It/Describe/Context/When.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Touched files are componentreadiness library code; no Ginkgo e2e specs or SNO-related skips were found, and no e2e test files were added.
Topology-Aware Scheduling Compatibility	✅ Passed	Only component-readiness query/interface plumbing changed; no deployment manifests, controllers, affinities, nodeSelectors, replicas, or topology logic were added.
Ote Binary Stdout Contract	✅ Passed	No added stdout writes in process-level code: the diff has no fmt.Print/klog/log stdout calls, and the only init() in touched files just assigns a function pointer.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added in the changed files, so there are no IPv4 or external-connectivity assumptions to flag.
No-Weak-Crypto	✅ Passed	No weak-crypto symbols, custom crypto, or secret/token comparisons appear in the touched files or diff.
Container-Privileges	✅ Passed	The PR only changes Go code under pkg/api/componentreadiness; no container/K8s manifests were touched, and no privileged/securityContext flags appear in the diff.
No-Sensitive-Data-In-Logs	✅ Passed	No added log lines in the diff expose passwords, tokens, PII, hostnames, or customer data; the change is query/signature refactoring only.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: moving job-variant caching to the BigQuery provider to prevent cross-provider cache contamination.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci-robot]

@mstaeble: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

Details

In response to this:

Summary

• Moves variant caching inside the BigQuery provider with a provider-specific cache key (BQJobVariants~), eliminating the shared TestAllVariants~ key that caused cross-provider cache contamination
• Removes allJobVariants parameter from the DataProvider interface and middleware since it was a BigQuery implementation detail the Postgres provider always ignored
• Simplifies GetJobVariants from a cache-wrapping generator to a direct pass-through to provider.QueryJobVariants

Test plan

• go vet ./pkg/... ./test/... passes
• go test ./pkg/... passes
• make e2e (the originally failing TestRegressionCacheLoader requires GCS_SA_JSON_PATH; verified it is no longer affected by the shared cache key)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Performance

• Improved component readiness report queries by reducing redundant data retrieval.

• Added caching for job variant lookups to improve response times.

• Reliability

• Enhanced error aggregation and handling when retrieving job data.

• Simplified concurrent status and test-detail processing for more consistent results.

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.

[coderabbitai]

🧹 Nitpick comments (2)

pkg/api/componentreadiness/component_report.go (2)

338-338: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use structured logging for includeVariants.

As per coding guidelines, prefer structured logging with WithField over formatting values into strings.

♻️ Proposed refactor

-		fLog.Infof("running sample query with includeVariants: %+v", c.ReqOptions.VariantOption.IncludeVariants)
+		fLog.WithField("includeVariants", c.ReqOptions.VariantOption.IncludeVariants).Info("running sample query")

🤖 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 `@pkg/api/componentreadiness/component_report.go` at line 338, Update the
sample-query log in the component report flow to use structured logging via
WithField for includeVariants instead of embedding the value with formatted
string interpolation. Preserve the existing log message and value semantics
while attaching includeVariants as a dedicated field.

Source: Coding guidelines

---

70-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider dropping this wrapper. GetJobVariants only forwards to provider.QueryJobVariants(ctx), so removing it would simplify the package API if this helper isn’t meant to stay as a stable entrypoint.

🤖 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 `@pkg/api/componentreadiness/component_report.go` at line 70, Remove the
redundant GetJobVariants wrapper that only delegates to
provider.QueryJobVariants(ctx), and update its callers to invoke the provider
method directly. Preserve the existing behavior unless GetJobVariants is
intentionally required as a stable public entrypoint.

🤖 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 `@pkg/api/componentreadiness/component_report.go`:
- Line 338: Update the sample-query log in the component report flow to use
structured logging via WithField for includeVariants instead of embedding the
value with formatted string interpolation. Preserve the existing log message and
value semantics while attaching includeVariants as a dedicated field.
- Line 70: Remove the redundant GetJobVariants wrapper that only delegates to
provider.QueryJobVariants(ctx), and update its callers to invoke the provider
method directly. Preserve the existing behavior unless GetJobVariants is
intentionally required as a stable public entrypoint.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 299002c6-a91c-4a97-8038-f1686aa7dbc3

📥 Commits

Reviewing files that changed from the base of the PR and between 9451619 and dc70178.

📒 Files selected for processing (11)

• pkg/api/componentreadiness/component_report.go
• pkg/api/componentreadiness/dataprovider/bigquery/provider.go
• pkg/api/componentreadiness/dataprovider/interface.go
• pkg/api/componentreadiness/dataprovider/postgres/provider.go
• pkg/api/componentreadiness/middleware/interface.go
• pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go
• pkg/api/componentreadiness/middleware/list.go
• pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go
• pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go
• pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go
• pkg/api/componentreadiness/test_details.go

[openshift-merge-bot]

Scheduling required tests:
/test e2e

[openshift-ci]

@mstaeble: all tests passed!

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.

[smg247]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mstaeble, smg247

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:

• OWNERS [smg247]

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