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(conda): add licenses support for environment.yml files #6953

Conversation

DmitriyLewen
Copy link
Contributor

Description

Add licenses support for environment.yml files.
Licenses are taken from conda-meta directory.
Path to conda-meta is determined from prefix field of environment.yml

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@DmitriyLewen DmitriyLewen self-assigned this Jun 18, 2024
@@ -77,6 +78,7 @@ func (p *Parser) toPackage(dep Dependency) ftypes.Package {
EndLine: dep.Line,
},
},
FilePath: prefix,
Copy link
Contributor Author

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.

Copy link
Collaborator

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.

Copy link
Contributor Author

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:

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?

Copy link
Collaborator

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.

Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knqyf263 i updated PR (67daea7 + 9200c44)
take a look, please.

@DmitriyLewen DmitriyLewen marked this pull request as ready for review June 19, 2024 10:27
@@ -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 {
Copy link
Collaborator

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.

Suggested change
if res != nil && len(res.Applications) > 0 {
if res == nil || len(res.Applications) == 0 {
return nil, nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in d75bce2

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",
Copy link
Collaborator

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.

// 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()
}

Suggested change
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"))

Copy link
Collaborator

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))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am fixing that

Copy link
Contributor Author

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"

@knqyf263 knqyf263 mentioned this pull request Jul 1, 2024
DmitriyLewen and others added 2 commits July 1, 2024 12:19
Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>
@knqyf263 knqyf263 added this pull request to the merge queue Jul 1, 2024
Merged via the queue into aquasecurity:main with commit 654217a Jul 1, 2024
17 checks passed
@DmitriyLewen DmitriyLewen deleted the feat-conda/environment.yaml-license-support branch July 1, 2024 09:25
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.

feat(conda): detect licenses for environment.yml Conda environment.yml support
2 participants