Skip to content

✨ add support for receiving kubeconfig in op-con/catd options - #2562

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
grokspawn:hypershift-adaptation
Mar 19, 2026
Merged

✨ add support for receiving kubeconfig in op-con/catd options#2562
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
grokspawn:hypershift-adaptation

Conversation

@grokspawn

Copy link
Copy Markdown
Contributor

Description

Adds the ability for op-con / catd to interact with a non-default apiserver

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

CopilotAI review requested due to automatic review settings March 12, 2026 20:06
@netlify

netlifyBot commented Mar 12, 2026

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commitd68417a
🔍 Latest deploy loghttps://app.netlify.com/projects/olmv1/deploys/69b3236b5b94140008565cc4
😎 Deploy Previewhttps://deploy-preview-2562--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

CopilotAI 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.

Pull request overview

Adds a --kubeconfig CLI option to let operator-controller and catalogd connect to a non-default Kubernetes API server (e.g., HyperShift scenarios), instead of always using the default controller-runtime config loading behavior.

Changes:

  • Add --kubeconfig flag to operator-controller and catalogd.
  • When --kubeconfig is provided, build the REST config from the specified kubeconfig file before creating the controller-runtime manager.

Reviewed changes

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

FileDescription
cmd/operator-controller/main.goAdds --kubeconfig flag and uses it to construct the REST config passed to ctrl.NewManager.
cmd/catalogd/main.goAdds --kubeconfig flag and uses it to construct the REST config passed to ctrl.NewManager.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment threadcmd/operator-controller/main.go
Comment threadcmd/catalogd/main.go
@codecov

codecovBot commented Mar 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.85714% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.25%. Comparing base (6692d1b) to head (d68417a).
⚠️ Report is 15 commits behind head on main.

Files with missing linesPatch %Lines
cmd/catalogd/main.go45.45%5 Missing and 1 partial ⚠️
cmd/operator-controller/main.go40.00%5 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #2562 +/- ##
==========================================
- Coverage 68.65% 64.25% -4.41% 
==========================================
Files 131 131 Lines 9333 9352 +19 ==========================================
- Hits 6408 6009 -399 - Misses 2436 2864 +428 + Partials 489 479 -10 
FlagCoverage Δ
e2e42.19% <42.85%> (-0.02%)⬇️
experimental-e2e11.84% <23.80%> (-39.83%)⬇️
unit53.73% <0.00%> (-0.11%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@grokspawn
grokspawnforce-pushed the hypershift-adaptation branch from c3f5ed2 to 1d14a74CompareMarch 12, 2026 20:34
Signed-off-by: grokspawn <jordan@nimblewidget.com>
CopilotAI review requested due to automatic review settings March 12, 2026 20:34
@grokspawn
grokspawnforce-pushed the hypershift-adaptation branch from 1d14a74 to d68417aCompareMarch 12, 2026 20:34

CopilotAI 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.

Pull request overview

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


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

You can also share your feedback on Copilot code review. Take the survey.

return err
}
} else {
restConfig = ctrl.GetConfigOrDie()

CopilotAIMar 12, 2026

Copy link

Choose a reason for hiding this comment

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

run() now has two different failure behaviors for loading cluster config: the kubeconfig path returns an error, but the default path uses ctrl.GetConfigOrDie() which panics instead of returning an error. Since run() already returns error, prefer using a non-panicking config loader and propagate the error so startup failures are handled consistently.

Suggested change
restConfig=ctrl.GetConfigOrDie()
restConfig, err=ctrl.GetConfig()
iferr!=nil {
setupLog.Error(err, "unable to load in-cluster configuration")
returnerr
}

Copilot uses AI. Check for mistakes.
Comment threadcmd/catalogd/main.go
return err
}
} else {
restConfig = ctrl.GetConfigOrDie()

CopilotAIMar 12, 2026

Copy link

Choose a reason for hiding this comment

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

Similar to operator-controller: the kubeconfig branch returns an error, but the default branch calls ctrl.GetConfigOrDie() which panics on failure. Since run() returns error, prefer consistently returning an error for config-load failures rather than panicking.

Suggested change
restConfig=ctrl.GetConfigOrDie()
restConfig, err=ctrl.GetConfig()
iferr!=nil {
setupLog.Error(err, "unable to load in-cluster kubeconfig")
returnerr
}

Copilot uses AI. Check for mistakes.

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.

There is a point that GetConfigOrDie() is no longer necessary since it's not in the NewManager() call

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

On the other hand, it's established behavior if using the default path. While I can't foresee that someone would be expecting a panic outcome, it's been a precedent for a long time, and doesn't have to change here. Another alternative is even forcing a panic with an alternative kubeconfig, so that the branching behaviors align again.

@grokspawngrokspawn changed the title ✨ WIP: add support for receiving kubeconfig in op-con/catd options✨ add support for receiving kubeconfig in op-con/catd optionsMar 18, 2026
@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 Mar 18, 2026

@camilamacedo86camilamacedo86 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 Mar 19, 2026

@rashmigottipatirashmigottipati left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, rashmigottipati, 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

@grokspawn

Copy link
Copy Markdown
ContributorAuthor

/override codecov/patch
/override codecov/project

these are not required jobs and should not prevent merging

@openshift-ci

Copy link
Copy Markdown

@grokspawn: Overrode contexts on behalf of grokspawn: codecov/patch, codecov/project

Details

In response to this:

/override codecov/patch
/override codecov/project

these are not required jobs and should not prevent merging

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.

@openshift-merge-bot
openshift-merge-botBot merged commit 8ccea5a into operator-framework:mainMar 19, 2026
31 of 33 checks passed
@grokspawn
grokspawn deleted the hypershift-adaptation branch March 19, 2026 12:31
grokspawn added a commit to grokspawn/operator-controller that referenced this pull request Apr 3, 2026
openshift-merge-botBot pushed a commit that referenced this pull request Apr 6, 2026
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@grokspawn@tmshort@rashmigottipati@camilamacedo86