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

Add ruby.NewGemSpecCataloger to DirectoryCatalogers. #1971

Merged
merged 5 commits into from
Oct 23, 2023

Conversation

evanchaoli
Copy link
Contributor

I have a use case that needs to run Syft scan inside a Dockerfile. But I found that installed Ruby Gems couldn't be found. So I have to use --catalogers=all.

Then I noticed that ruby.NewGemSpecCataloger() is not included in DirectoryCatalogers.

But I also noticed that python.NewPythonPackageCataloger() is in the list, I think PythonPackage is the same place as RubyGemSpec, so I feel it's reasonable to add ruby.NewGemSpecCataloger to DirectoryCatalogers.

@evanchaoli evanchaoli force-pushed the add-gem-spec-to-dir-cata branch 2 times, most recently from 8550981 to 908140b Compare August 1, 2023 02:52
@wagoodman
Copy link
Contributor

Hey! I think doing this is a good idea, however, a little more work would be needed. Today the current gemspec cataloger looks at gemspecs that are installed within a specifications directory (which is done when installing a gem via the CLI).

https://github.com/anchore/syft/blob/main/syft/pkg/cataloger/ruby/catalogers.go#L19

Another constructor for the same cataloger would be needed that looks for gemspecs in any location, something like:

// the existing one today, renamed...
func NewInstalledGemSpecCataloger() *generic.Cataloger {
	return generic.NewCataloger("ruby-gemspec-cataloger").
		WithParserByGlobs(parseGemSpecEntries, "**/specifications/**/*.gemspec")
}

// new one with a more broad glob...
func NewGemSpecCataloger() *generic.Cataloger {
	return generic.NewCataloger("ruby-gemspec-cataloger").
		WithParserByGlobs(parseGemSpecEntries, "**/*.gemspec")
}

@evanchaoli
Copy link
Contributor Author

@wagoodman Thanks for your comment. I wonder why do we need a broader glob? In my case, --catalogers=all has worked around. Will *.gemspec files exist under other directories than "specifications"?

@willmurphyscode
Copy link
Contributor

Will *.gemspec files exist under other directories than "specifications"?

@evanchaoli yes, one example might be scanning the directory when a Gem's source code is checked out - we'd expected to have a *.gemspec in the root.

#2128 has some more information. If you'd like, reworking this PR to fix #2128 would be greatly appreciated.

@willmurphyscode
Copy link
Contributor

@evanchaoli did I answer your question adequately? Please let me know if there's anything I can do to help here.

@evanchaoli
Copy link
Contributor Author

Hey @willmurphyscode I just haven't got bandwidth to get back to this PR. I will try to do something next week.

Signed-off-by: Evan <chaol@vmware.com>
Signed-off-by: Evan <chaol@vmware.com>
@evanchaoli evanchaoli force-pushed the add-gem-spec-to-dir-cata branch from 908140b to 7ca8d6f Compare October 20, 2023 07:18
Signed-off-by: Evan <chaol@vmware.com>
@evanchaoli evanchaoli force-pushed the add-gem-spec-to-dir-cata branch from 7ca8d6f to e848757 Compare October 20, 2023 07:19
@evanchaoli
Copy link
Contributor Author

Hey @wagoodman, finally I got back to this PR. Please take a look at the new diff.

Because the files that the installed gemspec cataloger work off of are a
subset of the files that the more general gemspec cataloger will work
off of, we shouldn't have both of them on by default, since this could
result in finding the same package twice.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
@willmurphyscode willmurphyscode merged commit 671ff39 into anchore:main Oct 23, 2023
10 checks passed
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* Add ruby.NewGemSpecCataloger to DirectoryCatalogers.

Signed-off-by: Evan <chaol@vmware.com>

* fixed tests

Signed-off-by: Evan <chaol@vmware.com>

* Addressed review comment

Signed-off-by: Evan <chaol@vmware.com>

* Remove NewInstalledGemSpecCataloger from default dir catalogers

Because the files that the installed gemspec cataloger work off of are a
subset of the files that the more general gemspec cataloger will work
off of, we shouldn't have both of them on by default, since this could
result in finding the same package twice.

Signed-off-by: Will Murphy <will.murphy@anchore.com>

---------

Signed-off-by: Evan <chaol@vmware.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Co-authored-by: Will Murphy <will.murphy@anchore.com>
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