-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
[FLINK-36740] [WebFrontend] Update frontend dependencies to address vulnerabilities #25718
Conversation
@MartijnVisser I've raised this PR for upgrading dependencies to solve critical/high severities in case you may not have the notification |
"@angular/forms": "^18.2.12", | ||
"@angular/platform-browser": "^18.2.12", | ||
"@angular/platform-browser-dynamic": "^18.2.12", | ||
"@angular/router": "^18.2.12", |
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 rebase the latest code, currently the front-end dependencies have been upgraded in this PR #25713
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.
@simplejason Thank you, rebased the PR to take these changes
Hi @davidradl I've checked package-lock.json of this PR and seems to be the same version as raised by these two PRs so yes It will be solved by this PR |
ab30a12
to
8f15db1
Compare
This change is no longer needed after the merge of apache#25713 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
Thanks @davidradl |
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.
Great contribution, thanks a lot!
I've also tested the Web UI with this PR and couldn't find any regressions.
I found one thing that needs to be addressed with this PR as well, even though I think similar PRs in the recent past haven't addressed this -- so even better to address it now:
We are maintaining a NOTICE file which lists all dependencies and dep versions we are using for legal compliance reasons.
Many JS dependencies are packaged and distributed (in a minified version) as part of Flink releases. With this legal documentation, we want to make sure that companies using Flink, or distributing Flink don't get into trouble for not listing which open source dependencies they are using.
https://github.com/apache/flink/blob/master/flink-runtime-web/src/main/resources/META-INF/NOTICE
I'm not entirely sure how this list got initially created there. Maybe npm has a method of listing all dependencies?
Thanks a lot @rmetzger. |
Hey, thanks a lot for looking into this. This might indeed be generated from Blackduck. I used to have an employer with a black duck license. |
Sure! I'll look for packages that offer the same content as Blackduck. |
Doesn't have to be exactly the same -- as long as the package name, version and the license is included |
I've tried several NPM packages to extract licenses and here are the output of two dependencies.
Dependency Licensesflink-dashboard (2.0.0)Generated at: Wed Dec 11 2024 17:05:38 GMT+0100 (Central European Standard Time)
|
Thanks a lot. Does any of the tools also include copies of the actual licenses used? |
I personally like the |
@rmetzger Thanks for your response and yes it's possible to update the template to include copies of the actual licenses used. |
This package also support the output template so we get rid of these columns but it seems that he's limited to only direct dependencies (same result with option |
flink-runtime-web | ||
Copyright 2014-2024 The Apache Software Foundation | ||
|
||
This product includes software developed at | ||
The Apache Software Foundation (http://www.apache.org/). |
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.
Can you preserve this header in the NOTICE file (it is common in all Flink modules)?
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.
Sorry for this. I've restored this header in NOTICE file and updated the template to handle it.
| undici-types | 6.20.0 | MIT | | ||
| zone.js | 0.14.10 | MIT | | ||
|
||
|
||
## Licenses: | ||
@angular/[email protected] | ||
--- | ||
The MIT License | ||
|
||
Copyright (c) 2010-2024 Google LLC. https://angular.dev/license | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining a copy |
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.
This layout and output of the module is perfect!
Can you add instructions on how to generate those to the README.md file of flink-runtime-web?
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.
Thanks @rmetzger. I've added instructions to the README.md on how to generate the NOTICE
@@ -0,0 +1,12 @@ | |||
# Dependency Licenses |
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 guess the header I mentioned should go here
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.
Should be fixed :)
if ! npm list -g @wbmnky/license-report-generator > /dev/null | ||
then | ||
npm install -g @wbmnky/license-report-generator | ||
fi | ||
|
||
devDir=$(cd "$(dirname "$0")" && pwd) | ||
(cd "${devDir}/.." && license-report-generator --depth Infinity \ | ||
--template-file "notice-template" --template-dir "${devDir}" \ | ||
--out-dir "${devDir}/../../src/main/resources/META-INF" --out-file "NOTICE") |
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.
Great idea to add a script!
Perfect, thanks a lot. This is almost ready to be merged! |
Thanks a lot @rmetzger I've made the changes, do I need to squash the commits ? |
| pretty-format | 27.5.1 | MIT | | ||
| react-is | 17.0.2 | MIT | | ||
| regenerator-runtime | 0.14.1 | MIT | | ||
| robust-predicates | 3.0.1 | Unlicense | |
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.
Just for future reference: this license is ok for ASF projects: https://www.apache.org/legal/resolved.html#category-a
I'm sorry that this drags on for so long: the |
No, I can do it while merging. |
Thanks. Let's get this in once CI has passed. |
Thank you Robert. Juste seen that the |
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.
Thanks a lot for your contribution -- merging the PR!
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
…ulnerabilities (apache#25718) Follow up of https://issues.apache.org/jira/browse/FLINK-36739 Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
Follow up of https://issues.apache.org/jira/browse/FLINK-36739
Contribute-to: https://issues.apache.org/jira/browse/FLINK-36740
What is the purpose of the change
This PR aims to bump some dependencies version in order to address critical/high vulnerabilities after the merge of NodeJS v22 which is required to have these changes.
Before the PR
37 vulnerabilities (3 low, 14 moderate, 17 high, 3 critical)
After the PR
8 moderate severity vulnerabilities
Brief change log
Jobs
after the updateVerifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation