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

feat: add dockerfile and script to perform fuzzing test on all swagger files and individual #4569

Merged
merged 6 commits into from
Aug 15, 2023

Conversation

vli11
Copy link
Contributor

@vli11 vli11 commented May 16, 2023

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)

Testing Instructions

to build: docker build -f Dockerfile.fuzz -t fuzz-edgex-go:latest .
to run: docker run --net host -v $(pwd)/fuzz_results:/fuzz_results fuzz-edgex-go:latest

New Dependency Instructions (If applicable)

@vli11 vli11 marked this pull request as draft May 16, 2023 18:29
@vli11 vli11 self-assigned this May 16, 2023
@vli11 vli11 force-pushed the fuzz_docker_script branch from 832011d to 67c93f7 Compare May 16, 2023 18:31
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Dockerfile.fuzz Outdated Show resolved Hide resolved
fuzzing_docker.sh Outdated Show resolved Hide resolved
fuzzing_docker.sh Outdated Show resolved Hide resolved
fuzzing_docker.sh Outdated Show resolved Hide resolved
@bnevis-i bnevis-i self-requested a review August 9, 2023 16:47
@vli11 vli11 force-pushed the fuzz_docker_script branch from 68fae54 to 807f438 Compare August 12, 2023 00:56
@vli11 vli11 changed the title feat: core-data fuzzing dockerfile and script feat: add dockerfile and script to perform fuzzing test on all swagger files and individual Aug 12, 2023
@vli11 vli11 added the enhancement New feature or request label Aug 12, 2023
@vli11 vli11 requested a review from lenny-goodell August 12, 2023 01:00
@vli11 vli11 marked this pull request as ready for review August 12, 2023 01:00
Dockerfile.fuzz Outdated

RUN apk add --no-cache python3 py3-pip bash coreutils

RUN wget https://github.com/microsoft/restler-fuzzer/archive/refs/tags/v9.2.2.tar.gz
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lenny-goodell
Copy link
Member

@vli11 , please add make targets for building and running .

  • docker-fuzz
    • docker build -f Dockerfile.fuzz -t fuzz-edgex-go:latest .
  • fuxx-test
    • docker run --net host -v $(pwd)/fuzz_results:/fuzz_results fuzz-edgex-go:latest

Dockerfile.fuzz Outdated Show resolved Hide resolved
fuzzing_docker.sh Outdated Show resolved Hide resolved
Dockerfile.fuzz Outdated
Comment on lines 5 to 6
RUN wget -q "https://github.com/microsoft/restler-fuzzer/archive/refs/tags/v9.2.2.tar.gz"
RUN tar -xvf v9.2.2.tar.gz && mv restler-fuzzer-9.2.2 restler-fuzzer
Copy link
Contributor

Choose a reason for hiding this comment

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

combine these two runs and delete gz file after mv

Copy link
Collaborator

@bnevis-i bnevis-i Aug 14, 2023

Choose a reason for hiding this comment

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

Actually, it is more efficient to just pipe the wget output into tar directly, and use tar options to manipulate the target directory

WORKDIR /restler-fuzzer
RUN wget -q -O - (url.tar.gz) | tar xz --strip-components 1 && mkdir restler_bin`

Copy link
Contributor

Choose a reason for hiding this comment

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

that's great

Dockerfile.fuzz Outdated
Comment on lines 8 to 22
RUN cd restler-fuzzer; mkdir -p restler_bin
WORKDIR /restler-fuzzer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
RUN cd restler-fuzzer; mkdir -p restler_bin
WORKDIR /restler-fuzzer

Combined into previous statement

Dockerfile.fuzz Outdated Show resolved Hide resolved
Dockerfile.fuzz Outdated
RUN wget -q "https://github.com/microsoft/restler-fuzzer/archive/refs/tags/v9.2.2.tar.gz" && \
tar -xvf v9.2.2.tar.gz && \
mv restler-fuzzer-9.2.2 restler-fuzzer && \
rm v9.2.2.tar.gz && \
cd restler-fuzzer && \
mkdir -p restler_bin

WORKDIR /restler-fuzzer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
RUN wget -q "https://github.com/microsoft/restler-fuzzer/archive/refs/tags/v9.2.2.tar.gz" && \
tar -xvf v9.2.2.tar.gz && \
mv restler-fuzzer-9.2.2 restler-fuzzer && \
rm v9.2.2.tar.gz && \
cd restler-fuzzer && \
mkdir -p restler_bin
WORKDIR /restler-fuzzer
WORKDIR /restler-fuzzer
RUN wget -q -O - "https://github.com/microsoft/restler-fuzzer/archive/refs/tags/v9.2.2.tar.gz" | \
tar xvf - --strip-components 1 && \
mkdir -p restler_bin

fuzzing_docker.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@bnevis-i bnevis-i dismissed their stale review August 14, 2023 18:56

Dismissing my stale review.

@bnevis-i
Copy link
Collaborator

@vli11 You will have to squash and rebase this PR due to the Semantic PR check failing.

bnevis-i
bnevis-i previously approved these changes Aug 14, 2023
feat: add dockerfile and script to perform fuzzing test on all swagger files and individual

Closes: edgexfoundry#4568
Signed-off-by: Valina Li <[email protected]>
bnevis-i
bnevis-i previously approved these changes Aug 14, 2023
Copy link
Collaborator

@bnevis-i bnevis-i left a comment

Choose a reason for hiding this comment

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

Re-approved.

jim-wang-intel
jim-wang-intel previously approved these changes Aug 14, 2023
Copy link
Contributor

@jim-wang-intel jim-wang-intel left a comment

Choose a reason for hiding this comment

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

LGTM, except make test failed

@lenny-goodell
Copy link
Member

LGTM, except make test failed

@vli11 , Hado lint failing on your new docker file.

Dockerfile.fuzz Outdated Show resolved Hide resolved
Dockerfile.fuzz Outdated Show resolved Hide resolved
Dockerfile.fuzz Outdated Show resolved Hide resolved
Dockerfile.fuzz Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
fuzzing_docker.sh Outdated Show resolved Hide resolved
@vli11
Copy link
Contributor Author

vli11 commented Aug 15, 2023

LGTM, except make test failed

@vli11 , Hado lint failing on your new docker file.

FROM mcr.microsoft.com/dotnet/sdk:6.0-alpine hadolint error: DL3026 error: Use only an allowed registry in the FROM image; but docker hub does have this official image https://hub.docker.com/_/microsoft-dotnet-sdk/https://hub.docker.com/_/microsoft-dotnet-sdk/. any suggestions? @lenny-intel @bnevis-i

@bnevis-i
Copy link
Collaborator

FROM mcr.microsoft.com/dotnet/sdk:6.0-alpine hadolint error: DL3026 error: Use only an allowed registry in the FROM image; but docker hub does have this official image https://hub.docker.com/_/microsoft-dotnet-sdk/https://hub.docker.com/_/microsoft-dotnet-sdk/. any suggestions? @lenny-intel @bnevis-i

I would ignore the error in .hadolint.yaml. https://github.com/edgexfoundry/edgex-go/blob/main/.hadolint.yml

@vli11 vli11 dismissed stale reviews from jim-wang-intel and bnevis-i via 323adb0 August 15, 2023 21:22
@jim-wang-intel
Copy link
Contributor

LGTM, except make test failed

@vli11 , Hado lint failing on your new docker file.

FROM mcr.microsoft.com/dotnet/sdk:6.0-alpine hadolint error: DL3026 error: Use only an allowed registry in the FROM image; but docker hub does have this official image https://hub.docker.com/_/microsoft-dotnet-sdk/https://hub.docker.com/_/microsoft-dotnet-sdk/. any suggestions? @lenny-intel @bnevis-i

i wonder what is the reason that Microsoft doesn't publish the latest version to Docker hub. maybe the one in docker hub is stable version

@bnevis-i
Copy link
Collaborator

i wonder what is the reason that Microsoft doesn't publish the latest version to Docker hub. maybe the one in docker hub is stable version

Because Microsoft has the better registry? (Probably true, it supports the latest OCI registry standards.)

bnevis-i
bnevis-i previously approved these changes Aug 15, 2023
Copy link
Collaborator

@bnevis-i bnevis-i left a comment

Choose a reason for hiding this comment

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

LGTM. Third time lucky?

Makefile Outdated Show resolved Hide resolved
.hadolint.yml Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@jim-wang-intel jim-wang-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@vli11 vli11 requested a review from bnevis-i August 15, 2023 22:10
@bnevis-i bnevis-i removed their request for review August 15, 2023 22:11
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

4 participants