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

changes for argocd integration #724

Merged

Conversation

patilpankaj212
Copy link
Contributor

  1. modify Dockerfile to add a folder .ssh in /home/terrascan. This folder will be used to place ssh keys, ssh config and known_hosts required for api server to download repositories.
  2. modify the resources skipping for kubernetes
  3. modify response code of api server's remote repo scan response, when the result contains admission controller denied violations.

@codecov
Copy link

codecov bot commented May 3, 2021

Codecov Report

Merging #724 (5f8c83d) into master (02c8bce) will decrease coverage by 0.68%.
The diff coverage is 78.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #724      +/-   ##
==========================================
- Coverage   73.86%   73.18%   -0.69%     
==========================================
  Files         110      110              
  Lines        3176     3170       -6     
==========================================
- Hits         2346     2320      -26     
- Misses        652      668      +16     
- Partials      178      182       +4     
Impacted Files Coverage Δ
pkg/http-server/remote-repo.go 69.69% <66.66%> (+0.05%) ⬆️
pkg/iac-providers/kubernetes/v1/normalize.go 88.05% <100.00%> (-2.31%) ⬇️
pkg/k8s/admission-webhook/validating-webhook.go 62.00% <100.00%> (ø)
pkg/results/store.go 46.15% <0.00%> (-23.08%) ⬇️
pkg/policy/opa/engine.go 65.56% <0.00%> (-5.81%) ⬇️

@xortim xortim mentioned this pull request May 3, 2021
Copy link
Contributor

@devang-gaur devang-gaur left a comment

Choose a reason for hiding this comment

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

One very minor change requested.

skipRules := make([]output.SkipRule, 0)
err := json.Unmarshal([]byte(rules), &skipRules)
if err != nil {
zap.S().Debugf("terrascanSkip rules '%s' cannot be unmarshalled to json", rules)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
zap.S().Debugf("terrascanSkip rules '%s' cannot be unmarshalled to json", rules)
zap.S().Debugf("json string %s cannot be unmarshalled to [ ]output.SkipRules struct schema", rules)

Copy link
Contributor

Choose a reason for hiding this comment

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

Devang's comment is technically accurate, but not sure if that much detail would be useful to average user, or confusing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @jlk. This change will not be informative to most users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Debug is also used by us developers.. to diagnose what went wrong. Will ease it for us to help people reporting bugs and other issues.

jlk
jlk previously approved these changes May 4, 2021
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.

Generally LGTM. Note the 2 comments, but not requiring any changes...

build/Dockerfile Show resolved Hide resolved
skipRules := make([]output.SkipRule, 0)
err := json.Unmarshal([]byte(rules), &skipRules)
if err != nil {
zap.S().Debugf("terrascanSkip rules '%s' cannot be unmarshalled to json", rules)
Copy link
Contributor

Choose a reason for hiding this comment

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

Devang's comment is technically accurate, but not sure if that much detail would be useful to average user, or confusing?

@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
0.0% 0.0% Duplication

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.

Ship it! :)

@devang-gaur devang-gaur merged commit 881417a into tenable:master May 5, 2021
@patilpankaj212 patilpankaj212 deleted the argocd-integration-changes branch May 5, 2022 11:37
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

4 participants