Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
56 changes: 56 additions & 0 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jobs:
opa-services: ${{ steps.changes.outputs.opa-services }}
opa-builtins: ${{ steps.changes.outputs.opa-builtins }}
opa-slf4j: ${{ steps.changes.outputs.opa-slf4j }}
opa-proto: ${{ steps.changes.outputs.opa-proto }}
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
Expand All @@ -44,6 +45,7 @@ jobs:
echo "opa-services=true" >> $GITHUB_OUTPUT
echo "opa-builtins=true" >> $GITHUB_OUTPUT
echo "opa-slf4j=true" >> $GITHUB_OUTPUT
echo "opa-proto=true" >> $GITHUB_OUTPUT

if ! curl -s -o changed_files.json -H "Authorization: Bearer ${GITHUB_TOKEN}" \
"https://api.github.com/repos/${GH_REPO}/pulls/${PR_NUMBER}/files"; then
Expand Down Expand Up @@ -73,12 +75,14 @@ jobs:
opa_services=$(jq -r '.changes["opa-services"] // false' opa_result.json)
opa_builtins=$(jq -r '.changes["opa-builtins"] // false' opa_result.json)
opa_slf4j=$(jq -r '.changes["opa-slf4j"] // false' opa_result.json)
opa_proto=$(jq -r '.changes["opa-proto"] // false' opa_result.json)

echo "opa-evaluator=${opa_evaluator}" >> $GITHUB_OUTPUT
echo "opa-jackson=${opa_jackson}" >> $GITHUB_OUTPUT
echo "opa-services=${opa_services}" >> $GITHUB_OUTPUT
echo "opa-builtins=${opa_builtins}" >> $GITHUB_OUTPUT
echo "opa-slf4j=${opa_slf4j}" >> $GITHUB_OUTPUT
echo "opa-proto=${opa_proto}" >> $GITHUB_OUTPUT

gh-actions-lint:
name: GitHub Actions Lint
Expand Down Expand Up @@ -286,6 +290,56 @@ jobs:
- uses: gradle/actions/setup-gradle@3f131e8634966bd73d06cc69884922b02e6faf92 # v6.2.0
- run: ./gradlew :opa-slf4j:test

test-opa-proto:
needs: check-changes
if: needs.check-changes.outputs.opa-proto == 'true'
runs-on: ubuntu-latest
strategy:
matrix:
java: [17, 21] # 17 is the minimum supported / release JDK
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5.6.0
with:
distribution: temurin
java-version: ${{ matrix.java }}
- uses: gradle/actions/setup-gradle@3f131e8634966bd73d06cc69884922b02e6faf92 # v6.2.0
# ProtoComplianceTest reads the compliance fixtures from the sibling opa-evaluator module
# (../opa-evaluator/...); the full checkout above makes that relative path resolve.
- run: ./gradlew :opa-proto:test

verify-proto-vendor:
name: Verify vendored proto schemas match go.mod
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5.6.0
with:
distribution: temurin
java-version: 21
- uses: gradle/actions/setup-gradle@3f131e8634966bd73d06cc69884922b02e6faf92 # v6.2.0
- uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version-file: tools/generate-compliance-tests/go.mod
# The vendored .proto files in opa-proto must always match the OPA version
# pinned in tools/generate-compliance-tests/go.mod (the source of truth).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we generating and running protobuf variants of all the compliance test cases too?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like no. This is left for a future task?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your right there isn't a protobuf variant, but there should be! I hadn't thought about it. This verify-proto-vendor job here just checks the vendored proto schemas match the go.mod pinned OPA version. Probably better if it happens in this PR, will work on adding a proto variant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protobuf variant test added! Does have the negative side effect of having to regenerate all of the compliance tests to match the latest OPA so the file diff is crazy.

# Re-vendor via the Gradle task and fail if it produces any change — i.e. the
# OPA version was bumped, or a .proto was hand-edited, without re-vendoring.
- name: Re-vendor and check for drift
run: |
set -e
./gradlew --rerun-tasks :opa-proto:vendorProtoSchemas
if ! git diff --quiet -- opa-proto/src/main/proto; then
echo "::error::Vendored proto schemas are out of sync with the OPA version in tools/generate-compliance-tests/go.mod. Run ./gradlew :opa-proto:vendorProtoSchemas and commit the result."
git --no-pager diff -- opa-proto/src/main/proto
exit 1
fi
echo "Vendored proto schemas are in sync with the pinned OPA version."

pr-check-summary:
name: PR Check Summary
runs-on: ubuntu-latest
Expand All @@ -297,11 +351,13 @@ jobs:
- lint
- validate-pom
- build
- verify-proto-vendor
- test-opa-evaluator
- test-opa-jackson
- test-opa-services
- test-opa-builtins
- test-opa-slf4j
- test-opa-proto
if: always()
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
Expand Down
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
*.tar.gz
*.rar

# …but keep bundle fixtures checked in as test resources
!**/src/test/resources/**/*.tar.gz

# Build directories
build/
target/
Expand Down Expand Up @@ -60,3 +63,6 @@ nb-configuration.xml
.DS_Store
Thumbs.db

# Go — compiled helper tool binary (built by `go build` in the compliance-test tool)
/tools/generate-compliance-tests/generate-compliance-tests

1 change: 1 addition & 0 deletions cli/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ dependencies {

runtimeOnly(project(":opa-builtins"))
runtimeOnly(project(":opa-jackson"))
runtimeOnly(project(":opa-proto"))

testImplementation("org.junit.jupiter:junit-jupiter:6.1.2")
testRuntimeOnly("org.junit.platform:junit-platform-launcher:6.1.2")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
import io.github.open_policy_agent.opa.ast.types.RegoObject;
import io.github.open_policy_agent.opa.ast.types.RegoString;
import io.github.open_policy_agent.opa.ir.PolicyReader;
import io.github.open_policy_agent.opa.spi.Services;
import io.github.open_policy_agent.opa.storage.Store;

import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.ServiceLoader;

/**
* Processes individual bundle files and assembles them into a {@link Bundle}.
Expand Down Expand Up @@ -38,39 +36,43 @@ public class BundleAssembler {

static final BundleParser BUNDLE_PARSER = loadSingleton(BundleParser.class);

// Optional: null when opa-proto is absent (JSON-only bundles still work), the single
// implementation when exactly one is registered, and loadOptional throws on duplicates — a
// classpath error we want to surface early, just like POLICY_READER and BUNDLE_PARSER.
static final ProtoBundleDecoder PROTO_DECODER = loadOptional(ProtoBundleDecoder.class);

/**
* Loads exactly one implementation of the given SPI from the classpath. Throws if zero or more
* than one implementation is registered, since either case produces an ambiguous runtime.
*/
private static <T> T loadSingleton(Class<T> spi) {
List<T> impls = new ArrayList<>();
for (T impl : ServiceLoader.load(spi)) {
impls.add(impl);
}
if (impls.isEmpty()) {
throw new IllegalStateException(
"No "
+ spi.getSimpleName()
+ " implementation found on the classpath. Add a module that provides "
+ spi.getSimpleName()
+ " (e.g. opa-jackson).");
}
if (impls.size() > 1) {
StringBuilder names = new StringBuilder();
for (int i = 0; i < impls.size(); i++) {
if (i > 0) {
names.append(", ");
}
names.append(impls.get(i).getClass().getName());
}
return Services.loadAtMostOne(spi)
.orElseThrow(
() ->
new IllegalStateException(
"No "
+ spi.getSimpleName()
+ " implementation found on the classpath. Add a module that provides "
+ spi.getSimpleName()
+ " (e.g. opa-jackson)."));
}

/**
* Loads an optional SPI: returns {@code null} when no implementation is registered, the single
* implementation when exactly one is, and throws when more than one is registered.
*/
private static <T> T loadOptional(Class<T> spi) {
return Services.loadAtMostOne(spi).orElse(null);
}

private static ProtoBundleDecoder requireProtoDecoder() {
if (PROTO_DECODER == null) {
throw new IllegalStateException(
"Multiple "
+ spi.getSimpleName()
+ " implementations found on the classpath: "
+ names
+ ". Only one provider may be registered.");
"No ProtoBundleDecoder implementation found on the classpath, but the bundle is in proto"
+ " format (plan.pb / .manifest.pb). Add the opa-proto module to read proto-format"
+ " bundles.");
}
return impls.get(0);
return PROTO_DECODER;
}

private final Bundle.Builder builder = new Bundle.Builder();
Expand All @@ -83,6 +85,16 @@ public void loadPlan(InputStream in) throws IOException {
hasContent = true;
}

/**
* Load a compiled IR policy from a protobuf {@code plan.pb} stream.
*
* @throws IllegalStateException if no {@link ProtoBundleDecoder} is registered (add opa-proto)
*/
public void loadPlanProto(InputStream in) throws IOException {
builder.withIrPolicy(requireProtoDecoder().decodePlan(in));
hasContent = true;
}

/**
* Load data from a {@code data.json} at the given path within the bundle.
*
Expand Down Expand Up @@ -132,6 +144,63 @@ public void loadManifest(InputStream in) throws IOException {
builder.withManifest(BUNDLE_PARSER.parseManifest(in));
}

/**
* Load bundle metadata from a protobuf {@code .manifest.pb} stream.
*
* @throws IllegalStateException if no {@link ProtoBundleDecoder} is registered (add opa-proto)
*/
public void loadManifestProto(InputStream in) throws IOException {
builder.withManifest(requireProtoDecoder().decodeManifest(in));
}

/**
* Detect the bundle's wire format from which plan/manifest artifacts are present, reject
* mixed-format bundles, and load the plan and manifest in the correct format.
*
* <p>This centralizes the format-detection and routing policy so {@link BundleLoader}
* implementations only need to locate the four possible artifacts and expose each as an {@link
* InputStreamSource} (or {@code null} when absent), regardless of whether the bytes come from a
* directory, a tarball, or elsewhere. Proto takes precedence over JSON for each artifact, matching
* OPA; a proto plan cannot be paired with a JSON manifest and vice versa (see {@link
* BundleFormat#validate}).
*
* @param planJson source for {@code plan.json}, or {@code null} if absent
* @param planProto source for {@code plan.pb}, or {@code null} if absent
* @param manifestJson source for {@code .manifest}, or {@code null} if absent
* @param manifestProto source for {@code .manifest.pb}, or {@code null} if absent
* @throws IllegalArgumentException if the bundle mixes wire formats
* @throws IOException if a source cannot be opened or its contents cannot be parsed
*/
public void loadPlanAndManifest(
InputStreamSource planJson,
InputStreamSource planProto,
InputStreamSource manifestJson,
InputStreamSource manifestProto)
Comment thread
sspaink marked this conversation as resolved.
throws IOException {
BundleFormat.validate(
planJson != null, planProto != null, manifestJson != null, manifestProto != null);

if (planProto != null) {
try (InputStream in = planProto.open()) {
loadPlanProto(in);
}
} else if (planJson != null) {
try (InputStream in = planJson.open()) {
loadPlan(in);
}
}

if (manifestProto != null) {
try (InputStream in = manifestProto.open()) {
loadManifestProto(in);
}
} else if (manifestJson != null) {
try (InputStream in = manifestJson.open()) {
loadManifest(in);
}
}
}

/** Add a Rego source file by its relative path. */
public void addRego(String path, String content) {
builder.withRego(path, content);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package io.github.open_policy_agent.opa.bundle;

/**
* Constants and validation for the two on-disk bundle wire formats.
*
* <p>OPA's {@code opa build} emits plan bundles in either JSON (the default) or protobuf form. The
* two forms use distinct filenames for the plan and manifest:
*
* <table border="1">
* <caption>Format-specific filenames</caption>
* <tr><th>Artifact</th><th>JSON</th><th>Proto</th></tr>
* <tr><td>Plan</td><td>{@code plan.json}</td><td>{@code plan.pb}</td></tr>
* <tr><td>Manifest</td><td>{@code .manifest}</td><td>{@code .manifest.pb}</td></tr>
* </table>
*
* <p>Data files ({@code data.json}) are JSON in both forms; only the plan and manifest change.
*
* <p>OPA rejects bundles whose plan and manifest formats disagree (e.g. {@code plan.pb} paired with
* a JSON {@code .manifest}). {@link BundleLoader} implementations call {@link #validate} to enforce
* the same rule and to reject bundles that ambiguously contain both formats of the same artifact.
*/
public final class BundleFormat {

/** Filename of a JSON-format IR plan. */
public static final String PLAN_JSON = "plan.json";

/** Filename of a protobuf-format IR plan. */
public static final String PLAN_PROTO = "plan.pb";

/** Filename of a JSON-format bundle manifest. */
public static final String MANIFEST_JSON = ".manifest";

/** Filename of a protobuf-format bundle manifest. */
public static final String MANIFEST_PROTO = ".manifest.pb";

private BundleFormat() {}

/**
* Reject bundles that mix the two wire formats.
*
* <p>Fails if a bundle contains both formats of the same artifact (e.g. both {@code plan.json} and
* {@code plan.pb}), or if a plan and a manifest are present in disagreeing formats (e.g. a proto
* plan with a JSON manifest), matching OPA's own auto-detection semantics. A bundle carrying only
* a plan or only a manifest is always accepted.
*
* @param hasPlanJson whether a {@code plan.json} is present
* @param hasPlanProto whether a {@code plan.pb} is present
* @param hasManifestJson whether a {@code .manifest} is present
* @param hasManifestProto whether a {@code .manifest.pb} is present
* @throws IllegalArgumentException if the bundle mixes formats
*/
public static void validate(
boolean hasPlanJson,
boolean hasPlanProto,
boolean hasManifestJson,
boolean hasManifestProto) {
if (hasPlanJson && hasPlanProto) {
throw new IllegalArgumentException(
"Bundle contains both " + PLAN_JSON + " and " + PLAN_PROTO + "; plan format is ambiguous");
}
if (hasManifestJson && hasManifestProto) {
throw new IllegalArgumentException(
"Bundle contains both "
+ MANIFEST_JSON
+ " and "
+ MANIFEST_PROTO
+ "; manifest format is ambiguous");
}
if (hasPlanProto && hasManifestJson) {
throw new IllegalArgumentException(
"Bundle mixes plan and manifest formats: proto plan ("
+ PLAN_PROTO
+ ") with JSON manifest ("
+ MANIFEST_JSON
+ ")");
}
if (hasPlanJson && hasManifestProto) {
throw new IllegalArgumentException(
"Bundle mixes plan and manifest formats: JSON plan ("
+ PLAN_JSON
+ ") with proto manifest ("
+ MANIFEST_PROTO
+ ")");
}
}
}
Loading
Loading