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

Deploy flattened POM for consumer modules #8716

Closed
npepinpe opened this issue Feb 2, 2022 · 6 comments · Fixed by #8833
Closed

Deploy flattened POM for consumer modules #8716

npepinpe opened this issue Feb 2, 2022 · 6 comments · Fixed by #8833
Assignees
Labels
kind/feature Categorizes an issue or PR as a feature, i.e. new behavior

Comments

@npepinpe
Copy link
Member

npepinpe commented Feb 2, 2022

Is your feature request related to a problem? Please describe.

There are a few modules in the Zeebe project which are meant to be consumed by user's projects:

  • io.camunda:zeebe-client-java
  • io.camunda:zeebe-bpmn-model
  • io.camunda:zeebe-exporter-api
  • io.camunda:zeebe-gateway-protocol-impl
  • io.camunda:zeebe-protocol-jackson

The projects are deployed as is, with their "development" POM - this POM includes development specific profiles, plugins, and relies on the centralized dependency management in io.camunda:zeebe-parent as well as properties substitution.

This causes a few problems:

  1. Vulnerability issues introduced by transitive dependencies may re-appear in consumer's code. The Zeebe team scans these projects for security vulnerabilities, which are often remediated by updating transitive dependencies. The tools used to scan the whole project correctly resolve the dependencies to those specified in io.camunda:zeebe-parent, and thus assume there are no vulnerabilities. When a user introduces the library (such as io.camunda:zeebe-client-java) in their own project, however, without specifying versions for the transitive dependencies, these will get resolved based on the shortest path, which may be a version with security issues. While users should be expected to scan their own projects and fix vulnerability issues, this can still places an undue burden on them, and negates any effort we, the Zeebe team, did on that front.
  2. There may be slight behavioral changes between the tests ran by the Zeebe team using specific versions of transitive dependencies and the actual dependencies automatically resolved in the user's project, which can cause unexpected behavior.

For both issues above, it's perfectly fine if these occur when the user has explicitly overridden one of the dependency's versions. But when that's not the case, then no such issues should appear in their projects.

Describe the solution you'd like

Use the flatten-maven-plugin to generate flat POMs for the projects above, and any other projects which is explicitly meant to be included in another project. This will not only trim down the POM to its minimum, making it often more readable, but it will solve both of the issues above.

Describe alternatives you've considered

We could fix the dependencies ourselves manually in the POM when security issues arise, however this sort of negates the benefits we gain from centralized dependency management and creates more busy work for the team.

Additional context
Add any other context or screenshots about the feature request here.

@npepinpe npepinpe added the kind/feature Categorizes an issue or PR as a feature, i.e. new behavior label Feb 2, 2022
@korthout
Copy link
Member

korthout commented Feb 3, 2022

I like this approach, I used it successfully before. I recommend using <flattenMode>ossrh</flattenMode>, and would probably also try out <flattenMode>bom</flattenMode> for our bom.

@npepinpe
Copy link
Member Author

npepinpe commented Feb 3, 2022

Can you expand a little on why you would recommend these? And the BOM is good point, I do expect people to include the BOM in their projects for dependency management so we should add it.

Speaking of the BOM, I'm starting to think it might make more sense to have the BOM only contains those modules which we expect to be included in other projects, and to keep our internal modules in either a separate BOM-equivalent module, or just have them in the parent module's dependency management section. This would reinforce the idea for external users that only a certain subset of modules are actually public.

@korthout
Copy link
Member

korthout commented Feb 3, 2022

The ossrh option removes all unnecessary data from the pom. It inlines the properties and removes any build related stuff. But at the same time it leaves the tags necessary for publishing in maven central.

@npepinpe
Copy link
Member Author

npepinpe commented Feb 3, 2022

I would propose the following:

  1. Reduce the BOM to only those modules which are meant to be consumed by other projects outside of Zeebe
  2. Dependency management of internal modules (e.g. zeebe-broker, zeebe-util) is moved to the zeebe-parent. External projects can still use these as dependencies of course, and we can discuss adding a zeebe-internal-bom if we really need. However, I want to increase the effort, because I want to make it as clear/explicit to external projects that these modules are not meant to be included in your projects and don't carry the same guarantees.
  3. Modules listed in the BOM, and the BOM itself, are flattened via the flatten-maven-plugin before being published.

My goal is to have it in 1.4.0-alpha2 like this so that we can test it internally at Camunda with the other teams depending on these modules. Let me know before end of next week if you object.

@remcowesterhoud
Copy link
Contributor

Why would we flatten only these modules instead of all? Doing it for all modules has some advantages in my opinion:

  1. We can have the plugin configured in a single place, instead of for all 5 modules.
  2. Other modules are still published on maven. We might not encourage it in most cases, but people could still add them as dependencies, meaning this problems would still be there for these users.
  3. It's less confusing if everything behaves in the same way, instead of different modules having different behaviour.

@npepinpe
Copy link
Member Author

npepinpe commented Feb 8, 2022

Makes sense, we can flatten all of them, I don't see any downsides to that.

@npepinpe npepinpe added this to Ready in Zeebe Feb 10, 2022
@npepinpe npepinpe moved this from Ready to In progress in Zeebe Feb 22, 2022
@npepinpe npepinpe self-assigned this Feb 22, 2022
@npepinpe npepinpe moved this from In progress to Review in progress in Zeebe Feb 24, 2022
ghost pushed a commit that referenced this issue Mar 1, 2022
8833: Deploy flattened POMs and reduce BOM to "public" modules r=oleschoenburg a=npepinpe

## Description

This PR introduces the `flatten-maven-plugin`, which will flatten any hierarchy in a module's POM before it's deployed somewhere. The original POM will remain intact. The goal here is to simplify the integration of the modules in other people's projects, by making it more explicit which dependencies are pulled by the module, and which version they should be.

Additionally, the BOM is now reduced to only including these modules which were tailor explicitly to be consumed by third party projects. The goal with that is to make it explicit to users which modules they can safely rely on in their projects, and all the rest, so called internal modules, which may be renamed, changed, dropped, etc., and are only expected to be used in Zeebe. Note that they can still be imported in third party projects, but that is then at the user's risks.

## Related issues

closes #8716 



Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
@ghost ghost closed this as completed in d271b72 Mar 1, 2022
Zeebe automation moved this from Review in progress to Done Mar 1, 2022
ghost pushed a commit that referenced this issue Mar 1, 2022
8856: [Backport release-1.4.0-alpha2] Deploy flattened POMs and reduce BOM to "public" modules r=oleschoenburg a=github-actions[bot]

# Description
Backport of #8833 to `release-1.4.0-alpha2`.

relates to #8716

Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
@KerstinHebel KerstinHebel removed this from Done in Zeebe Mar 23, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes an issue or PR as a feature, i.e. new behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants