enterprise-contract / ec-cli

Supply chain artifact verifier and policy checker
https://enterprisecontract.dev/docs/ec-cli/
Apache License 2.0
27 stars 29 forks source link

Use go-toolset golang base image for redhat builds #1607

Closed simonbaird closed 4 months ago

simonbaird commented 5 months ago

The --chown=1001:0 was required to avoid an error like this:

fatal: detected dubious ownership in repository at '/build' To add an exception for this directory, call: git config --global --add safe.directory /build

I'm not exactly sure the reason why, but I believe it's because the new base image comes with a user called 'default' with uid 1001. When we copy the source code into the build container it comes in owned by root, and that causes git to be suspicious when it runs as user default.

Ref: https://issues.redhat.com/browse/EC-625

simonbaird commented 5 months ago

Draft because I don't know if this is something we want or not. Happy to abandon unless we can decide there's some value in using ubi9/go-toolset instead of plain ubi9/ubi-minimal.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.39%. Comparing base (3ce6dd4) to head (903a911). Report is 2 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/graphs/tree.svg?width=650&height=150&src=pr&token=CY5ORXJB33&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract)](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract) ```diff @@ Coverage Diff @@ ## main #1607 +/- ## ========================================== + Coverage 80.70% 87.39% +6.69% ========================================== Files 65 76 +11 Lines 4794 5134 +340 ========================================== + Hits 3869 4487 +618 + Misses 925 647 -278 ``` | [Flag](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract) | Coverage Δ | | |---|---|---| | [acceptance](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract) | `73.07% <ø> (?)` | | | [generative](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract) | `80.70% <ø> (ø)` | | | [integration](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract) | `80.70% <ø> (ø)` | | | [unit](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract) | `80.70% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract#carryforward-flags-in-the-pull-request-comment) to find out more. [see 43 files with indirect coverage changes](https://app.codecov.io/gh/enterprise-contract/ec-cli/pull/1607/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enterprise-contract)
zregvart commented 5 months ago

I've seen RPM repositories fail with 500/404, perhaps just for Fedora mirrors, not sure. So the benefit of this could be that we depend on only registry.access.redhat.com rather than that and the RPM repository as well...

simonbaird commented 5 months ago

I've seen RPM repositories fail with 500/404, perhaps just for Fedora mirrors, not sure. So the benefit of this could be that we depend on only registry.access.redhat.com rather than that and the RPM repository as well...

Hmm yeah, it also means we don't need to figure out the cachi2 prefetch for those rpms.

simonbaird commented 5 months ago

The relevant Dockerfile iiuc.

simonbaird commented 5 months ago

Hmm yeah, it also means we don't need to figure out the cachi2 prefetch for those rpms.

Not true, since we also microdnf install jq later on.

zregvart commented 5 months ago

Why don't we give this a try, I don't see any downsides really...

simonbaird commented 5 months ago

I think #1608 is slightly preferable, wdyt?

simonbaird commented 4 months ago

Merged #1608 and abandoned this one.