Support Maven mirror configuration for tools downloads - #2099
Conversation
c9ac2d8 to
32b99d4
Compare
Signed-off-by: Partho Sarthi <psarthi@nvidia.com>
32b99d4 to
fe8fe2b
Compare
Signed-off-by: Partho Sarthi <psarthi@nvidia.com>
amahussein
left a comment
There was a problem hiding this comment.
For the scala webrawler thing: Do we actually need this feature? It felt like we needed in old days when the tools would point out to an outdated RAPIDS version. However, the RAPIDS added the buildEventInfo that dumps the information about the rapids jar used on the job. Given that RAPIDS is released bi-monthly we can get approximation that the RAPIDS is out-dated without doing any webcrawls.
IF we want to be realy precise on minor releases, then we can probe the minor version incrementally before we hit a miss.
In that case we won't need any webcrawling anymore.
On Python/build side:
- For someone configuring his mvn settings to use internal archive, mvn command should automatically work fine and fetch the jars from the internal repo.
- Github would still use the public repo which is fine as long as no one is using it for testing builds.
In the AutoTuner path, I agree that removing the web lookup entirely would be cleaner long term. We would have to figure out the semantics for getting an approximation that the plugin is out-dated.
For the Python/fat-wheel side, Maven settings do not cover the failing path. The CSP dependencies are read from JSON configs and downloaded through Python. For this MR, I wanted to have a less invasive fix by keeping the existing behavior, but let CI point the lookup to the internal Maven mirror in both cases. |
The PR should not merge until authenticated Maven requests are prevented from transmitting credentials over plaintext HTTP. Findings
|
| private def openMavenUrlStream( | ||
| mavenURL: String, | ||
| env: Map[String, String]): InputStream = { | ||
| val connection = new URL(mavenURL).openConnection() | ||
| getMavenBasicAuthHeader(env).foreach { authHeader => | ||
| connection.setRequestProperty("Authorization", authHeader) | ||
| } | ||
| connection.getInputStream | ||
| } |
There was a problem hiding this comment.
Missing connection and read timeouts on Maven URL connection
URLConnection.openConnection() uses the JVM default timeouts (effectively infinite). If the configured mirror is slow or unresponsive, connection.getInputStream can block indefinitely, stalling whichever CI job or tool invocation triggered getLatestMvnReleaseForNVPackage. The old code via XML.load(url) had the same problem, but now that an explicit openConnection() is exposed, it's straightforward to set timeouts. Consider calling connection.setConnectTimeout(...) and connection.setReadTimeout(...) before getInputStream.
| def get_maven_base_url(cls) -> str: | ||
| env_value = os.environ.get(cls.maven_base_url_env) | ||
| if env_value is None or env_value.strip() == '': | ||
| return cls.maven_central_base_url | ||
| return env_value.strip().rstrip('/') |
There was a problem hiding this comment.
Credentials Allow Plaintext Transport
If RAPIDS_TOOLS_MAVEN_BASE_URL uses http://, this code accepts the URL and the reachable metadata request attaches the configured Basic-auth username and password. The Scala implementation behaves the same way, and its new test explicitly combines an HTTP endpoint with credentials. A network observer can therefore recover the credentials. Reject authenticated non-HTTPS endpoints or only attach credentials to HTTPS URLs.
How this was verified: Both URL builders accept an arbitrary configured scheme, and their reachable metadata request paths attach the Basic Authorization header before opening that URL.
Fixes #2098
This PR lets RAPIDS tools use a configured Maven mirror for direct artifact and metadata downloads. Maven Central remains the default when no environment variables are set.
What Changed
RAPIDS_TOOLS_MAVEN_BASE_URLsupport in ScalaWebCrawlerUtil.RAPIDS_TOOLS_MAVEN_USERNAMEandRAPIDS_TOOLS_MAVEN_PASSWORDfor Basic Auth on Maven metadata and artifact page requests.Why
Some tools CI paths call Maven URLs directly instead of going through Maven settings. This includes the AutoTuner latest-plugin check:
Using env vars lets CI point those direct downloads at an internal mirror and avoid Maven Central throttling.
Env Vars
Testing
ToolUtilsSuitewith Maven mirror env set: passed.