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

Implement a global non-maven download policy #1669

Open
nedtwigg opened this issue Apr 6, 2023 · 9 comments
Open

Implement a global non-maven download policy #1669

nedtwigg opened this issue Apr 6, 2023 · 9 comments

Comments

@nedtwigg
Copy link
Member

nedtwigg commented Apr 6, 2023

The Spotless core has few dependencies. During plugin configuration, we add maven dependencies which get downloaded in the normal way.

There are some exceptions where we use non-maven formatters. For example

There are other cases we are considering, such as

A very good thing about Spotless is that it "just works". Search the docs for the kind of formatter you want, specify that formatter, and you're ready to go. But for these non-maven-based formatters, I think it's fair for users to be surprised that their build plugin is making network requests and caching artifacts outside of the expected maven channel.

I'm proposing something like this:

spotless {
  allowDownloadsFrom('https://download.eclipse.org/')

As a user, it would work like this:

  • I add Eclipse CDT to my spotless configuration
  • I get an error like so

Eclipse CDT is not available in a maven repository, it must be downloaded from a p2 repository. You can allow this download by adding a block like this to your spotless configuration

spotless {
  allowDownloadsFrom('https://download.eclipse.org/')

This will allow Spotless to download p2 metadata and jar files using the Equo p2 client. The metadata and jar files will be cached at ~/.equo. Alternatively, you can specify an alternative p2 update site, and add that URL to your allowDownloadsFrom.

It's not a perfect mechanism, it relies on our own diligence in making sure that PR's such as the Rome PR follow the rules about checking the allowed URLs. But it takes away the surprise, which is important. Users might choose a different formatter based on whether it is available through a standard maven proxy or not.

@blacelle
Copy link
Contributor

blacelle commented Apr 7, 2023

In mvn leg, this relates to <server> section. Some plugins does this : spotify-maven-plugin, or azure-maven-plugin Reading more about it, I feel it is unrelated to this matter (except if downloading from external sources would require specific additional customizable configuration (e.g. auth)).

allowDownloadsFrom('https://download.eclipse.org/')

Maybe a list of hosts would be a better fit.

I feel premature to consider mirroring capabilities, but I would add it in the radar.

The metadata and jar files will be cached at ~/.equo

This is also a relevant information for people integrating Spotless in a SaaS deployment :). (For Cleanthat, how can I customize this path?)

@nedtwigg
Copy link
Member Author

nedtwigg commented Apr 7, 2023

Maybe a list of hosts would be a better fit ... premature to consider mirroring capabilities ...

This is different formatter-to-formatter. For example, the Equo formatters download locations are based on this (customization info), and you can change mirroring like this. I agree that just an allowed-host list would work great for Eclipse.

But for the Rome formatter, it downloads to ~/.m2/repository/com/diffplug/spotless/spotless-data, and it downloads from https://github.com/rome/tools/releases/download/ (lots of people willing to add that URL that would be less comfortable with just github.com).

Equo is the latest change from non-downloading to downloading, but it's not the only place in the codebase where we break out of the expected maven-only behavior.

@blacelle
Copy link
Contributor

blacelle commented Apr 8, 2023

We agree Equo is just the latest example of non-mvn downloads.

I was about to argue than individual users or corporate users would be interested in URL-prefix whitelists or host-only whitelists. Writing about it, I rather feel:

  1. Corporate users are first interested is host-only whitelisting (similarly to PROXY, and possibly confusing this matter with whitelisting matter). (e.g. at first sight, whitelisting or not github.com would be enough)
  2. Corporate and individual users would be interested in URL-prefix whitelisting (as a way to safeguard download link (github.com being generally not sufficient)).

For my personal case (an individual and a corporate users), I'm still not fully convinced I care much about URL-prefix as:

  1. I trust Spotless (maybe too much trust, as Spotless itself relies on third-parties) hence whitelist('*') is generally fine with me
  2. But by corporate customers (which turned into Spotless Users) requires going through proxy, while they may host mirrors for large artifacts providers (m2central, npmregistry)
  3. The paranoid myself would need deeper control over what could be downloaded, but is hesitant is host-only is good-enough or URL-prefix is required.

The metadata and jar files will be cached at ~/.equo

I would suggest switching this to ~/.m2/repository/com/diffplug/spotless/spotless-data/equo

@blacelle
Copy link
Contributor

I would suggest switching this to ~/.m2/repository/com/diffplug/spotless/spotless-data/equo

Here is a similar request but gradle-related: #1524 (comment)

@SemyonRomankov
Copy link

Did I understand you right that you just hardcode "https://download.eclipse.org/' as the only one repo from you can fetch eclipse(versions 2.4+)? If the answer is "yes", I believe you should fix that, because any other repositories(mirrors) that we will pass to maven to fetch eclipse will be ignored.

@sann3
Copy link

sann3 commented Aug 1, 2024

We initially added download.eclipse.org to -Dhttp.nonProxyHosts in gradle build, but still it is failing with following exception

Caused by: java.io.IOException: Failed to load eclipse groovy formatter: java.lang.RuntimeException: java.io.IOException: Unexpected response code for CONNECT: 403
at com.diffplug.spotless.extra.EquoBasedStepBuilder.get(EquoBasedStepBuilder.java:116)
at com.diffplug.spotless.FormatterStepImpl.calculateState(FormatterStepImpl.java:58)
at com.diffplug.spotless.LazyForwardingEquality.state(LazyForwardingEquality.java:56)
... 225 more
Caused by: java.lang.RuntimeException: java.io.IOException: Unexpected response code for CONNECT: 403
at dev.equo.solstice.p2.Unchecked.wrap(Unchecked.java:25)
at dev.equo.solstice.p2.P2Model.query(P2Model.java:144)
at com.diffplug.spotless.extra.EquoBasedStepBuilder.get(EquoBasedStepBuilder.java:114)
... 227 more
Caused by: java.io.IOException: Unexpected response code for CONNECT: 403
at okhttp3.internal.connection.RealConnection.createTunnel(RealConnection.kt:483)
at okhttp3.internal.connection.RealConnection.connectTunnel(RealConnection.kt:262)

Then to find the actual issue we added --debug to gradle build and found that it is looking for groovy url

We got the groovy url from
https://github.com/diffplug/spotless/blob/main/lib-extra/src/main/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStep.java

So we added both download.eclipse.org,groovy.jfrog.io to -Dhttp.nonProxyHosts which solved the issue.

@gregallen
Copy link

gregallen commented Jan 24, 2025

Many firms do not allow repositories to be accessed by mirrors at all. Instead we have a single corporate mirror that presents a security scanned cache of several external sites. This is preconfigured and we do not want any plugins to attempt to modify the repository list.

Ideally there would be a well documented configuration option to disable adding repos. Also be good to be clear which tasks might be affected by selecting that

@gregallen
Copy link

There might be some configuration cache issue at play here. With latest gradle I see it trying to serialize the state of tasks which trigger an eclipse download attempt for spotlessGroovy task. This is unfortunate since we don't want nor use this task and so had not previously attempted to configure p2mirrors

@gregallen
Copy link

Problem is due to EquoStepBuilders state() method calling jarPromise.get()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants