From 306a3b0fc21da6314ce4ba1a683f2990a99c544b Mon Sep 17 00:00:00 2001 From: Daniel Chao Date: Tue, 22 Jul 2025 15:42:07 -0700 Subject: [PATCH] Polish http rewrites (#1133) * Polish rewrite docs * Add documentation comments, add missing evaluator options * Add ability to set HTTP builder in ConfigEvaluatorBuilder * Add ability to set rewrites in executor API --- .../config/java/ConfigEvaluatorBuilder.java | 32 ++++++++-- .../java/org/pkl/core/http/HttpClient.java | 27 ++++++++ .../org/pkl/core/service/ExecutorSpiImpl.java | 54 ++++++---------- .../org/pkl/executor/ExecutorOptions.java | 56 ++++++++++++++++- .../executor/spi/v1/ExecutorSpiOptions3.java | 63 +++++++++++++++++++ .../org/pkl/executor/EmbeddedExecutorTest.kt | 43 ++++++++++--- stdlib/EvaluatorSettings.pkl | 4 +- 7 files changed, 225 insertions(+), 54 deletions(-) create mode 100644 pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions3.java diff --git a/pkl-config-java/src/main/java/org/pkl/config/java/ConfigEvaluatorBuilder.java b/pkl-config-java/src/main/java/org/pkl/config/java/ConfigEvaluatorBuilder.java index e909f279..e2165a59 100644 --- a/pkl-config-java/src/main/java/org/pkl/config/java/ConfigEvaluatorBuilder.java +++ b/pkl-config-java/src/main/java/org/pkl/config/java/ConfigEvaluatorBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ import org.pkl.config.java.mapper.ValueMapperBuilder; import org.pkl.core.EvaluatorBuilder; import org.pkl.core.SecurityManager; import org.pkl.core.StackFrameTransformer; +import org.pkl.core.http.HttpClient; import org.pkl.core.project.DeclaredDependencies; import org.pkl.core.project.Project; import org.pkl.core.util.Nullable; @@ -242,6 +243,15 @@ public final class ConfigEvaluatorBuilder { return this; } + /** + * Returns the currently set evaluation timeout. + * + *

This is a convenience method that delegates to the underlying evaluator builder. + */ + public @Nullable Duration getTimeout() { + return evaluatorBuilder.getTimeout(); + } + /** * Sets the set of URI patterns to be allowed when importing modules. * @@ -305,12 +315,24 @@ public final class ConfigEvaluatorBuilder { } /** - * Returns the currently set evaluation timeout. + * Sets the HTTP Client to be used. * - *

This is a convenience method that delegates to the underlying evaluator builder. + *

Defaults to {@code HttpClient.builder().buildLazily()}. + * + * @since 0.29.0 */ - public @Nullable Duration getTimeout() { - return evaluatorBuilder.getTimeout(); + public ConfigEvaluatorBuilder setHttpClient(HttpClient httpClient) { + evaluatorBuilder.setHttpClient(httpClient); + return this; + } + + /** + * Returns the currently set HTTP client. + * + * @since 0.29.0 + */ + public HttpClient getHttpClient() { + return evaluatorBuilder.getHttpClient(); } /** diff --git a/pkl-core/src/main/java/org/pkl/core/http/HttpClient.java b/pkl-core/src/main/java/org/pkl/core/http/HttpClient.java index b463dd60..c0d9c4bd 100644 --- a/pkl-core/src/main/java/org/pkl/core/http/HttpClient.java +++ b/pkl-core/src/main/java/org/pkl/core/http/HttpClient.java @@ -119,8 +119,35 @@ public interface HttpClient extends AutoCloseable { */ Builder setProxy(@Nullable URI proxyAddress, List noProxy); + /** + * Removes any existing rewrites, then adds the given rewrites. + * + *

A rewrite changes outbound HTTP URLs by replacing a source prefix with a targert prefix. + * + *

Each rewrite URI must start with {@code http://} or {@code https://}, and end with {@code + * /}. + * + *

Each key describes the prefix of a request, and each value describes the replacement + * prefix. + * + *

This can be useful for setting up mirroring of packages, which are fetched over HTTPS. + * + *

In the case of multiple matches, the longest prefix is used. + * + *

The URL hostname is case-insensitive. + * + * @throws IllegalArgumentException if {@code rewrites} is invalid. + * @since 0.29.0 + */ Builder setRewrites(Map rewrites); + /** + * Adds a rewrite rule. + * + * @see Builder#setRewrites(Map) + * @throws IllegalArgumentException if {@code sourcePrefix} or {@code targetPrefix} is invalid. + * @since 0.29.0 + */ Builder addRewrite(URI sourcePrefix, URI targetPrefix); /** diff --git a/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java b/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java index 9cce6ecb..2f865a7d 100644 --- a/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java +++ b/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,14 +17,13 @@ package org.pkl.core.service; import static org.pkl.core.module.ProjectDependenciesManager.PKL_PROJECT_FILENAME; +import java.net.URI; import java.nio.file.Path; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.Objects; -import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; import org.pkl.core.*; @@ -37,6 +36,7 @@ import org.pkl.executor.spi.v1.ExecutorSpi; import org.pkl.executor.spi.v1.ExecutorSpiException; import org.pkl.executor.spi.v1.ExecutorSpiOptions; import org.pkl.executor.spi.v1.ExecutorSpiOptions2; +import org.pkl.executor.spi.v1.ExecutorSpiOptions3; public final class ExecutorSpiImpl implements ExecutorSpi { private static final int MAX_HTTP_CLIENTS = 3; @@ -142,8 +142,14 @@ public final class ExecutorSpiImpl implements ExecutorSpi { private HttpClient getOrCreateHttpClient(ExecutorSpiOptions options) { List certificateFiles; List certificateBytes; + Map rewrites; int testPort; try { + if (options instanceof ExecutorSpiOptions3 options3) { + rewrites = options3.getHttpRewrites(); + } else { + rewrites = Map.of(); + } if (options instanceof ExecutorSpiOptions2 options2) { certificateFiles = options2.getCertificateFiles(); certificateBytes = options2.getCertificateBytes(); @@ -153,14 +159,15 @@ public final class ExecutorSpiImpl implements ExecutorSpi { certificateBytes = List.of(); testPort = -1; } - // host pkl-executor does not have class ExecutorOptions2 defined. + // host pkl-executor does not have class ExecutorOptions2/ExecutorOptions3 defined. // this will happen if the pkl-executor distribution is too old. } catch (NoClassDefFoundError e) { certificateFiles = List.of(); certificateBytes = List.of(); + rewrites = Map.of(); testPort = -1; } - var clientKey = new HttpClientKey(certificateFiles, certificateBytes, testPort); + var clientKey = new HttpClientKey(certificateFiles, certificateBytes, testPort, rewrites); return httpClients.computeIfAbsent( clientKey, (key) -> { @@ -171,6 +178,7 @@ public final class ExecutorSpiImpl implements ExecutorSpi { for (var bytes : key.certificateBytes) { builder.addCertificates(bytes); } + builder.setRewrites(key.rewrites); builder.setTestPort(key.testPort); // If the above didn't add any certificates, // builder will use the JVM's default SSL context. @@ -178,35 +186,9 @@ public final class ExecutorSpiImpl implements ExecutorSpi { }); } - private static final class HttpClientKey { - final Set certificateFiles; - final Set certificateBytes; - final int testPort; - - HttpClientKey(List certificateFiles, List certificateBytes, int testPort) { - // also serves as defensive copy - this.certificateFiles = Set.copyOf(certificateFiles); - this.certificateBytes = Set.copyOf(certificateBytes); - this.testPort = testPort; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null || getClass() != obj.getClass()) { - return false; - } - HttpClientKey that = (HttpClientKey) obj; - return certificateFiles.equals(that.certificateFiles) - && certificateBytes.equals(that.certificateBytes) - && testPort == that.testPort; - } - - @Override - public int hashCode() { - return Objects.hash(certificateFiles, certificateBytes, testPort); - } - } + private record HttpClientKey( + List certificateFiles, + List certificateBytes, + int testPort, + Map rewrites) {} } diff --git a/pkl-executor/src/main/java/org/pkl/executor/ExecutorOptions.java b/pkl-executor/src/main/java/org/pkl/executor/ExecutorOptions.java index e6e3d17f..27c92f9e 100644 --- a/pkl-executor/src/main/java/org/pkl/executor/ExecutorOptions.java +++ b/pkl-executor/src/main/java/org/pkl/executor/ExecutorOptions.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,7 @@ */ package org.pkl.executor; +import java.net.URI; import java.nio.file.Path; import java.time.Duration; import java.util.List; @@ -22,6 +23,7 @@ import java.util.Map; import java.util.Objects; import org.pkl.executor.spi.v1.ExecutorSpiOptions; import org.pkl.executor.spi.v1.ExecutorSpiOptions2; +import org.pkl.executor.spi.v1.ExecutorSpiOptions3; /** * Options for {@link Executor#evaluatePath}. @@ -53,6 +55,8 @@ public final class ExecutorOptions { private final List certificateBytes; + private final Map httpRewrites; + private final int testPort; // -1 means disabled private final int spiOptionsVersion; // -1 means use latest @@ -84,6 +88,7 @@ public final class ExecutorOptions { private /* @Nullable */ Path projectDir; private List certificateFiles = List.of(); private List certificateBytes = List.of(); + private Map httpRewrites = Map.of(); private int testPort = -1; // -1 means disabled private int spiOptionsVersion = -1; // -1 means use latest @@ -197,6 +202,18 @@ public final class ExecutorOptions { return this; } + /** + * API equivalent of the {@code --http-rewrite} CLI option. + * + *

This option is ignored on Pkl 0.28 and older. + * + * @since 0.29.0 + */ + public Builder httpRewrites(Map httpRewrites) { + this.httpRewrites = httpRewrites; + return this; + } + /** Internal test option. -1 means disabled. */ Builder testPort(int testPort) { this.testPort = testPort; @@ -223,6 +240,7 @@ public final class ExecutorOptions { projectDir, certificateFiles, certificateBytes, + httpRewrites, testPort, spiOptionsVersion); } @@ -271,6 +289,7 @@ public final class ExecutorOptions { projectDir, List.of(), List.of(), + Map.of(), -1, -1); } @@ -288,6 +307,7 @@ public final class ExecutorOptions { /* @Nullable */ Path projectDir, List certificateFiles, List certificateBytes, + Map httpRewrites, int testPort, int spiOptionsVersion) { @@ -303,6 +323,7 @@ public final class ExecutorOptions { this.projectDir = projectDir; this.certificateFiles = List.copyOf(certificateFiles); this.certificateBytes = List.copyOf(certificateBytes); + this.httpRewrites = Map.copyOf(httpRewrites); this.testPort = testPort; this.spiOptionsVersion = spiOptionsVersion; } @@ -374,6 +395,17 @@ public final class ExecutorOptions { return certificateBytes; } + /** + * API equivalent of the {@code --http-rewrite} CLI option. + * + *

This option is ignored on Pkl 0.28 and older. + * + * @since 0.29.0 + */ + public Map getHttpRewrites() { + return httpRewrites; + } + @Override public boolean equals(/* @Nullable */ Object obj) { if (this == obj) return true; @@ -392,6 +424,7 @@ public final class ExecutorOptions { && Objects.equals(projectDir, other.projectDir) && Objects.equals(certificateFiles, other.certificateFiles) && Objects.equals(certificateBytes, other.certificateBytes) + && Objects.equals(httpRewrites, other.httpRewrites) && testPort == other.testPort && spiOptionsVersion == other.spiOptionsVersion; } @@ -411,6 +444,7 @@ public final class ExecutorOptions { projectDir, certificateFiles, certificateBytes, + httpRewrites, testPort, spiOptionsVersion); } @@ -442,6 +476,8 @@ public final class ExecutorOptions { + certificateFiles + ", certificateBytes=" + certificateBytes + + ", httpRewrites=" + + httpRewrites + ", testPort=" + testPort + ", spiOptionsVersion=" @@ -451,7 +487,23 @@ public final class ExecutorOptions { ExecutorSpiOptions toSpiOptions() { return switch (spiOptionsVersion) { - case -1, 2 -> + case -1, 3 -> + new ExecutorSpiOptions3( + allowedModules, + allowedResources, + environmentVariables, + externalProperties, + modulePath, + rootDir, + timeout, + outputFormat, + moduleCacheDir, + projectDir, + certificateFiles, + certificateBytes, + testPort, + httpRewrites); + case 2 -> new ExecutorSpiOptions2( allowedModules, allowedResources, diff --git a/pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions3.java b/pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions3.java new file mode 100644 index 00000000..81c910e4 --- /dev/null +++ b/pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions3.java @@ -0,0 +1,63 @@ +/* + * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pkl.executor.spi.v1; + +import java.net.URI; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; +import java.util.Map; + +public class ExecutorSpiOptions3 extends ExecutorSpiOptions2 { + + private final Map httpRewrites; + + public ExecutorSpiOptions3( + List allowedModules, + List allowedResources, + Map environmentVariables, + Map externalProperties, + List modulePath, + Path rootDir, + Duration timeout, + String outputFormat, + Path moduleCacheDir, + Path projectDir, + List certificateFiles, + List certificateBytes, + int testPort, + Map httpRewrites) { + super( + allowedModules, + allowedResources, + environmentVariables, + externalProperties, + modulePath, + rootDir, + timeout, + outputFormat, + moduleCacheDir, + projectDir, + certificateFiles, + certificateBytes, + testPort); + this.httpRewrites = httpRewrites; + } + + public Map getHttpRewrites() { + return httpRewrites; + } +} diff --git a/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt b/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt index 4e3b4e26..4243feae 100644 --- a/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt +++ b/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt @@ -16,12 +16,15 @@ package org.pkl.executor import java.io.File +import java.net.URI import java.nio.file.Files import java.nio.file.Path import java.time.Duration import kotlin.io.path.createDirectories import kotlin.io.path.exists +import kotlin.io.path.writeText import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatCode import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows @@ -64,9 +67,9 @@ class EmbeddedExecutorTest { // This context has a pkl-executor version that is lower than the distribution version. TestExecutor(executor1_2.value, 1, "SpiOptions1, Executor1, Distribution2"), TestExecutor(executor2_1.value, 1, "SpiOptions1, Executor2, Distribution1"), - TestExecutor(executor2_1.value, 2, "SpiOptions2, Executor2, Distribution1"), + TestExecutor(executor2_1.value, 3, "SpiOptions3, Executor2, Distribution1"), TestExecutor(executor2_2.value, 1, "SpiOptions1, Executor2, Distribution2"), - TestExecutor(executor2_2.value, 2, "SpiOptions2, Executor2, Distribution2"), + TestExecutor(executor2_2.value, 3, "SpiOptions3, Executor2, Distribution2"), ) } @@ -81,19 +84,19 @@ class EmbeddedExecutorTest { } // A pkl-executor library that supports ExecutorSpiOptions up to v1 - // and a Pkl distribution that supports ExecutorSpiOptions up to v2. + // and a Pkl distribution that supports ExecutorSpiOptions up to v3. private val executor1_2: Lazy = lazy { EmbeddedExecutor(listOf(pklDistribution2), pklExecutorClassLoader1) } - // A pkl-executor library that supports ExecutorSpiOptions up to v2 + // A pkl-executor library that supports ExecutorSpiOptions up to v3 // and a Pkl distribution that supports ExecutorSpiOptions up to v1. private val executor2_1: Lazy = lazy { EmbeddedExecutor(listOf(pklDistribution1), pklExecutorClassLoader2) } - // A pkl-executor library that supports ExecutorSpiOptions up to v2 - // and a Pkl distribution that supports ExecutorSpiOptions up to v2. + // A pkl-executor library that supports ExecutorSpiOptions up to v3 + // and a Pkl distribution that supports ExecutorSpiOptions up to v3. private val executor2_2: Lazy = lazy { EmbeddedExecutor(listOf(pklDistribution2), pklExecutorClassLoader2) } @@ -103,11 +106,11 @@ class EmbeddedExecutorTest { // a pkl-executor class loader that supports ExecutorSpiOptions up to v1 private val pklExecutorClassLoader1: ClassLoader by lazy { FilteringClassLoader(pklExecutorClassLoader2) { className -> - !className.endsWith("ExecutorSpiOptions2") + !className.matches(Regex(".*ExecutorSpiOptions\\d+$")) } } - // a pkl-executor class loader that supports ExecutorSpiOptions up to v2 + // a pkl-executor class loader that supports ExecutorSpiOptions up to v3 private val pklExecutorClassLoader2: ClassLoader by lazy { EmbeddedExecutor::class.java.classLoader } @@ -127,7 +130,7 @@ class EmbeddedExecutorTest { .apply { if (!exists()) missingTestFixture() } } - // a Pkl distribution that supports ExecutorSpiOptions up to v2 + // a Pkl distribution that supports ExecutorSpiOptions up to v3 private val pklDistribution2: Path by lazy { FileTestUtils.rootProjectDir .resolve( @@ -556,6 +559,28 @@ class EmbeddedExecutorTest { assertThat(cacheDir.toFile().list()).isNotEmpty() } + @Test + fun `http rewrites option`(@TempDir tempDir: Path) { + val pklFile = tempDir.resolve("test.pkl") + pklFile.writeText( + """ + @ModuleInfo { minPklVersion = "0.29.0" } + result = import("https://example.com/foo.pkl") + """ + .trimIndent() + ) + assertThatCode { + currentExecutor.evaluatePath(pklFile) { + allowedModules("file:", "https:") + allowedResources("prop:") + httpRewrites(mapOf(URI("https://example.com/") to URI("https://example.example/"))) + } + } + .hasMessageContaining( + "Error connecting to host `example.example`. (request was rewritten: https://example.com/foo.pkl -> https://example.example/foo.pkl)" + ) + } + @ParameterizedTest @MethodSource("getAllTestExecutors") @DisabledOnOs(OS.WINDOWS, disabledReason = "Can't populate legacy cache dir on Windows") diff --git a/stdlib/EvaluatorSettings.pkl b/stdlib/EvaluatorSettings.pkl index 4afc2784..76c84064 100644 --- a/stdlib/EvaluatorSettings.pkl +++ b/stdlib/EvaluatorSettings.pkl @@ -133,8 +133,8 @@ class Http { /// Each key describes the prefix of a request, and each value describes the replacement prefix. /// /// This can be useful for setting up mirroring of packages, which are fetched over HTTPS. - // - /// In the case of multiple matches, the longest prefix is is used. + /// + /// In the case of multiple matches, the longest prefix is used. /// /// The URL hostname is case-insensitive. ///