Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Dockerfile): move to ubi-micro [OSD-8579] #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

georgettica
Copy link
Contributor

@georgettica georgettica commented Jan 9, 2022

the previous solution was ok, but the current one is more compatible
with the security team

  • bump golang to latest (1.17)
  • fix secuirty vuln found with 'trivy image scan'
  • use ubi-micro to allow code-scanning using 'clair'
  • did a 'go mod vendor && go mod tidy'
  • change circleci to accomodate to this

related to #14

Dockerfile Outdated
@@ -7,7 +7,7 @@ WORKDIR /opt

RUN git update-index --refresh; make token-refresher

FROM scratch as runner
FROM registry.redhat.io/ubi8-micro:8.5-596 as runner
Copy link
Member

Choose a reason for hiding this comment

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

Why is this preferable to scratch? Especially for a community project like Observatorium?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because scratch is not scannable + not FIPS compliant

So using it will mask the issues that exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@squat now the image is pullable from dockerhub
does that address the issue you had?

Copy link

Choose a reason for hiding this comment

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

@squat 's concern here comes from a place of not wanting to implicitly tie a community project to a base image that is not widely used / understood by other members of the community. The upstream Thanos & Prometheus repos use busybox. Is that a preferable option here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to build using both patterns and I am facing an issue with them
additionally one of the requirements is having quay.io scan the image, which it can't for the blackbox image

Copy link
Member

Choose a reason for hiding this comment

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

additionally one of the requirements is having quay.io scan the image

Requirements from whom? I don't know of any proposal or issue in the Observatorium organization where the project committed itself to ensuring all containers are scanned by Quay. 🙂 FIPS compliance sounds great but none of our other images follow this pattern. I would say that in terms of maintainability, it is far more important for the project to have consistent and coherent practices in all repos than for one individual image to be in Quay.

As a maintainer outside of Red Hat, would encourage Red Hatters to ensure that internal business requirements are not conflated with the project's independent goals and commitments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

working on changing this in a different location, but it should be noted that before I moved the image to scratch we noticed some security issues
https://quay.io/repository/observatorium/token-refresher/manifest/sha256:6ce9b80cd1d907cb6c9ed2a18612f386f7503257772d1d88155a4a2e6773fd00?tab=vulnerabilities
if this is not available to you I will find a way to parlay that information to you.

basically, we are concerned now for the cve https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-36159. and hiding this under scratch might not solve this problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@squat does that point make sense?

Copy link
Member

Choose a reason for hiding this comment

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

not quite.. why would scratch have a vulnerable version of libfetch? there is no libfetch at all on a scratch system

the previous solution was ok, but the current one is more compatible
with the security team

- bump golang to latest (1.17)
- fix secuirty vuln found with 'trivy image scan'
- use ubi-micro to allow code-scanning using 'clair'
- did a 'go mod vendor && go mod tidy'
- gofmt a file using 'gofmt -w <filename>'

related to observatorium#14
@georgettica georgettica force-pushed the georgettica/reduce-attack-surface_2 branch from 1050de9 to f3e4f55 Compare January 24, 2022 10:19
Copy link
Contributor

@matej-g matej-g left a comment

Choose a reason for hiding this comment

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

Looking good to me, thanks @georgettica!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants