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 terraform_checkov, that run per folder. Deprecate checkov hook #290

Merged

Conversation

bmbferreira
Copy link
Contributor

@bmbferreira bmbferreira commented Nov 30, 2021

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

This change applies the same logic to checkov that is already being used for other tools, such as tflint. If we have a monorepo with multiple modules such as module1 and module2 and we run for example pre-commit run --files module1/istio/*, it would run checkov for all the modules but tflint would just run for module1, which is inconsistent with the argument that was passed to pre-commit.

How has this code been tested

In a monorepo with multiple modules, before if you ran pre-commit run --files module1/istio/*, checkov would execute for all the modules because of the -d . while other tools such as tflint would just execute for the paths passed as argument to pre-commit. With this change, checkov will start having the same behaviour as other tools, improving the consistency between the different tools.

@MaxymVlasov MaxymVlasov added estimate/1h Need 1 hour to be done hook/terraform_checkov Bash hook labels Dec 8, 2021
@MaxymVlasov MaxymVlasov self-assigned this Dec 8, 2021
@MaxymVlasov MaxymVlasov added breaking-changes The current implementation contains Breaking Changes / Can't be implemented without Breaking Changes estimate/2h Need 2 hours to be done and removed estimate/1h Need 1 hour to be done labels Dec 8, 2021
Copy link
Collaborator

@MaxymVlasov MaxymVlasov left a comment

Choose a reason for hiding this comment

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

checkov used in 176 OSS projects.
So we can't merge this PR as is before 2.0.0 version.

I suggest adding a new Checkov hook, like checkov_2, and marking checkov as deprecated with the note "Will be removed in 2.0.0. Please, use checkov_2" (and in 2.0.0 rename checkov_2 to checkov)

@antonbabenko what do you think?

@antonbabenko
Copy link
Owner

antonbabenko commented Dec 8, 2021

Can we achieve backward compatibility with both checkov versions 1 and 2? From what I see, it is possible. It is rather a bad idea to ask people to update anything unless it is absolutely necessary when we bump to version 2 of this repo.

terraform_checkov.sh Outdated Show resolved Hide resolved
@yermulnik
Copy link
Collaborator

yermulnik commented Dec 8, 2021

It is rather a bad idea to ask people to update anything unless it is absolutely necessary when we bump version 2 of this repo.

@MaxymVlasov Do I get it right that the backward incompatible thing with the new hook would be the args parameter which is passed to hook via .pre-commit-config.yaml like in the README here https://github.com/antonbabenko/pre-commit-terraform#checkov ? Or am I missing something else?
If it's about args only I'd guess that most probably vast majority of those OSS repos don't pass custom args to checkov hook, and even those who do, most probably, have pinned their config to a specific version tag of pre-commit-terraform, hence I 100% agree with your suggestion about decommissioning legacy check with v2 of pre-commit-terraform with the understanding that those few who pass custom args to checkov and who don't pin to version tag, are smart enough to surf to this repo and read change log, explore issues, and re-read the readme.
@antonbabenko FYI. I don't think it's worth of implementing quirks to add compatibility of the two checkov hook versions as of what I wrote above.

UPD: especially having that people should know that major version is something to pay a very close attention to.

@MaxymVlasov MaxymVlasov added the feature New feature or request label Dec 9, 2021
@MaxymVlasov
Copy link
Collaborator

On my test infra repo:

took 14s

repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: v1.60.0
  hooks:
  - id: checkov

took 4m 20s

repos:
- repo: [email protected]:bmbferreira/pre-commit-terraform.git
  rev: 9885aaf # latest commit in this PR
  hooks:
  - id: checkov

Something wrong with the hook implementation in this PR. It shouldn't take so much time.

Copy link
Collaborator

@MaxymVlasov MaxymVlasov left a comment

Choose a reason for hiding this comment

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

Performance issue detected.
To fix it, you need to find an error in logic. It is based on how the checkov works

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Dec 9, 2021

@yermulnik yeah, it's about args: section only

I checked all +- related 84 OSS results and found that args used in 16 repos by 7 folks.

Here their configs:

args:
  - '--quiet'
  - '--skip-check'
  - 'CKV_GCP_43,CKV_GCP_62,CKV_GCP_37,CKV_GCP_38'  # Key Rotation, Bucket log, CSEK

https://github.com/mikinhas/terraform-scaleway-instance-module/blob/d3fdfdf31502e2ab569678cc44b2667298956fd4/.pre-commit-config.yaml#L11-L15


args:
  - --skip-path
  - .*(?<!\.tf)$

https://github.com/htec-infra/project-scaffold/blob/07848452c5b68b219169ce1f92b676dcca4df6e0/apps/terraform/tmpl/.pre-commit-config.yaml#L35-L38


args: ['-d .']

https://github.com/ravi-skills/AWS-EKS/blob/15acd1a2a3d9a70f0d3d44422b9e5702d9a28eb2/InfraCode/.pre-commit-config.yaml#L8-L9


args: ["-d", "."]

https://github.com/kazhala/terraform-aws-tags/blob/79ca76a9d7f37ad6846a760536b5263ad96584c9/.pre-commit-config.yaml#L9-L10
https://github.com/kazhala/terraform-aws-ecs-ec2-cluster/blob/4febe01e8c04e02548b0062cf5c92aab330caebd/.pre-commit-config.yaml#L9-L10
https://github.com/kazhala/terraform-aws-rds/blob/8124dd6105eb71526c52095d784fc0c4c5595d4f/.pre-commit-config.yaml#L9-L10
https://github.com/kazhala/terraform-aws-vpc/blob/a7c05cbf523f50dd9572f2abe90dec0418a11216/.pre-commit-config.yaml#L9-L10
https://github.com/kazhala/terraform-aws-ecs-service/blob/f44eda6f216c389a3321b948199041e3a8e1dc52/.pre-commit-config.yaml#L9-L10
https://github.com/kazhala/terraform-aws-efs/blob/af4f06d04cb63e45f04e5ae6302ff5737b8a8a06/.pre-commit-config.yaml#L9-L10
https://github.com/kazhala/terraform-aws-ses-smtp-identity/blob/f484e01c88b70f3ee9c1239e052aaaee14c82219/.pre-commit-config.yaml#L9-L10
https://github.com/anhdle14/terraform-module-template/blob/1098d5c3babe63d5b5c7fefde063cbd01384181c/.pre-commit-config.yaml#L20-L25


args: [--framework, 'terraform']

https://github.com/silazare/terraform-eks-base/blob/7dca5aeff1c4ec0553ee98c7d053643043e2929c/.pre-commit-config.yaml#L13-L14
https://github.com/silazare/terraform-gke-base/blob/20cdd542603990ed8a273e1265b19326331bc38c/.pre-commit-config.yaml#L12-L13


args: [-d ./staging, --skip-check, 'CKV_AWS_18,CKV_AWS_28,CKV_AWS_52,CKV_AWS_65,CKV_AWS_97', --framework, 'terraform']

https://github.com/silazare/aws-ecs-workshop/blob/5da377bcef0ffd6246ebbdcfcc2deabb43b0cef8/.pre-commit-config.yaml#L15-L16


args: [
  "-d", ".",
  "--skip-check", "CKV2_AWS_8",
]

https://github.com/mackah666/terraform-aws-sqs/blob/dfef1849eb07a2f44a072fc3711ca0b2bc8527e2/.pre-commit-config.yaml#L11-L14
[Commented] https://github.com/mackah666/terraform-aws-lambda/blob/ddbe71e2ff8062f6d34d5a31bfa92b8739daa3cc/.pre-commit-config.yaml#L9-L13

@yermulnik
Copy link
Collaborator

and found that args used in 16 repos by 7 folks.

Great finding mate. This means we can introduce such a backward incompatible change and even create issues in those projects to let the folks know they need to update their configs, right?

@MaxymVlasov
Copy link
Collaborator

GH doesn't count private repos, which means that it only helps to understand ~% repos that use checkov with args: relative to the total repos.

16/176 ~= 9% repos will be affected.

And no, we shouldn't go to folks and open issues about any changes.

@MaxymVlasov
Copy link
Collaborator

  1. Have the same performance issue for pre-commit run --all as in Improve perfomance during pre-commit --all (-a) run #309, so this PR depends on Improve perfomance during pre-commit --all (-a) run #309 resolution.
  2. I committed some changes, that dramatically improved speed
Test results

5 runs 'checkov (pre-commit run --all) PR #290:'

time command max min mean median
users seconds 194.15 183.31 187.162 184.49
system seconds 121.55 119.64 120.796 121.24
CPU % 149 144 146.6 147
Total time 218.03 203.03 209.576 206.83
Run details
  • Test Start: Wed Dec 22 18:55:01 UTC 2021
  • Test End: Wed Dec 22 19:12:29 UTC 2021
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo -a /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'checkov PR #290:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         6307376 kB
MemAvailable:    8895252 kB
Buffers:          356464 kB
Cached:          2229868 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.007
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'checkov PR #290:'

time command max min mean median
users seconds 194.15 7.04 97.346 95.625
system seconds 121.55 4.81 62.839 62.325
CPU % 155 140 147.1 147
Total time 218.03 8.08 108.988 106.045
Run details
  • Test Start: Wed Dec 22 19:22:51 UTC 2021
  • Test End: Wed Dec 22 19:23:33 UTC 2021
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'checkov PR #290:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         6285132 kB
MemAvailable:    8874268 kB
Buffers:          357716 kB
Cached:          2229876 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.007
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'checkov (pre-commit run --all) v1.62.3:'

time command max min mean median
users seconds 194.15 7.04 69.267333333333 13.35
system seconds 121.55 2.65 42.863333333333 4.84
CPU % 155 106 134.46666666667 146
Total time 218.03 8.08 77.54 14.84
Run details
  • Test Start: Wed Dec 22 19:24:26 UTC 2021
  • Test End: Wed Dec 22 19:25:40 UTC 2021
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo -a /mnt/c/Users/vm/code/open-source/pre-commit-terraform checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'checkov PR v1.62.3:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         6318444 kB
MemAvailable:    8910328 kB
Buffers:          358448 kB
Cached:          2229980 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.007
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'checkov v1.62.3:'

time command max min mean median
users seconds 194.15 7.04 55.241 13.18
system seconds 121.55 2.47 32.826 4.035
CPU % 155 104 127.5 126.5
Total time 218.03 8.08 61.8665 14.85
Run details
  • Test Start: Wed Dec 22 19:25:48 UTC 2021
  • Test End: Wed Dec 22 19:27:03 UTC 2021
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo /mnt/c/Users/vm/code/open-source/pre-commit-terraform checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'checkov PR v1.62.3:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         6270784 kB
MemAvailable:    8862552 kB
Buffers:          359112 kB
Cached:          2229980 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.007
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'checkov improvments:'

time command max min mean median
users seconds 194.15 5.22 45.258 12.99
system seconds 121.55 2.47 26.882 3.11
CPU % 184 104 136 146
Total time 218.03 4.59 50.4892 14.7
Run details
  • Test Start: Wed Dec 22 20:01:22 UTC 2021
  • Test End: Wed Dec 22 20:01:47 UTC 2021
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'checkov improvments:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         6116244 kB
MemAvailable:    8744184 kB
Buffers:          366044 kB
Cached:          2257108 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.007
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'checkov (pre-commit -a) improvments:'

time command max min mean median
users seconds 194.15 5.22 57.949333333333 13.18
system seconds 121.55 2.47 35.683666666667 4.1
CPU % 184 104 141.93333333333 147
Total time 218.03 4.59 61.578 14.85
Run details
  • Test Start: Wed Dec 22 20:02:13 UTC 2021
  • Test End: Wed Dec 22 20:11:59 UTC 2021
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo -a /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'checkov (pre-commit -a) improvments:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         6027688 kB
MemAvailable:    8657460 kB
Buffers:          367748 kB
Cached:          2257132 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.007
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

@MaxymVlasov MaxymVlasov changed the title fix: allow checkov to run per folder like tflint feat: Allow checkov to run per folder Dec 23, 2021
@github-actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 23, 2022
@MaxymVlasov MaxymVlasov added on-hold Indicates that an issue or PR should not be auto-closed due to staleness. and removed stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 26, 2022
@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Feb 10, 2022

performance issue for pre-commit run --all fixed in d4b1096 (#290)

Tests results: 5 runs 'v1.63.0 `pre-commit run`'
time command max min mean median
users seconds 29.93 26.1 27.922 27.83
system seconds 10.2 8.12 9.062 8.68
CPU % 165 159 162 162
Total time 24.77 20.93 22.754 22.83
Run details
  • Test Start: Thu Feb 10 16:57:19 UTC 2022
  • Test End: Thu Feb 10 16:59:13 UTC 2022
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo /mnt/c/Users/vm/code/open-source/pre-commit-terraform checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs 'v1.63.0 pre-commit run'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         4908188 kB
MemAvailable:    8161860 kB
Buffers:          300264 kB
Cached:          2923732 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.008
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

2 runs 'v1.63.0 pre-commit run -a'

time command max min mean median
users seconds 25.87 25.02 25.445 25.445
system seconds 8.98 8.06 8.52 8.52
CPU % 168 161 164.5 164.5
Total time 20.64 20.47 20.555 20.555
Run details
  • Test Start: Thu Feb 10 16:59:35 UTC 2022
  • Test End: Thu Feb 10 17:00:16 UTC 2022
Variable name Value
TEST_NUM 2
TEST_COMMAND pre-commit try-repo -a /mnt/c/Users/vm/code/open-source/pre-commit-terraform checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 2 runs 'v1.63.0 pre-commit run -a'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         4906488 kB
MemAvailable:    8160928 kB
Buffers:          300568 kB
Cached:          2923872 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.008
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs '290 pre-commit run:'

time command max min mean median
users seconds 7.89 7.64 7.724 7.66
system seconds 4.09 4.04 4.072 4.08
CPU % 175 159 163.6 161
Total time 7.42 6.79 7.196 7.26
Run details
  • Test Start: Thu Feb 10 17:01:42 UTC 2022
  • Test End: Thu Feb 10 17:02:18 UTC 2022
Variable name Value
TEST_NUM 5
TEST_COMMAND pre-commit try-repo /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 5 runs '290 pre-commit run:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         4879912 kB
MemAvailable:    8134636 kB
Buffers:          301016 kB
Cached:          2924104 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.008
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

2 runs '290 pre-commit run -a without speedup:'

time command max min mean median
users seconds 137.59 135.64 136.615 136.615
system seconds 80.82 79.96 80.39 80.39
CPU % 171 169 170 170
Total time 127.46 127.17 127.315 127.315
Run details
  • Test Start: Thu Feb 10 17:03:28 UTC 2022
  • Test End: Thu Feb 10 17:07:42 UTC 2022
Variable name Value
TEST_NUM 2
TEST_COMMAND pre-commit try-repo -a /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 2 runs '290 pre-commit run -a without speedup:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         4908756 kB
MemAvailable:    8163916 kB
Buffers:          301620 kB
Cached:          2924188 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.008
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

2 runs '290 pre-commit run -a with speedup:'

time command max min mean median
users seconds 24.59 24.06 24.325 24.325
system seconds 8.36 7.73 8.045 8.045
CPU % 182 176 179 179
Total time 18.33 17.78 18.055 18.055
Run details
  • Test Start: Thu Feb 10 17:49:26 UTC 2022
  • Test End: Thu Feb 10 17:50:02 UTC 2022
Variable name Value
TEST_NUM 2
TEST_COMMAND pre-commit try-repo -a /tmp/290 checkov
TEST_DIR /home/vm/code/Oslo
TEST_DESCRIPTION 2 runs '290 pre-commit run -a with speedup:'
RAW_TEST_RESULTS_FILE_NAME pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         4798432 kB
MemAvailable:    8062356 kB
Buffers:          308048 kB
Cached:          2925804 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.008
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

@MaxymVlasov MaxymVlasov mentioned this pull request Feb 10, 2022
4 tasks
@amontalban
Copy link

@MaxymVlasov Looks like this is ready to be merged, isn't it? I can test if there is more testing needed. Thanks!

@MaxymVlasov MaxymVlasov removed on-hold Indicates that an issue or PR should not be auto-closed due to staleness. breaking-changes The current implementation contains Breaking Changes / Can't be implemented without Breaking Changes labels Apr 13, 2022
@MaxymVlasov MaxymVlasov changed the title feat: Allow checkov to run per folder feat: Add terraform_checkov, that run per folder. Deprecate checkov hook Apr 13, 2022
Copy link
Owner

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Let's keep the existing hook but customize its behavior to support both recursive and non-recursive runs (or what else this PR adds).

It is pretty bad to have two hooks doing very similar things.

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Apr 13, 2022

Let's keep the existing hook but customize

No way. @antonbabenko

It is pretty bad to have two hooks doing very similar things.

So the old one will be removed in 2.0.0

or what else this PR adds).

This PR makes the hook customizable as all other hooks, which allows us to introduce custom settings for hook etc., and support it at all.
Also, the old hook can run only on the whole repo or on hardcoded in .pre-commit-config.yaml path, which is not what you want to see when you change a tiny thing somewhere in big monorepo.

Copy link
Owner

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

We need to make it a bit clear that checkov hook is deprecated in favor of terraform_checkov hook.

Update name in .pre-commit-hooks.yaml (line 96) with (deprecated, use "terraform_checkov").

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@MaxymVlasov
Copy link
Collaborator

@antonbabenko merge, please

@antonbabenko antonbabenko merged commit e3a9834 into antonbabenko:master Apr 15, 2022
antonbabenko pushed a commit that referenced this pull request Apr 15, 2022
# [1.67.0](v1.66.0...v1.67.0) (2022-04-15)

### Features

* Added `terraform_checkov` (run per folder), deprecated `checkov` hook ([#290](#290)) ([e3a9834](e3a9834))
@antonbabenko
Copy link
Owner

This PR is included in version 1.67.0 🎉

@amontalban
Copy link

Thank you very much @MaxymVlasov !

@bmbferreira bmbferreira deleted the execute-checkov-per-folder branch April 16, 2022 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
estimate/2h Need 2 hours to be done feature New feature or request hook/terraform_checkov Bash hook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants