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

[TB] FINOS Publish container image to public registry #481

Open
aaronreed708 opened this issue Aug 17, 2023 · 13 comments
Open

[TB] FINOS Publish container image to public registry #481

aaronreed708 opened this issue Aug 17, 2023 · 13 comments
Assignees
Labels
theme builder app Theme Builder application

Comments

@aaronreed708
Copy link
Contributor

Problem/Concern

The goal of this issue is to develop a process, establish a CI/CD pipeline, and publish a container image of Theme Builder to a publicly-available registry. This would make it much easier for non-developers to "kick the tires" with Theme Builder, especially in the near term before we have Theme Builder deployed in the FINOS environment. The container image could eventually be used in deployments, too.

It would be wonderful if this could be completed in anticipation of the Grace Hopper event which is on 09/22.

Proposed Solution

@aaronreed708 aaronreed708 added the theme builder app Theme Builder application label Aug 17, 2023
@aaronreed708
Copy link
Contributor Author

From Mao:

We use DockerHub, see https://hub.docker.com/u/finos. Before publishing, we'll have to do some security/compliance scanning of the package to publish (we use trivy, auditjs and OWASP dependency check, see: https://github.com/finos/security-scanning.

@aaronreed708
Copy link
Contributor Author

I replied that even if we have to scan and upload the image by hand, making it available on the registry soon-ish would be good.

Reply from Mao:

We have some projects already doing it - ie Legend Studio you can already start working on a branch and play with it, let me know if you need any support (im maoo on GitHub), I'd be happy to help.

Regarding security and compliance scanning of the image, I think it would be beneficial to look at the Dockerfile, in order to understand what exactly needs to be scanned.

@aaronreed708 aaronreed708 self-assigned this Aug 23, 2023
@aaronreed708
Copy link
Contributor Author

@smithbk911 to follow up with Dan to see if we can publish to DockerHub before code lives in FINOS

@aaronreed708
Copy link
Contributor Author

aaronreed708 commented Sep 27, 2023

The result of this issue is that we never published a docker image to Docker Hub. Dan said that we were not allowed. NearForm agreed to publish it for us for Grace Hopper, but since our code wasn't complete and forked until that morning, NearForm didn't feel comfortable publishing the Docker image that close to event time.

I used Clair to scan our Docker image in preparation for this issue and found 200+ vulnerabilities, mostly Debian. By switching to the latest ubi8-nodejs-16 base image from RedHat/Quay (https://catalog.redhat.com/software/containers/ubi8/nodejs-16/615aee9fc739c0a4123a87e1), I cut down all of the vulnerabilities. I just need to get approval for the change on the call today and then I'll push it in.

@aaronreed708
Copy link
Contributor Author

diff --git a/code/Dockerfile b/code/Dockerfile
index c616948..22609b8 100644
--- a/code/Dockerfile
+++ b/code/Dockerfile
@@ -1,4 +1,4 @@
-FROM node:16
+FROM registry.access.redhat.com/ubi8/nodejs-16:1-129
 RUN node -v
 # Copy source
 RUN mkdir $HOME/code

@aaronreed708
Copy link
Contributor Author

@brycecurtis is concerned switching to a RedHat based image might have licensing issues that a Debian based image wouldn't have. So we will lean on the FINOS leaders since the transition is set for tomorrow.

Reading https://snyk.io/blog/choosing-the-best-node-js-docker-image/, if we stick with the official nodejs images from DockerHub, it sounds like the best tag for us would probably be: node:18.18-bookworm-slim. This change would put us on the latest stable Debian and latest LTS of nodejs. This image, according to DockerHub, has 17 low severity issues. When I run a11y-theme-builder using this base image through Clair, I see 1 medium vulnerability, a dozen or two low severity issues and some vulnerabilities that haven't been assigned severities, yet.

evangk6 added a commit that referenced this issue Oct 6, 2023
#481: update base docker image to latest node…
@aaronreed708
Copy link
Contributor Author

@TheJuanAndOnly99 setup DOCKER_PASSWORD repo key that we can use

Juan also proposed that we use https://github.com/finos/legend-studio/blob/master/.github/workflows/manual__publish-docker.yml as a reference as we build our GitHub Action.

@aaronreed708
Copy link
Contributor Author

When I asked @TheJuanAndOnly99 about container scanning requirements, he said:

We use a GitHub action (similar to the other scans) to scan Docker images. You can read about it at https://github.com/finos/security-scanning#docker and find an example as well. The job can be added into the existing action at https://github.com/finos/a11y-theme-builder-sdk/blob/main/.github/workflows/security.yml.

aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Oct 23, 2023
aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Oct 23, 2023
aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Oct 23, 2023
@aaronreed708
Copy link
Contributor Author

Hey @TheJuanAndOnly99, do you have a recommendation for how to make sure that the docker publish doesn't happen unless the docker scan (and other scans) do not have an error? In my branch I added the docker scan to the security.yml workflow and I added a publish_docker.yml workflow. Currently both will be triggered if we push to main (plus some other rules). It didn't seem to make sense to combine the workflows. Should I make security a reusable workflow ala https://docs.github.com/en/actions/using-workflows/reusing-workflows? Any help appreciated.

aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Oct 24, 2023
@TheJuanAndOnly99
Copy link
Member

TheJuanAndOnly99 commented Oct 24, 2023

Hey @aaronreed708 we suggest creating a single GitHub action that runs docker build, scan and publish in sequence. That way if the scan steps fails, the action quits. Let me know what you think!

@aaronreed708
Copy link
Contributor Author

@TheJuanAndOnly99 we should probably still have a build and scan based in the security flow then too, right? So that the image scan is run routinely every day, even if there haven't been release-level commits. And then, to your point, I can add the build and scanning to the push of the image workflow and if scan fails, push doesn't happen. That sounds good?

aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Oct 24, 2023
@TheJuanAndOnly99
Copy link
Member

@aaronreed708 That sounds good!

aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Nov 16, 2023
aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Nov 16, 2023
aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Nov 16, 2023
aaronreed708 added a commit to aaronreed708/a11y-theme-builder that referenced this issue Nov 16, 2023
aaronreed708 added a commit that referenced this issue Dec 6, 2023
aaronreed708 added a commit that referenced this issue Dec 6, 2023
@aaronreed708
Copy link
Contributor Author

This issue should be complete once the PR is reviewed. Currently there are vulnerabilities so the Docker image won't publish. But I tested publishing when scanning with no threshold (e.g. anything goes) and it worked fine. Vulnerabilities bug is #742.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme builder app Theme Builder application
Projects
Status: In Progress
Development

No branches or pull requests

2 participants