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

Amend Dockerfile to support alternative base images #264

Merged
merged 10 commits into from
Feb 15, 2022

Conversation

jason-fox
Copy link
Contributor

Proposed changes

This PR updates the Dockerfile so it is flexible enough to be able to use alternative base images should you wish. The base image still defaults to using the node:slim distro, but other base images can be injected using --build-arg parameters on the command line. For example, to create a container based on Red Hat UBI (Universal Base Image) 8
add BUILDER, DISTRO, PACKAGE_MANAGER and USER parameters as shown:

sudo docker build -t keyrock \
  --build-arg BUILDER=registry.access.redhat.com/ubi8/nodejs-14 \
  --build-arg DISTRO=registry.access.redhat.com/ubi8/nodejs-14-minimal \
  --build-arg PACKAGE_MANAGER=yum \
  --build-arg USER=1001 .

To create a container based on Alpine Linux add BUILDER, DISTRO, PACKAGE_MANAGER and USER parameters as shown:

docker build -t keyrock \
  --build-arg BUILDER=node:16-alpine \
  --build-arg DISTRO=node:16-alpine \
  --build-arg PACKAGE_MANAGER=apk . \
  --build-arg USER=406 . --no-cache

This allows users to upgrade to their preferred Linux distro and helps to reduces Critical Vulnerabilities, and therefore makes the final product more secure.

Types of changes

What types of changes does your code introduce to the project: Put an x in
the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality
    to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.

  • I have read the
    CONTRIBUTING
    doc
  • I have signed the
    CLA
  • I have added tests that prove my fix is effective or that my feature
    works
  • x ] I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream
    modules

Further comments

Similar to ging/fiware-pep-proxy#134 , however Keyrock is more complex. The exisiting Node-Sass dependency is problematic - the version in package.json is not supported in Node:16 and furthermore rebuilding the dependency requires a CXXFLAGS directive.

With Node:16 running npm install results in the following error:

: error: no template named 'remove_cv_t' in namespace
      'std'; did you mean 'remove_cv'?
            !std::is_same<Data, std::remove_cv_t<T>>::value>::Perform(data);
                                ~~~~~^~~~~~~~~~~
                                     remove_cv
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/type_traits:697:50: note: 'remove_cv' declared here
template <class _Tp> struct _LIBCPP_TEMPLATE_VIS remove_cv
                                                 ^
1 error generated.
make: *** [Release/obj.target/binding/src/binding.o] Error 1

With Node:16 running CXXFLAGS="--std=c++14" npm install works fine though with node-saas 7.0.1`

Similar to ging/fiware-pep-proxy#135 the package.json must be updated - a lot of dependencies are out of date, not just node-sass. This PR updates all of them except sequelize which is known to be problematic.

found 35 vulnerabilities (2 low, 19 moderate, 13 high, 1 critical)

which is a significant improvement on

found 83 vulnerabilities (5 low, 46 moderate, 29 high, 3 critical)

For UBI Images run:

```
docker build -t keyrock \
  --build-arg BUILDER=registry.access.redhat.com/ubi8/nodejs-14 \
  --build-arg DISTRO=registry.access.redhat.com/ubi8/nodejs-14-minimal \
  --build-arg PACKAGE_MANAGER=yum \
  --build-arg USER=1001 . --no-cache
```
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2022

CLA Assistant Lite bot All contributors have signed the CLA ✍️


SHELL ["/bin/bash", "-o", "pipefail", "-c"]
# hadolint ignore=DL3002
USER root

ENV PYTHONUNBUFFERED=1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ENV looks very suspicious - I guess it is an artifact from an older python-based version. Should the line be deleted?

Setting PYTHONUNBUFFERED to a non empty value ensures that the python output is sent straight to terminal (e.g. your container log) without being first buffered and that you can see the output of your application (e.g. django logs) in real time.

jason-fox and others added 6 commits January 6, 2022 17:10
`description`,`name` and `summary` are standard UBI `LABELS`. These need to be present in the Dockerfile  for the underlining `LABELS` from the base image are to be overwritten with meaningful descriptions.
- Remove unmaintained node-sass and node-sass-middleware
- Add library to use sass dart and add dependency
- Replace bootstrap-3-sass dependency with updated dart equivalent
@apozohue10
Copy link
Contributor

I have read the CLA Document and I hereby sign the CLA

@apozohue10
Copy link
Contributor

recheckcla

@apozohue10 apozohue10 merged commit 217997b into ging:master Feb 15, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants