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.

Sign up for GitHub

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

Jump to bottom

fix(docker): Checkov installation silently fails on docker build in arm64. Workaround till issue will be fixed in checkov itself #635

Merged
merged 20 commits into from
Mar 11, 2024
Merged

fix(docker): Checkov installation silently fails on docker build in arm64. Workaround till issue will be fixed in checkov itself #635

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5cf0ca5
fix-workaround: Checkov install fails aarch64. Awaiting checkov versi…
antm-pp Feb 23, 2024
63f4bef
fix: Docker Checkov - Keep libgcc and remove gcc for compatibility wi…
antm-pp Feb 23, 2024
7014fa0
fix:checkob install - pinned apk versions
antm-pp Feb 23, 2024
3d85490
Add comments on package dependencies and re-order
antm-pp Feb 23, 2024
a13a983
fix: Docker Checkov install: Added gcc container check
antm-pp Feb 23, 2024
42d77d9
fix: Docker checkov install: reorder values and comments on package i…
antm-pp Feb 24, 2024
f46e8f8
fix: Docker Checkov Install: Container structure test: regex excape c…
antm-pp Feb 24, 2024
9b58092
Update .github/.container-structure-test-config.yaml
antm-pp Feb 24, 2024
3e0d679
fix: Docker Checkov Install: Use git fetch for crates for consistency…
antm-pp Feb 24, 2024
b5c6379
Merge branch 'master' into fix/docker-checkov-install-aarch64
MaxymVlasov Feb 26, 2024
54e5ea2
fix(dockerfile): Deal with "successful silent fails" during `docker b…
MaxymVlasov Mar 7, 2024
8798d3b
fix Checkov?
MaxymVlasov Mar 7, 2024
c3322d0
Try use checkov binary. Checkov still needs all deps installed by pip
MaxymVlasov Mar 7, 2024
875999c
Revert "Try use checkov binary. Checkov still needs all deps installe…
MaxymVlasov Mar 7, 2024
3481201
Revert "fix Checkov?"
MaxymVlasov Mar 7, 2024
c4c1b16
Merge remote-tracking branch 'upstream/fix/dockerfile' into fix/docke…
MaxymVlasov Mar 7, 2024
2f00dcf
fix silent fail
MaxymVlasov Mar 7, 2024
5e98ae3
test
MaxymVlasov Mar 7, 2024
1ca976a
Apply suggestions from code review
MaxymVlasov Mar 8, 2024
01d0868
Apply suggestions from code review
MaxymVlasov Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/.container-structure-test-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ commandTests:
args: ["-version"]
expectedOutput: ["^Terraform v([0-9]+\\.){2}[0-9]+\\non linux_amd64\\n$"]

- name: "gcc"
command: "gcc"
args: ["--version"]
expectedOutput: ["^gcc \\(Alpine 12\\."]

- name: "checkov"
command: "checkov"
args: ["--version"]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-image-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: "Build Dockerfile if changed and run smoke tests"

on: [pull_request]
on: push
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

env:
IMAGE_TAG: pr-test
Expand Down
73 changes: 49 additions & 24 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ ARG PRE_COMMIT_VERSION=${PRE_COMMIT_VERSION:-latest}
ARG TERRAFORM_VERSION=${TERRAFORM_VERSION:-latest}

# Install pre-commit
RUN [ ${PRE_COMMIT_VERSION} = "latest" ] && pip3 install --no-cache-dir pre-commit \
|| pip3 install --no-cache-dir pre-commit==${PRE_COMMIT_VERSION}
RUN if [ ${PRE_COMMIT_VERSION} = "latest" ]; \
then pip3 install --no-cache-dir pre-commit; \
else pip3 install --no-cache-dir pre-commit==${PRE_COMMIT_VERSION}; \
fi

# Install terraform because pre-commit needs it
RUN if [ "${TERRAFORM_VERSION}" = "latest" ]; then \
Expand Down Expand Up @@ -66,10 +68,15 @@ RUN if [ "$INSTALL_ALL" != "false" ]; then \
RUN . /.env && \
if [ "$CHECKOV_VERSION" != "false" ]; then \
( \
apk add --no-cache gcc=~12 libffi-dev=~3 musl-dev=~1; \
[ "$CHECKOV_VERSION" = "latest" ] && pip3 install --no-cache-dir checkov \
|| pip3 install --no-cache-dir checkov==${CHECKOV_VERSION}; \
apk del gcc libffi-dev musl-dev \
# cargo, gcc, git, musl-dev, rust and CARGO envvar required for compilation of [email protected], no longer required once checkov version depends on rustworkx >0.14.0
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
# gcc libffi-dev musl-dev required for compilation of cffi, until it contains musl aarch64
export CARGO_NET_GIT_FETCH_WITH_CLI=true && \
apk add --no-cache cargo=~1 gcc=~12 git=~2 libffi-dev=~3 libgcc=~12 musl-dev=~1 rust=~1 ; \
if [ "$CHECKOV_VERSION" = "latest" ]; \
then pip3 install --no-cache-dir checkov || exit 1; \
else pip3 install --no-cache-dir checkov==${CHECKOV_VERSION} || exit 1; \
Comment on lines +77 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

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

exit 1 prevents silent fail of error: can't find Rust compiler

When the rust compiler call fails it generates a false
#635 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

btw, we can't just add comments inside the code about it. Probably, the best place is on L73

Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of interest why at # Install pre-commit code block (lines 20-23) pip3 has no || exit 1 bits? Is ii intentional?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because pre-commit doesn't use rust?

I prefer somehow make structured tests work for arm64, rather than add exit 1 in every possible place

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because pre-commit doesn't use rust?

That's kind of weird: if pip3 install ... fails (as in returns non-zero code) then exit the shell with non-successful code (exit 1). And this is implemented for checkov install only, which means it's okay to not exit for the same e.g. with pre-commit install by pip. Does pip behave differently if it fails to install pre-commit? 🤔 Aint't Docker's RUN imply somewhat set -e? Should we try and add set -e to each RUN to explicitly let Docker RUN exit when any downstream command fails within if/else statement? 🤔 I'm a bit lost to be honest 😲 We definitely should use consistent solution for all similar expressions for consistency

Copy link
Collaborator

@yermulnik yermulnik Mar 11, 2024 •
edited

Choose a reason for hiding this comment

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

I'll rewrite it to bash scripts.

What do you think of a single script that takes args? Like install_deps.sh pre-commit, install_deps.sh checkov, install_deps.sh pre-commit checkov ..., or install_deps.sh ALL? Just to keep code in the same place and have re-usable snippets (like shell funcxtions) 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Problem there that they are different

Copy link
Collaborator

Choose a reason for hiding this comment

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

currently, I just moving it out and implementing logic as

COPY tools/install/ /install/

WORKDIR /bin_dir

RUN /install/pre-commit.sh
RUN /install/foo.sh

We can discuss it in next PR :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Problem there that they are different

Some are slightly different, whilst others use the same solutions (like extracting download URLs from GH releases API response). And also I see it odd to have a bunch of almost-the-same ten-lines shell scripts instead of one that can handle installation of all the deps one-by-one or all-at-once.
On the other hand such approach with single script would negatively impact build caching and each RUN layer would get rebuilt if the file is updated with a change to the installation steps of a specific dep =(
From this point if view I'd better stay with the current approach =)

We can discuss it in next PR :)

Makes sense 🤝 (apologies that I already outlined my thoughts — this helps me imprint them in memory 😺)

Copy link

Choose a reason for hiding this comment

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

@MaxymVlasov do not install distributions with pip separately because it will not take all things installed in the previous session into account when you run a new install command. Enumerate everything and let the dependency resolver know all your requirements together. Ideally, use pip-compile to produce and commit constraint files (lockfiles) and invoke it via pip install -r direct-deps.txt -c constraint.txt.

If you do run separate pip installs, inject a pip check invocation at the end to verify integrity.

fi; \
apk del cargo gcc git libffi-dev musl-dev rust \
) \
; fi

Expand All @@ -78,8 +85,10 @@ RUN . /.env && \
if [ "$INFRACOST_VERSION" != "false" ]; then \
( \
INFRACOST_RELEASES="https://api.github.com/repos/infracost/infracost/releases" && \
[ "$INFRACOST_VERSION" = "latest" ] && curl -L "$(curl -s ${INFRACOST_RELEASES}/latest | grep -o -E -m 1 "https://.+?-${TARGETOS}-${TARGETARCH}.tar.gz")" > infracost.tgz \
|| curl -L "$(curl -s ${INFRACOST_RELEASES} | grep -o -E "https://.+?v${INFRACOST_VERSION}/infracost-${TARGETOS}-${TARGETARCH}.tar.gz")" > infracost.tgz \
if [ "$INFRACOST_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${INFRACOST_RELEASES}/latest | grep -o -E -m 1 "https://.+?-${TARGETOS}-${TARGETARCH}.tar.gz")" > infracost.tgz; \
else curl -L "$(curl -s ${INFRACOST_RELEASES} | grep -o -E "https://.+?v${INFRACOST_VERSION}/infracost-${TARGETOS}-${TARGETARCH}.tar.gz")" > infracost.tgz; \
fi; \
) && tar -xzf infracost.tgz && rm infracost.tgz && mv infracost-${TARGETOS}-${TARGETARCH} infracost \
; fi

Expand All @@ -88,8 +97,10 @@ RUN . /.env && \
if [ "$TERRAFORM_DOCS_VERSION" != "false" ]; then \
( \
TERRAFORM_DOCS_RELEASES="https://api.github.com/repos/terraform-docs/terraform-docs/releases" && \
[ "$TERRAFORM_DOCS_VERSION" = "latest" ] && curl -L "$(curl -s ${TERRAFORM_DOCS_RELEASES}/latest | grep -o -E -m 1 "https://.+?-${TARGETOS}-${TARGETARCH}.tar.gz")" > terraform-docs.tgz \
|| curl -L "$(curl -s ${TERRAFORM_DOCS_RELEASES} | grep -o -E "https://.+?v${TERRAFORM_DOCS_VERSION}-${TARGETOS}-${TARGETARCH}.tar.gz")" > terraform-docs.tgz \
if [ "$TERRAFORM_DOCS_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${TERRAFORM_DOCS_RELEASES}/latest | grep -o -E -m 1 "https://.+?-${TARGETOS}-${TARGETARCH}.tar.gz")" > terraform-docs.tgz; \
else curl -L "$(curl -s ${TERRAFORM_DOCS_RELEASES} | grep -o -E "https://.+?v${TERRAFORM_DOCS_VERSION}-${TARGETOS}-${TARGETARCH}.tar.gz")" > terraform-docs.tgz; \
fi; \
) && tar -xzf terraform-docs.tgz terraform-docs && rm terraform-docs.tgz && chmod +x terraform-docs \
; fi

Expand All @@ -98,8 +109,10 @@ RUN . /.env \
&& if [ "$TERRAGRUNT_VERSION" != "false" ]; then \
( \
TERRAGRUNT_RELEASES="https://api.github.com/repos/gruntwork-io/terragrunt/releases" && \
[ "$TERRAGRUNT_VERSION" = "latest" ] && curl -L "$(curl -s ${TERRAGRUNT_RELEASES}/latest | grep -o -E -m 1 "https://.+?/terragrunt_${TARGETOS}_${TARGETARCH}")" > terragrunt \
|| curl -L "$(curl -s ${TERRAGRUNT_RELEASES} | grep -o -E -m 1 "https://.+?v${TERRAGRUNT_VERSION}/terragrunt_${TARGETOS}_${TARGETARCH}")" > terragrunt \
if [ "$TERRAGRUNT_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${TERRAGRUNT_RELEASES}/latest | grep -o -E -m 1 "https://.+?/terragrunt_${TARGETOS}_${TARGETARCH}")" > terragrunt; \
else curl -L "$(curl -s ${TERRAGRUNT_RELEASES} | grep -o -E -m 1 "https://.+?v${TERRAGRUNT_VERSION}/terragrunt_${TARGETOS}_${TARGETARCH}")" > terragrunt; \
fi; \
) && chmod +x terragrunt \
; fi

Expand All @@ -112,8 +125,10 @@ RUN . /.env && \
OS="$(echo ${TARGETOS} | cut -c1 | tr '[:lower:]' '[:upper:]' | xargs echo -n; echo ${TARGETOS} | cut -c2-)"; \
( \
TERRASCAN_RELEASES="https://api.github.com/repos/tenable/terrascan/releases" && \
[ "$TERRASCAN_VERSION" = "latest" ] && curl -L "$(curl -s ${TERRASCAN_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${OS}_${ARCH}.tar.gz")" > terrascan.tar.gz \
|| curl -L "$(curl -s ${TERRASCAN_RELEASES} | grep -o -E "https://.+?${TERRASCAN_VERSION}_${OS}_${ARCH}.tar.gz")" > terrascan.tar.gz \
if [ "$TERRASCAN_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${TERRASCAN_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${OS}_${ARCH}.tar.gz")" > terrascan.tar.gz; \
else curl -L "$(curl -s ${TERRASCAN_RELEASES} | grep -o -E "https://.+?${TERRASCAN_VERSION}_${OS}_${ARCH}.tar.gz")" > terrascan.tar.gz; \
fi; \
) && tar -xzf terrascan.tar.gz terrascan && rm terrascan.tar.gz && \
./terrascan init \
; fi
Expand All @@ -123,8 +138,10 @@ RUN . /.env && \
if [ "$TFLINT_VERSION" != "false" ]; then \
( \
TFLINT_RELEASES="https://api.github.com/repos/terraform-linters/tflint/releases" && \
[ "$TFLINT_VERSION" = "latest" ] && curl -L "$(curl -s ${TFLINT_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${TARGETOS}_${TARGETARCH}.zip")" > tflint.zip \
|| curl -L "$(curl -s ${TFLINT_RELEASES} | grep -o -E "https://.+?/v${TFLINT_VERSION}/tflint_${TARGETOS}_${TARGETARCH}.zip")" > tflint.zip \
if [ "$TFLINT_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${TFLINT_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${TARGETOS}_${TARGETARCH}.zip")" > tflint.zip; \
else curl -L "$(curl -s ${TFLINT_RELEASES} | grep -o -E "https://.+?/v${TFLINT_VERSION}/tflint_${TARGETOS}_${TARGETARCH}.zip")" > tflint.zip; \
fi; \
) && unzip tflint.zip && rm tflint.zip \
; fi

Expand All @@ -133,8 +150,10 @@ RUN . /.env && \
if [ "$TFSEC_VERSION" != "false" ]; then \
( \
TFSEC_RELEASES="https://api.github.com/repos/aquasecurity/tfsec/releases" && \
[ "$TFSEC_VERSION" = "latest" ] && curl -L "$(curl -s ${TFSEC_RELEASES}/latest | grep -o -E -m 1 "https://.+?/tfsec-${TARGETOS}-${TARGETARCH}")" > tfsec \
|| curl -L "$(curl -s ${TFSEC_RELEASES} | grep -o -E -m 1 "https://.+?v${TFSEC_VERSION}/tfsec-${TARGETOS}-${TARGETARCH}")" > tfsec \
if [ "$TFSEC_VERSION" = "latest" ]; then \
curl -L "$(curl -s ${TFSEC_RELEASES}/latest | grep -o -E -m 1 "https://.+?/tfsec-${TARGETOS}-${TARGETARCH}")" > tfsec; \
else curl -L "$(curl -s ${TFSEC_RELEASES} | grep -o -E -m 1 "https://.+?v${TFSEC_VERSION}/tfsec-${TARGETOS}-${TARGETARCH}")" > tfsec; \
fi; \
) && chmod +x tfsec \
; fi

Expand All @@ -144,8 +163,10 @@ RUN . /.env && \
if [ "$TARGETARCH" != "amd64" ]; then ARCH="$TARGETARCH"; else ARCH="64bit"; fi; \
( \
TRIVY_RELEASES="https://api.github.com/repos/aquasecurity/trivy/releases" && \
[ "$TRIVY_VERSION" = "latest" ] && curl -L "$(curl -s ${TRIVY_RELEASES}/latest | grep -o -E -i -m 1 "https://.+?/trivy_.+?_${TARGETOS}-${ARCH}.tar.gz")" > trivy.tar.gz \
|| curl -L "$(curl -s ${TRIVY_RELEASES} | grep -o -E -i -m 1 "https://.+?/v${TRIVY_VERSION}/trivy_.+?_${TARGETOS}-${ARCH}.tar.gz")" > trivy.tar.gz \
if [ "$TRIVY_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${TRIVY_RELEASES}/latest | grep -o -E -i -m 1 "https://.+?/trivy_.+?_${TARGETOS}-${ARCH}.tar.gz")" > trivy.tar.gz; \
else curl -L "$(curl -s ${TRIVY_RELEASES} | grep -o -E -i -m 1 "https://.+?/v${TRIVY_VERSION}/trivy_.+?_${TARGETOS}-${ARCH}.tar.gz")" > trivy.tar.gz; \
fi; \
) && tar -xzf trivy.tar.gz trivy && rm trivy.tar.gz \
; fi

Expand All @@ -154,8 +175,10 @@ RUN . /.env && \
if [ "$TFUPDATE_VERSION" != "false" ]; then \
( \
TFUPDATE_RELEASES="https://api.github.com/repos/minamijoyo/tfupdate/releases" && \
[ "$TFUPDATE_VERSION" = "latest" ] && curl -L "$(curl -s ${TFUPDATE_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${TARGETOS}_${TARGETARCH}.tar.gz")" > tfupdate.tgz \
|| curl -L "$(curl -s ${TFUPDATE_RELEASES} | grep -o -E -m 1 "https://.+?${TFUPDATE_VERSION}_${TARGETOS}_${TARGETARCH}.tar.gz")" > tfupdate.tgz \
if [ "$TFUPDATE_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${TFUPDATE_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${TARGETOS}_${TARGETARCH}.tar.gz")" > tfupdate.tgz; \
else curl -L "$(curl -s ${TFUPDATE_RELEASES} | grep -o -E -m 1 "https://.+?${TFUPDATE_VERSION}_${TARGETOS}_${TARGETARCH}.tar.gz")" > tfupdate.tgz; \
fi; \
) && tar -xzf tfupdate.tgz tfupdate && rm tfupdate.tgz \
; fi

Expand All @@ -164,8 +187,10 @@ RUN . /.env && \
if [ "$HCLEDIT_VERSION" != "false" ]; then \
( \
HCLEDIT_RELEASES="https://api.github.com/repos/minamijoyo/hcledit/releases" && \
[ "$HCLEDIT_VERSION" = "latest" ] && curl -L "$(curl -s ${HCLEDIT_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${TARGETOS}_${TARGETARCH}.tar.gz")" > hcledit.tgz \
|| curl -L "$(curl -s ${HCLEDIT_RELEASES} | grep -o -E -m 1 "https://.+?${HCLEDIT_VERSION}_${TARGETOS}_${TARGETARCH}.tar.gz")" > hcledit.tgz \
if [ "$HCLEDIT_VERSION" = "latest" ]; \
then curl -L "$(curl -s ${HCLEDIT_RELEASES}/latest | grep -o -E -m 1 "https://.+?_${TARGETOS}_${TARGETARCH}.tar.gz")" > hcledit.tgz; \
else curl -L "$(curl -s ${HCLEDIT_RELEASES} | grep -o -E -m 1 "https://.+?${HCLEDIT_VERSION}_${TARGETOS}_${TARGETARCH}.tar.gz")" > hcledit.tgz; \
fi; \
) && tar -xzf hcledit.tgz hcledit && rm hcledit.tgz \
; fi

Expand Down