Skip to content

Conversation

@joelanford
Copy link
Member

Description of the change:
Use netgo and osusergo when building binaries for opm image to ensure completely static binary builds.

Motivation for the change:
The current goreleaser build results in an image with a binary that panics when the network is used (e.g. opm render with a remote image)

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

… builds

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
@openshift-ci openshift-ci bot requested review from njhale and timflannagan November 5, 2021 02:31
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 5, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joelanford

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 5, 2021
@codecov
Copy link

codecov bot commented Nov 5, 2021

Codecov Report

Merging #821 (e73407a) into master (7c8187a) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #821      +/-   ##
==========================================
+ Coverage   50.93%   50.94%   +0.01%     
==========================================
  Files         103      103              
  Lines        9001     9001              
==========================================
+ Hits         4585     4586       +1     
+ Misses       3552     3551       -1     
  Partials      864      864              
Impacted Files Coverage Δ
pkg/registry/query.go 60.93% <0.00%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c8187a...e73407a. Read the comment docs.

mod_timestamp: "{{ .CommitTimestamp }}"
flags: &build-flags
- -tags=json1
- -tags=json1,netgo,osusergo
Copy link
Member

Choose a reason for hiding this comment

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

I think if I squint I can see why osusergo is needed but it's not immediately obvious outside of opm's general domain what command is triggering the need for that tag.

/lgtm

Copy link
Member Author

Choose a reason for hiding this comment

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

🙏

I'm not positive if osusergo is actually necessary, but since we don't have anything that actually tests the results of our binary builds, I figured it was best to err on the side of caution.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2021
@openshift-merge-robot openshift-merge-robot merged commit d4b7d96 into operator-framework:master Nov 5, 2021
@joelanford joelanford deleted the goreleaser-no-cgo branch November 5, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants