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

add terrascan atlantis container files, scripts and doc. #684

Merged
merged 2 commits into from
May 5, 2021

Conversation

devang-gaur
Copy link
Contributor

@devang-gaur devang-gaur commented Apr 22, 2021

@codecov
Copy link

codecov bot commented Apr 22, 2021

Codecov Report

Merging #684 (1eba9c4) into master (585edcc) will decrease coverage by 0.69%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #684      +/-   ##
==========================================
- Coverage   74.56%   73.86%   -0.70%     
==========================================
  Files         110      110              
  Lines        3082     3176      +94     
==========================================
+ Hits         2298     2346      +48     
- Misses        609      652      +43     
- Partials      175      178       +3     
Impacted Files Coverage Δ
pkg/k8s/admission-webhook/validating-webhook.go 62.00% <0.00%> (-18.44%) ⬇️
pkg/k8s/dblogs/webhook-scan-logger.go 70.90% <0.00%> (-7.28%) ⬇️
pkg/iac-providers/kubernetes/v1/load-dir.go 67.74% <0.00%> (-4.68%) ⬇️
pkg/cli/run.go 88.57% <0.00%> (-2.86%) ⬇️
pkg/iac-providers/helm/v3/load-dir.go 82.89% <0.00%> (-2.73%) ⬇️
pkg/iac-providers/terraform/commons/load-dir.go 80.59% <0.00%> (-2.48%) ⬇️
pkg/iac-providers/kustomize/v3/load-dir.go 71.69% <0.00%> (-2.31%) ⬇️
pkg/results/types.go 100.00% <0.00%> (ø)
pkg/http-server/webhook-scan-logs.go 0.00% <0.00%> (ø)
pkg/utils/resource.go
... and 5 more

@amirbenv
Copy link
Contributor

Mind having descriptive filenames? atlantis-entrypoint.sh and atlantis.sh is confusing, same with setup and setup1. Other files can also be clearer (workflow.yaml --> terrascan_workflow,yaml)

Also, maybe consolidate the 3 commands in setup.sh with the Dockerfile?

Lastly we've refactored the docs page, so instead of reuploading the old doc, edit the doc inside /docs/integrations/

@devang-gaur devang-gaur marked this pull request as draft April 23, 2021 01:37
@devang-gaur devang-gaur force-pushed the atlantis_container branch 3 times, most recently from 33e6a8b to 6c2617e Compare April 24, 2021 00:50
@devang-gaur devang-gaur marked this pull request as ready for review April 24, 2021 00:51
@devang-gaur
Copy link
Contributor Author

Mind having descriptive filenames? atlantis-entrypoint.sh and atlantis.sh is confusing, same with setup and setup1. Other files can also be clearer (workflow.yaml --> terrascan_workflow,yaml)

Also, maybe consolidate the 3 commands in setup.sh with the Dockerfile?

Lastly we've refactored the docs page, so instead of reuploading the old doc, edit the doc inside /docs/integrations/

@amirbenv Changes made

@@ -62,7 +61,7 @@ $ atlantis server \
--gh-token="$TOKEN" \
--gh-webhook-secret="$SECRET" \
--repo-allowlist="$REPO_ALLOWLIST" \
--repo-config=terrascan-workflow.yaml
--repo-config=terrascan-terrascan_workflow.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's an extra "terrascan-" in there

Copy link
Contributor

@jlk jlk left a comment

Choose a reason for hiding this comment

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

I don't see anything related to building accurics/terrascan_atlantis or pushing it to docker hub?

ENV DEFAULT_TERRASCAN_VERSION=1.5.0
ENV PLANFILE tfplan
ADD setup.sh terrascan.sh launch-atlantis.sh entrypoint.sh /usr/local/bin/
RUN touch ${PLANFILE} && mkdir -p /etc/atlantis/ && chmod +x /usr/local/bin/setup.sh /usr/local/bin/terrascan.sh /usr/local/bin/launch-atlantis.sh /usr/local/bin/entrypoint.sh && setup.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Break each command onto a separate line after each && to help with readability.

Also - specify the full path to setup.sh

atlantis/entrypoint.sh Show resolved Hide resolved
@devang-gaur devang-gaur marked this pull request as draft April 28, 2021 13:46
@devang-gaur
Copy link
Contributor Author

Blocked on #705

@devang-gaur devang-gaur force-pushed the atlantis_container branch 3 times, most recently from 9c4df1a to 5bd1cf8 Compare May 3, 2021 11:27
@devang-gaur devang-gaur marked this pull request as ready for review May 3, 2021 11:28
@devang-gaur
Copy link
Contributor Author

For verifying the output :

Incase of no violations from terrascan: devang-gaur/atlantis-demo#3 (comment)

(basically no comments from terrascan). The CI passes.

Incase of 1 or more violations from terrascan: devang-gaur/atlantis-demo#3 (comment)

( THE CI WILL BREAK )

To test the container yourself, you can follow along the docs I added on this PR. or try

docker pull devanggaur7/terrascan_atlantis:v0.1.0 

@jlk
Copy link
Contributor

jlk commented May 4, 2021

Besides the comment on the version stuff in the docs, I think this is good to go.

Copy link
Contributor

@jlk jlk left a comment

Choose a reason for hiding this comment

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

Hmm...I as new versions of helm/terraform/kustomize/etc come out, we usually have to do a new release to support, so probably the same for Atlantis?

(sorry for double comment, GH not letting me delete)

docs/integrations/atlantis-integration.md Outdated Show resolved Hide resolved
@devang-gaur devang-gaur requested a review from amirbenv May 4, 2021 14:59
@sonarcloud
Copy link

sonarcloud bot commented May 4, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@devang-gaur
Copy link
Contributor Author

@jlk I updated the doc once more. please check

Copy link
Contributor

@jlk jlk left a comment

Choose a reason for hiding this comment

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

LGTM

@devang-gaur devang-gaur merged commit 0c7f4ca into tenable:master May 5, 2021
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