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

fix: optional python dependencies #614

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Conversation

gitphill
Copy link
Contributor

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Optional Python dependencies are being over connected.

When building Python dep-graphs the current algorithm does not consider whether a transitive should be traversed based on 'extra' definitions.

When a Python dependency uses extras, this means that dependency is optional and will only be installed when the installer asks.

For example if 'my-package' has the following METADATA snippet:

Requires-Dist: logger>=1.0
Provides-Extra: test
Requires-Dist: tester>=0.5.0; extra == 'test'

This means 'logger' is always installed, but 'tester' is only installed if 'my-package' is installed with extra 'test' dependencies, like so:

my-package[test]==1.2.3

The current algorithm sometimes gets away with this lack of accuracy because if the dependency is not used at all the the METADATA file doesn't even exist and we don't attach any node to the graph. However in many cases what's optional in one transitive line is not in another and this can mean the METADATA file does exists. This results in the current algorithm accidentally associating potentially large sub-graphs to transitive lines that should be terminated when extras are not being used.

The first change here introduces code that firstly parses out these extra definitions in both requirements.txt and METADATA files. Then also parsers that pick out the Provides-Extra and Requires-Dist extra environment markers.

The second change adapts the pip dep-graph builder to take into account extra properties to decide whether a dependency should be traversed or not.

Screenshots

Screenshot 2024-11-14 at 15 11 39

@gitphill gitphill force-pushed the fix/optional-python-dependencies branch from a72733d to 0b6ddbb Compare November 14, 2024 15:13
@gitphill gitphill marked this pull request as ready for review November 14, 2024 15:15
@gitphill gitphill requested a review from a team as a code owner November 14, 2024 15:15
@gitphill gitphill self-assigned this Nov 14, 2024
@gitphill gitphill force-pushed the fix/optional-python-dependencies branch from 0b6ddbb to b71e613 Compare November 14, 2024 15:21
Optional Python dependencies are being over connected.

When building Python dep-graphs the current algorithm does not consider
whether a transitive should be traversed based on 'extra' definitions.

When a Python dependency uses extras, this means that dependency is
optional and will only be installed when the installer asks.

For example if 'my-package' has the following METADATA snippet:

```
Requires-Dist: logger>=1.0
Provides-Extra: test
Requires-Dist: tester>=0.5.0; extra == 'test'
```

This means 'logger' is always installed, but 'tester' is only installed
if 'my-package' is installed with extra 'test' dependencies, like so:

```
my-package[test]==1.2.3
```

The current algorithm sometimes gets away with this lack of accuracy
because the dependency is skipped if the METADATA file doesn't even
exist. However in many cases what's optional in one transitive line is
not in another and this can mean the METADATA file does exists. This
results in the current algorithm accidentally associating potentially
large sub-graphs to transitive lines that should be terminated when
extras are not being used.

The first change here introduces code that firstly parses out these
extra definitions in both requirements.txt and METADATA files. Then also
parsers that pick out the Provides-Extra and Requires-Dist extra
environment markers.

The second change adapts the pip dep-graph builder to take into account
extra properties to decide whether a dependency should be traversed or
not.
@gitphill gitphill force-pushed the fix/optional-python-dependencies branch from b71e613 to 8a8f796 Compare November 14, 2024 15:52
@gitphill gitphill merged commit cdb9eba into main Nov 15, 2024
17 checks passed
@gitphill gitphill deleted the fix/optional-python-dependencies branch November 15, 2024 11:54
@snyk-team-unify
Copy link

🎉 This PR is included in version 6.13.15 🎉

The release is available on:

Your semantic-release bot 📦🚀

gitphill added a commit to snyk/cli that referenced this pull request Nov 15, 2024
See snyk/snyk-docker-plugin#614

Fixing a bug in snyk-docker-plugin so that optional dependencies are
properly connected in the dep-graph.

This could mean that 'snyk container monitor' commands that previously
timed out or errored may now start working. The fix reduces unecessary
(optional) paths in the dep-graph and so reduce the work Snyk needs to
do when scanning for vulnerabilities.
gitphill added a commit to snyk/cli that referenced this pull request Nov 15, 2024
See snyk/snyk-docker-plugin#614

Fixing a bug in snyk-docker-plugin so that optional dependencies are
properly connected in the dep-graph.

This could mean that 'snyk container monitor' commands that previously
timed out or errored may now start working. The fix reduces
unnecessary (optional) paths in the dep-graph and so reduce the work
Snyk needs to do when scanning for vulnerabilities.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants