IBM / template-node-typescript

Github Template that uses TypeScript with Node.js to create a BFF or Microservice API with Open API Specification
Apache License 2.0
71 stars 41 forks source link

Dockerfile updated to pass all the Red Hat Certification requirements #27

Closed manojjahgirdar closed 3 years ago

manojjahgirdar commented 3 years ago

The Dockerfile now passes all the following Tests on Red Hat Certification:

Note: At times the tests free_of_critical_vulnerabilities or free_of_important_vulnerabilities may fail if the ubi8/nodejs-14 has any known vulnerability.

bwoolf1 commented 3 years ago

Manoj: Looking at https://github.com/IBM/template-node-typescript/pull/27/commits/d4a9813e1ec9cf24ac4967255603d812826831ce, lots of good stuff here. There's a couple of things I'm confused about though.

The Dockerfile is adding security updates and license files to the builder image. Shouldn't those go in the second image, the one that will be deployed? Also, it probably doesn't need to be root to run WORKDIR /opt/app-root/src, I would think default could do that.

Also, your file set currently includes .DS_Store. If that's not needed, let's take it out. Maybe add it to .gitignore to prevent this in the future.

Nice progress. Thanks!

manojjahgirdar commented 3 years ago

Manoj: Looking at d4a9813, lots of good stuff here. There's a couple of things I'm confused about though.

The Dockerfile is adding security updates and license files to the builder image. Shouldn't those go in the second image, the one that will be deployed? Also, it probably doesn't need to be root to run WORKDIR /opt/app-root/src, I would think default could do that.

Also, your file set currently includes .DS_Store. If that's not needed, let's take it out. Maybe add it to .gitignore to prevent this in the future.

Nice progress. Thanks!

Fixed these issues. Thanks

bwoolf1 commented 3 years ago

Fixed these issues. Thanks

Manoj: I see where you deleted .DS_Store and moved the WORKDIR command so that it's run by the default user. Good, thanks.

I'm still confused about where the security update and license stuff is being done. It looks like the code is doing the right stuff but to the wrong image. It's updating the builder image. Seems to me it should be updating the second image, the one with the labels and exposed port, etc.

To make sure I'm explaining what I mean: This code is between the two FROM statements:

FROM ...
USER root
## comment the below line if there are no sec severities
RUN dnf -y update-minimal --security --sec-severity=Important --sec-severity=Critical && dnf clean all
COPY ./licenses /licenses
USER default
...
FROM ...
...

Shouldn't it be after the second from statement? Or if it's in the right place, please explain.

bwoolf1 commented 3 years ago

Minoj: The value for the name label should be of the form <namespace>/<image-name>. For example, it's ubi8/openjdk-11 in the Java UBI. So "Typescript Microservice" (in https://github.com/IBM/template-node-typescript/pull/27/commits/25de4945a52c9b4a94027ea152ef9077ba284d57) is an OK example but could be better.

For this image, it's not a base image stored in a registry anywhere, so the image namespace and name are not yet determined. So for this example, the name of the GitHub org and repo is probably as good a default as any: ibm/template-node-typescript. If the image (with no application) were stored in a registry, that's probably what we would name it.

So let's change:

LABEL name="Typescript Microservice"

to:

LABEL name="ibm/template-node-typescript"
bwoolf1 commented 3 years ago

Manoj: Likewise, Red Hat doesn't put the letter "v" in their version name, so let's not show that in our example. For example, ubi8/openjdk-11:1.3-15 sets version="1.3" and release="15".

So let's change:

      version="v1.0.0" \

to:

      version="1.0.0" \
manojjahgirdar commented 3 years ago

Bobby: Answering your questions:

I'm still confused about where the security update and license stuff is being done.

I believe as the base image is the same in both the FROM commands, updating of image security contents, copying the license can be done anywhere. However keeping it consistent across all the starter kit templates, I'll move it after the 2nd FROM statement.

The value for the name label should be of the form /. For example, it's ubi8/openjdk-11 in the Java UBI. So "Typescript Microservice" (in 25de494) is an OK example but could be better.

Ok noted, I'll change the name label.

Likewise, Red Hat doesn't put the letter "v" in their version name, so let's not show that in our example. For example, ubi8/openjdk-11:1.3-15 sets version="1.3" and release="15".

Ok makes sense, I'll change the version and release tags accordingly.

Thanks for the Feedback.

bwoolf1 commented 3 years ago

LGTM. Nice job.

@csantanapr:

bwoolf1 commented 3 years ago

@csantanapr: What updates do you want to this PR to approve it? Please document these requests for @manojjahgirdar. Thanks.

manojjahgirdar commented 3 years ago
csantanapr commented 3 years ago

Please verify app still works on OpenShift, build and deploy with Toolkit

manojjahgirdar commented 3 years ago

Please verify app still works on OpenShift, build and deploy with Toolkit

@csantanapr,

I tried it, and it fails on tag-release, it says:

ERROR fatal: tag 'v1.0.4' already exists

Screenshot 2021-05-05 at 8 29 32 PM

What does this mean?

bwoolf1 commented 3 years ago

@manojjahgirdar: Now that your pipelines are able to log into Artifactory, is this pipeline able to run successfully? If so, please post a screenshot.

ERROR fatal: tag 'v1.0.4' already exists means that the pipeline tried to use a tag that it has already used. This might happen if you reran the same pipeline run (I'm not sure). Try making a change to your code and merging that to your branch, which will trigger the pipeline to start a new run.

manojjahgirdar commented 3 years ago

@manojjahgirdar: Now that your pipelines are able to log into Artifactory, is this pipeline able to run successfully? If so, please post a screenshot.

ERROR fatal: tag 'v1.0.4' already exists means that the pipeline tried to use a tag that it has already used. This might happen if you reran the same pipeline run (I'm not sure). Try making a change to your code and merging that to your branch, which will trigger the pipeline to start a new run.

@bwoolf1, this still fails with the same error.

manojjahgirdar commented 3 years ago

@csantanapr, Here is the verification that the app still works on OpenShift, using the Toolkit pipeline to build and deploy. Screenshot 2021-05-07 at 10 41 01 PM

manojjahgirdar commented 3 years ago

@csantanapr Vulnerability Advisor now finds no issues, here is proof. Screenshot 2021-05-07 at 10 45 12 PM