Skip to content

Bugfixed scanner parseDefinitions ttlSecondsAfterFinished HelmValue inconsistencies #616

Merged
rfelber merged 5 commits intomainfrom
fixing-parse-definitions-inconsistencies
Aug 31, 2021
Merged

Bugfixed scanner parseDefinitions ttlSecondsAfterFinished HelmValue inconsistencies #616
rfelber merged 5 commits intomainfrom
fixing-parse-definitions-inconsistencies

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.

SebieF added 2 commits August 30, 2021 14:53
Replacing
ttlSecondsAfterFinished: {{ .Values.parser.ttlSecondsAfterFinished }} with
  ttlSecondsAfterFinished: {{ .Values.parser.image.ttlSecondsAfterFinished }}

Signed-off-by: Sebastian <[email protected]>
@SebieF SebieF self-assigned this 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 <[email protected]>
@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 <[email protected]>
@SebieF SebieF marked this pull request as draft August 31, 2021 11:20
This reverts commit 2371720.

Signed-off-by: Sebastian <[email protected]>
@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 rfelber August 31, 2021 11:29
@rfelber rfelber changed the title Fixing parse definitions inconsistencies Bugfixed scanner parseType ttlSecondsAfterFinished value inconsistencies Aug 31, 2021
@rfelber rfelber added bug Bugs scanner Implement or update a security scanner and removed maintenance labels Aug 31, 2021
@rfelber rfelber added this to the v3.1.0 milestone Aug 31, 2021
@rfelber rfelber changed the title Bugfixed scanner parseType ttlSecondsAfterFinished value inconsistencies Bugfixed scanner parsedefinitions ttlSecondsAfterFinished value inconsistencies Aug 31, 2021
@rfelber rfelber changed the title Bugfixed scanner parsedefinitions ttlSecondsAfterFinished value inconsistencies Bugfixed scanner parseDefinitions ttlSecondsAfterFinished HelmValue inconsistencies Aug 31, 2021
@rfelber rfelber merged commit dddac0d into main Aug 31, 2021
@rfelber rfelber deleted the fixing-parse-definitions-inconsistencies branch August 31, 2021 14:09
@rfelber rfelber linked an issue Aug 31, 2021 that may be closed by this pull request
9 tasks
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