operator-lifecycle-manager icon indicating copy to clipboard operation
operator-lifecycle-manager copied to clipboard

pkg/package-server: Add null check when attempting to initialize an API server config

Open timflannagan opened this issue 4 years ago • 1 comments

Description of the change: Update the pkg/package-server/server/server.go package and add a null check for the config variable returned when running the Config method for the PackageServerOptions structure. That Config method doesn't ensure that the variable returned is populated when the error returned is nil.

When attempting to initialize a delegating authentication configuration, and the environment isn't properly setup (e.g. no cluster is running), and SIGINT is sent to that process, an error will be returned from the poll operation due to a timeout being reached, yet only the lastApplyErr will be returned, which is nil in that scenario.

Note: this appears to only be reproducible when $KUBECONFIG is set locally, but no cluster is running, so this wouldn't be all that present in the wild.

Motivation for the change: Properly handle signals during the package-server executable so a panic isn't produced.

Reviewer Checklist

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

timflannagan avatar Sep 15 '21 15:09 timflannagan

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: timflannagan

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

The pull request process is described here

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-ci[bot] avatar Feb 17 '22 16:02 openshift-ci[bot]

Do you think it's worth removing the two merge commits we've added @timflannagan?

awgreene avatar Sep 03 '22 15:09 awgreene