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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

read-package-tree depreciated #9129

Closed
Tracked by #12688
JakeGinnivan opened this issue Mar 7, 2022 · 3 comments 路 Fixed by #15503
Closed
Tracked by #12688

read-package-tree depreciated #9129

JakeGinnivan opened this issue Mar 7, 2022 · 3 comments 路 Fixed by #15503
Assignees
@julienp
Labels
area/sdks Pulumi language SDKs good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Milestone
0.101

Comments

@JakeGinnivan
Copy link

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already)

Issue details

As part of good dependency hygine I try to remove deprecation notices when they appear. NPM has deprecated read-package-tree@5.3.1 which is replaced with arborist.

I can see 2 ways forward

  1. Migrate to arborist, there are no decent migration docs and the package is quite a bit larger than the current read-package-tree dependency
  2. Inline the read-package-tree code, it's code is pretty simple and could just be absorbed into the pulumi codebase

Steps to reproduce

  1. Install @pulumi/pulumi into an Node project

Expected: No deprecation warning
Actual: Deprecation warning
@JakeGinnivan JakeGinnivan added the kind/bug Some behavior is incorrect or out of spec label Mar 7, 2022
@mikhailshilkov mikhailshilkov added help-wanted We'd love your contributions on this issue language/javascript labels Mar 7, 2022
@RobbieMcKinstry
Copy link
Contributor

I'd personally prefer we migrate to arborist. I expect the increase in package size (~450KB) is not a major issue. Personally, I don't believe we should absorb new code into our code base if we can avoid it. We already support a large volume of code since we support six different languages, each with its own runtime and SDK. In the future, we can also minimize the impact of this package with tree shaking and modification. There are existing issues to track that work (listed here).

That's just my 2 cents! :) I'd be happy to accept a PR that removes this deprecation.

@RobbieMcKinstry RobbieMcKinstry added good-first-issue Start here if you'd like to start contributing to Pulumi area/sdks Pulumi language SDKs labels Feb 15, 2023
@Frassle Frassle assigned Frassle and unassigned Frassle Jun 28, 2023
@kmosher
Copy link
Contributor

kmosher commented Jan 29, 2024

This is apparently now tripping some vulnerability scanners for CWE-772

@data-stack404
Copy link

I'd like to add some context to this:
Pulumi uses the deprecated package "read-package-tree", which itself is using "read-package-content". This one requires the abandoned package "inflight", that has the mentioned security vulnerability (https://security.snyk.io/vuln/SNYK-JS-INFLIGHT-6095116).

@julienp julienp self-assigned this Feb 23, 2024
This was referenced Feb 23, 2024
Closed
Merged
github-merge-queue bot pushed a commit that referenced this issue Feb 26, 2024
@julienp
Replace deprecated read-package-tree with @npmcli/arborist (#15503)
be0fe6b 
<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

read-package-tree is deprecated. Additionally it has a dependency that
is flagged by security scanners.

<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

Fixes #9129

Ref #12688

## Checklist

- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
  - [x] I have formatted my code using `gofumpt`

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Feb 26, 2024
@justinvp justinvp added this to the 0.101 milestone Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@julienp julienp

Labels
area/sdks Pulumi language SDKs good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.101
Development

Successfully merging a pull request may close this issue.

Replace deprecated read-package-tree with @npmcli/arborist pulumi/pulumi
9 participants
@julienp @JakeGinnivan @justinvp @kmosher @Frassle @mikhailshilkov @RobbieMcKinstry @pulumi-bot @data-stack404