Skip to content

fix(pkg/validation/internal): check nil pointers before dereference - #493

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:masterfrom
alrs:internal-nil
Jun 2, 2026
Merged

fix(pkg/validation/internal): check nil pointers before dereference#493
openshift-merge-bot[bot] merged 1 commit into
operator-framework:masterfrom
alrs:internal-nil

Conversation

@alrs

@alrsalrs commented May 26, 2026

Copy link
Copy Markdown
Contributor

Several of the functions in pkg/validation/internal properly check to make sure that they haven't received a nil pointer, but they imporperly do it only after first attempting to dereference the pointer.

CopilotAI review requested due to automatic review settings May 26, 2026 04:39

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR prevents potential panics when validating bundles by avoiding dereferencing bundle.Name before confirming bundle is non-nil.

Changes:

  • Initialize errors.ManifestResult without using bundle.Name upfront in multiple validators
  • Assign result.Name only after the bundle == nil guard

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

FileDescription
pkg/validation/internal/operatorhubv2.goAvoids nil deref by setting result.Name after the nil check
pkg/validation/internal/operatorhub.goAvoids nil deref by setting result.Name after the nil check
pkg/validation/internal/community.goAvoids nil deref by setting result.Name after the nil check

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@perdasilva

Copy link
Copy Markdown
Contributor

/ok-to-test

@openshift-ciopenshift-ciBot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 26, 2026
@perdasilva

Copy link
Copy Markdown
Contributor

@grokspawn it would be good if you could 👀 this. I don't know if there was a reason to set the name in the result.

@codecov

codecovBot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 22.16%. Comparing base (3acdcf5) to head (4b5f809).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #493 +/- ##
==========================================
+ Coverage 22.13% 22.16% +0.02% 
==========================================
Files 60 60 Lines 7869 7872 +3 ==========================================
+ Hits 1742 1745 +3 
Misses 5969 5969 Partials 158 158 

☔ View full report in Codecov by Sentry.
📢 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.

@tmshort

Copy link
Copy Markdown
Contributor

/approve

@openshift-ciopenshift-ciBot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2026

@tmshorttmshort left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2026
@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

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:

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

@openshift-merge-bot
openshift-merge-botBot merged commit da918bd into operator-framework:masterJun 2, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.lgtmIndicates that a PR is ready to be merged.ok-to-testIndicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@alrs@perdasilva@tmshort