-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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(conda): add licenses support for environment.yml
files
#6953
feat(conda): add licenses support for environment.yml
files
#6953
Conversation
@@ -77,6 +78,7 @@ func (p *Parser) toPackage(dep Dependency) ftypes.Package { | |||
EndLine: dep.Line, | |||
}, | |||
}, | |||
FilePath: prefix, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use prefix
as FilePath
in dependency parser.
This is a workaround.
Perhaps we need to add a prefix or a new field to Package
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's inefficient, but what if parsing environment.yaml
again in pkg/fanal/analyzer/language/conda/environment/environment.go
to just extract a prefix? Using FilePath
is a bit hacky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. I doubted this decision.
I didn't think about it right away, but what if we will use logic as for package.json
file.
I mean don't implement Parser interface:
trivy/pkg/dependency/parser/nodejs/packagejson/parse.go
Lines 27 to 41 in e9fc3e3
type Package struct { | |
ftypes.Package | |
Dependencies map[string]string | |
OptionalDependencies map[string]string | |
DevDependencies map[string]string | |
Workspaces []string | |
} | |
type Parser struct{} | |
func NewParser() *Parser { | |
return &Parser{} | |
} | |
func (p *Parser) Parse(r io.Reader) (Package, error) { |
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also okay for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DmitriyLewen Do you think you'll update it shortly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to do this today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -26,8 +31,68 @@ func (a environmentAnalyzer) Analyze(_ context.Context, input analyzer.AnalysisI | |||
if err != nil { | |||
return nil, xerrors.Errorf("unable to parse environment.yaml: %w", err) | |||
} | |||
|
|||
if res != nil && len(res.Applications) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I prefer an early return.
if res != nil && len(res.Applications) > 0 { | |
if res == nil || len(res.Applications) == 0 { | |
return nil, nil | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in d75bce2
…environment.yaml-license-support
if err != nil { | ||
// Show log once per file | ||
once.Do(func() { | ||
log.WithPrefix("conda").Debug("License not found. See https://aquasecurity.github.io/trivy/latest/docs/coverage/os/conda/#license_1 for details", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use this function for a versioned document link.
Lines 25 to 31 in 3d4ae8b
// URL returns the URL for the versioned documentation with the given path | |
func URL(rawPath, fragment string) string { | |
base := BaseURL(app.Version()) | |
base.Path = path.Join(base.Path, rawPath) | |
base.Fragment = fragment | |
return base.String() | |
} |
log.WithPrefix("conda").Debug("License not found. See https://aquasecurity.github.io/trivy/latest/docs/coverage/os/conda/#license_1 for details", | |
log.WithPrefix("conda").Debugf("License not found. See %s for details", doc.URL("docs/coverage/os/conda/", "license_1")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I missed some arguments, and my suggestion was broken.
log.String("pkg", pkg.Name), log.Err(err))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am fixing that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b68efe7
DEBUG [conda] License not found. See https://aquasecurity.github.io/trivy/dev/docs/coverage/os/conda#license_1 for details. pkg="_openmp_mutex" err="unable to read conda-meta dir: open /opt/conda/envs/test-env/conda-meta: no such file or directory"
Co-authored-by: Teppei Fukuda <[email protected]>
…curity#6953) Co-authored-by: Teppei Fukuda <[email protected]>
Description
Add licenses support for
environment.yml
files.Licenses are taken from
conda-meta
directory.Path to
conda-meta
is determined from prefix field ofenvironment.yml
Related issues
Checklist