-
Notifications
You must be signed in to change notification settings - Fork 201
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
NO-ISSUE: Upgrade Maven plugins to newer versions and Apache parent from 32
to 33
#2819
Conversation
@yesamer I guess I didn't write this anywhere, but the idea of managing the versions of Maven plugins ourselves is to have more control over patches that may contain security fixes. If we delegate it to the Apache parent we may find ourselves in a position of having to declare them anyway to fix something that the Apache parent hasn't. |
@tiagobento Well, at the moment a CVE is detected in any plugin version, we can temporarily override. I guess that is a good compromise on having the same plugin versions used anywhere and keeping them updated. |
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.
Thank you for the PR, my comments ale inline.
@tiagobento I checked the CVE history of all declared plugins in Apache parent pom. |
@yesamer Problems are often in transitive dependencies... |
@tiagobento Even in such a case, those dependencies are not transitively imported in the production code, as the dependencies imported by the plugin are used during its own compile time only. |
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.
Thank you. I did a small sanity check with the online-editor and extended-services:
- open bpmn sample
- open dmn sample, old dmn gwt editor
- run dmn model, old dmn gwt editor
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 don't see it that simply @yesamer... I'm not 100% onboard with this version management strategy this PR introduces, as it delegates to Apache parent for maven-base but not for some other packages. I'd like to hold merging it so we can discuss it in more depth. Such a change is not to be done casually, IMHO, as it has to do with the way we manage and maintain dependencies in general.
@tiagobento Setting this PR on hold until you share your input on the whole strategy. Again, apologies for my previous bad answer that I removed. |
@tiagobento After our discussion now I fully understand the scenario you described, and I agree it can be a realistic scenario. Thank you for raising that and apologize again for not giving the proper attention. |
<artifactId>maven-helper-plugin</artifactId> | ||
<version>${version.maven.helper.plugin}</version> | ||
<artifactId>maven-help-plugin</artifactId> | ||
<version>${version.maven.help.plugin}</version> |
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.
maven-helper-plugin
doesn't exist
32
to 33
In this PR:
- Removed all plugin declarations already present in the Apache parent pom- Removed stale profile reference in serverless-workflow