Reject cyclic substitutesFor chains instead of looping until OOM

[haklein]

A bundle whose olm.substitutesFor annotation references its own CSV name (or a set of bundles forming an indirect cycle) causes an endless loop when walking the chain in addSubstitutesFor. This finally leads to a OOM condition: fatal error: runtime: out of memory. (OCPBUGS-37284).

Guard every substitutesFor chain walk with a visited set seeded with the walk's starting node, so any cycle - direct or indirect - fails fast with an error naming the offending bundle instead of exhausting memory.

Description of the change:

Add a visited-set guard (markSeen) to all five substitutesFor chain
walks in pkg/sqlite/addSubstitutesFor. A cyclic chain now fails with
cyclic substitutesFor chain detected involving "<name>" instead of
looping forever.

Regression tests: self-reference (olm.substitutesFor == own CSV name)
and indirect cycle (A→B→A). Both hang on master, "green" with the fix.

Motivation for the change:

OCPBUGS-37284 — a bundle whose olm.substitutesFor references its own
name sends the walk at load.go:293 into an infinite loop, growing
skips each iteration until fatal error: runtime: out of memory
(broke IIB builds for web-terminal-operator 1.7.x).

Same guard pattern as the existing replaces cycle checks:
pkg/sqlite/load.go (replaceCycle) and
alpha/model/model.go (validateReplacesChain).

Reviewer Checklist

• Implementation matches the proposed design, or proposal is updated to match implementation
• Sufficient unit test coverage
• Sufficient end-to-end test coverage
• Docs updated or added to /docs
• Commit messages sensible and descriptive

[openshift-ci]

Hi @haklein. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[perdasilva]

/ok-to-test

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.83%. Comparing base (37385d0) to head (ac5d94f).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/sqlite/load.go	60.00%	4 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2038      +/-   ##
==========================================
+ Coverage   58.81%   58.83%   +0.01%     
==========================================
  Files         141      141              
  Lines       13434    13454      +20     
==========================================
+ Hits         7901     7915      +14     
- Misses       4323     4325       +2     
- Partials     1210     1214       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[grokspawn]

Hey @haklein thank you for your PR.
While this PR tackles a legitimate issue, it is for a feature which was deprecated 4+ years ago (SQLite catalogs) and which will shortly be deleted.

IMO we should merge this PR but we should not allow further PRs against deprecated SQLite code.

[haklein]

IMO we should merge this PR but we should not allow further PRs against deprecated SQLite code.

fair enough, thanks for your feedback. Will keep that in mind when picking further backlog candidates.

[grokspawn]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grokspawn

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 [grokspawn]

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

[grokspawn]

fair enough, thanks for your feedback. Will keep that in mind when picking further backlog candidates.

No fault of yours. I just want to make sure we're careful about communicating that this is going away soon, so there will be extremely limited opportunity for anyone to make use of this fix.