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 gitlab-ci include parser #144

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nejch
Copy link

@nejch nejch commented Oct 22, 2022

Very similar to #143, this is part 2 of adding a few git-based dependencies to trivy - see aquasecurity/trivy#3067 for more context.

This adds basic support for gitlab-ci includes, mostly for discussion in aquasecurity/trivy#3067. I'll extend/adapt this based on feedback and discussions in the issue, and first see if it makes sense to add it here :)

@CLAassistant
Copy link

CLAassistant commented Oct 22, 2022

CLA assistant check
All committers have signed the CLA.

@nejch nejch marked this pull request as ready for review October 23, 2022 00:01
@nejch
Copy link
Author

nejch commented Nov 17, 2022

Sorry for the ping @DmitriyLewen. Are these types of parsers something you'd consider here? If not, I'd maybe look into creating standalone plugins. Thanks a lot!

@DmitriyLewen
Copy link
Collaborator

Hello @nejch
Thanks for your work!

I investigated this case.
There is problem with integration this into Trivy.
This parser doesn't match existing categories (i mean languages, os package, etc...).

My opinion - it is better to start from Plugin(as you said).
If this plugin will have greatest popularity, then we will think about integrating this logic to Trivy.

@nejch
Copy link
Author

nejch commented Nov 18, 2022

Thanks for your response @DmitriyLewen!

I understand. I was hoping it would maybe fit into the new non-packaged category that was recently introduced for unpackaged executables, see:

https://github.com/aquasecurity/trivy/blob/5bb3a47e03e0156a7949dfd453908798eb8a0b65/pkg/fanal/analyzer/const.go#L77-L80

However, I'll look into packaging this into a plugin as well and maybe open/edit the issue to track popularity if needed.

@DmitriyLewen
Copy link
Collaborator

I can suggest you(if you have time for this) to create draft PR in Trivy (use these changes in PR) with tests.
Then we can look at this logic, test it and after then decide on the integration of this functionality.

@nejch
Copy link
Author

nejch commented Nov 18, 2022

That's great, I'll give that a shot :)

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.

3 participants