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

Bugfixed scanner parseDefinitions ttlSecondsAfterFinished HelmValue inconsistencies #616

Merged
merged 5 commits into from
Aug 31, 2021

Conversation

SebieF
Copy link
Contributor

@SebieF SebieF commented Aug 30, 2021

Description

We had a lot of new scanner pull requests recently, that's why I came to think about code/file consistency for our scanners. I think it is worth discussing this topic publicly or in our retro, because it makes our code more maintainable and easier to understand.
For now, I only fixed some small inconsistencies and a typo for templates/scanner-name-parse-definition.yaml.

No pun intended

Signed-off-by: Sebastian <sebastian.franz@iteratec.com>
Replacing
ttlSecondsAfterFinished: {{ .Values.parser.ttlSecondsAfterFinished }} with
  ttlSecondsAfterFinished: {{ .Values.parser.image.ttlSecondsAfterFinished }}

Signed-off-by: Sebastian <sebastian.franz@iteratec.com>
@SebieF SebieF self-assigned this Aug 30, 2021
@SebieF SebieF added this to In progress in secureCodeBox v3 via automation Aug 30, 2021
@J12934
Copy link
Member

J12934 commented Aug 30, 2021

+1 for consistency.

But placing the ttl after finished would be conceptually wrong for me, as the ttl has nothing to do with the image, but the kubernetes job created by the parseDefinitions. I think it is fine where it it.

Replacing
ttlSecondsAfterFinished: {{ .Values.parser.image.ttlSecondsAfterFinished }} with
  ttlSecondsAfterFinished: {{ .Values.parser.ttlSecondsAfterFinished }}

Signed-off-by: Sebastian <sebastian.franz@iteratec.com>
@SebieF
Copy link
Contributor Author

SebieF commented Aug 30, 2021

+1 for consistency.

But placing the ttl after finished would be conceptually wrong for me, as the ttl has nothing to do with the image, but the kubernetes job created by the parseDefinitions. I think it is fine where it it.

Fair point, wasn't sure about that. In hindsight, I think only ncrack used this .image. so it makes more sense to adjust that.

Because of failing ci pipeline integration tests

Signed-off-by: Sebastian <sebastian.franz@iteratec.com>
@SebieF SebieF marked this pull request as draft August 31, 2021 11:20
This reverts commit 2371720.

Signed-off-by: Sebastian <sebastian.franz@iteratec.com>
@SebieF SebieF force-pushed the fixing-parse-definitions-inconsistencies branch from 0f766ec to b5b16d6 Compare August 31, 2021 11:29
@SebieF SebieF marked this pull request as ready for review August 31, 2021 11:29
@SebieF SebieF requested a review from rseedorff August 31, 2021 11:29
secureCodeBox v3 automation moved this from In progress to Reviewer approved Aug 31, 2021
@rseedorff rseedorff changed the title Fixing parse definitions inconsistencies Bugfixed scanner parseType ttlSecondsAfterFinished value inconsistencies Aug 31, 2021
@rseedorff rseedorff added bug Bugs scanner Implement or update a security scanner and removed maintenance labels Aug 31, 2021
@rseedorff rseedorff added this to the v3.1.0 milestone Aug 31, 2021
@rseedorff rseedorff changed the title Bugfixed scanner parseType ttlSecondsAfterFinished value inconsistencies Bugfixed scanner parsedefinitions ttlSecondsAfterFinished value inconsistencies Aug 31, 2021
@rseedorff rseedorff changed the title Bugfixed scanner parsedefinitions ttlSecondsAfterFinished value inconsistencies Bugfixed scanner parseDefinitions ttlSecondsAfterFinished HelmValue inconsistencies Aug 31, 2021
@rseedorff rseedorff merged commit dddac0d into main Aug 31, 2021
@rseedorff rseedorff deleted the fixing-parse-definitions-inconsistencies branch August 31, 2021 14:09
secureCodeBox v3 automation moved this from Reviewer approved to Done Aug 31, 2021
@rseedorff rseedorff linked an issue Aug 31, 2021 that may be closed by this pull request
9 tasks
@SebieF SebieF moved this from Done to counter in secureCodeBox v3 Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs scanner Implement or update a security scanner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚧 [Consistency] Ensuring file/code consistency for scanners
3 participants