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

[chore] Update node to fix docker image vulnerabilities #136

Merged
merged 4 commits into from
Oct 13, 2023

Conversation

w1kman
Copy link
Contributor

@w1kman w1kman commented Oct 12, 2023

This PR aims to fix vulnerabilities inherited by node:16-slim image.

image

  • Node v16 => v18
  • CLI/Docker
    • Added -s, --stdout so that docker image can be used without mounting a volume
    • entrypoint defaults to be using --stdout
  • Docs
    • Updated README.md to include docker usage

How to test

  • Build docker image and run it as shown in README.md
    • Build: docker build --tag har-to-k6:local .
    • Run: docker run har-to-k6:local ./example/inconsistent-content-type.har > loadtest.js
      • Expected result is a loadtest.js file on the host system
    • Run: docker run har-to-k6:local -o loadtest.js ./example/inconsistent-content-type.har
      • Expected result is a loadtest.js file inside the container (old default).
    • Note: warnings and errors should not end up in the test script
  • Run ./bin/har-to-k6 with -stdout and --output in different combinations to make sure that we get a desired result

- Add possibility to output to stdout, instead of just file
- Update webpack
- Add `--stdout` option
Copy link
Contributor Author

@w1kman w1kman left a comment

Choose a reason for hiding this comment

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

Self-review

@@ -7,4 +7,4 @@ COPY . .
RUN npm install
RUN npm run bundle

ENTRYPOINT ["node", "bin/har-to-k6.js"]
ENTRYPOINT ["node", "bin/har-to-k6.js", "--stdout"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user would add option -o, --output <filename>, output would not be sent to stdout

package.json Outdated Show resolved Hide resolved
Comment on lines +7 to +11
resolve: {
fallback: {
fs: false,
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change required when updating webpack

@w1kman w1kman marked this pull request as ready for review October 12, 2023 06:13
@w1kman w1kman requested a review from a team as a code owner October 12, 2023 06:13
@w1kman w1kman requested review from allansson and e-fisher and removed request for a team October 12, 2023 06:13
@w1kman w1kman changed the title [chore] Update node to fix docker image vunerabilities [chore] Update node to fix docker image vulnerabilities Oct 12, 2023
Copy link
Contributor Author

@w1kman w1kman left a comment

Choose a reason for hiding this comment

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

self-review#2

Copy link
Contributor

@allansson allansson left a comment

Choose a reason for hiding this comment

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

Works on my computer. 👍

Comment on lines +59 to +61
"webpack": "^5.88.2",
"webpack-bundle-analyzer": "^4.9.1",
"webpack-cli": "^5.1.4"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you no change to parceljs? 😞

Copy link
Contributor

@e-fisher e-fisher left a comment

Choose a reason for hiding this comment

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

LGTM

@w1kman w1kman merged commit 48e3db3 into master Oct 13, 2023
2 checks passed
@w1kman w1kman deleted the chore/npm-audit-on-20231009 branch October 13, 2023 05:18
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.

None yet

3 participants