Conversation
Ranger validates policy access types against the registered service definition, but until now the serviceDef only existed inside the unit-test fixture, so operators had no artifact to register with Ranger Admin. Extract it into extensions/auth/ranger/src/main/resources/ polaris-ranger-servicedef.json as the artifact operators register, and generate the unit/intTest authz fixtures from it at build time (via new generateAuthzTestFixture/generateAuthzItTestFixture Gradle tasks) so the shipped artifact and the serviceDef the tests actually exercise can no longer drift apart.
These are raw JSON with a build-time placeholder token spliced in, not standalone JSON/source files that can carry a license header of their own.
| val serviceDef = polarisRangerServiceDefFile.asFile.readText().trim() | ||
| val merged = templateFile.asFile.readText().replace(servicedefPlaceholder, serviceDef) | ||
| val outDir = outputDir.get().dir("authz_tests").asFile.apply { mkdirs() } | ||
| outDir.resolve("dev_polaris.json").writeText(merged) |
There was a problem hiding this comment.
Producing these resources in gradle is fine, but it's a bit detached from the related test code 🤔
Would it be possible to switch to LocalFolderPolicySource instead of EmbeddedResourcePolicySource and let test java code create the policy file in a temp dir (provided by JUnit)?
Cf.
There was a problem hiding this comment.
Done in 7774562 — switched RangerPolarisAuthorizerTest to LocalFolderPolicySource and moved the splice into RangerTestUtils.createConfig(Path), writing into a JUnit-managed @TempDir at test-run time instead of a build-time Gradle task. The intTest fixture stays Gradle-generated since it configures a separate server process that starts before any test code runs, so there's no JUnit lifecycle to hook into there.
|
|
||
| // Ranger authz test fixture templates: raw JSON with a build-time placeholder token spliced | ||
| // in, so they're not standalone JSON/source files that can carry their own license header. | ||
| excludes.add("**/*.json.template") |
There was a problem hiding this comment.
Since we process these files in our code, we could add the header to the source and ignore it in runtime... WDYT?
There was a problem hiding this comment.
... but JSON supports comments, does it not?
There was a problem hiding this comment.
Good push — you're right that these are consumed by our own text-processing code, not a JSON parser directly (they were never valid JSON to begin with, thanks to the placeholder). Added the ASF header as a plain block comment at the top of both .template files, and had the generating code strip everything up to the closing */ before splicing/writing, so it never leaks into the actual JSON that gets parsed at runtime. Dropped the rat exclude entirely — done in 7774562.
There was a problem hiding this comment.
Correct — standard JSON has no comment syntax, and these files were never meant to be parsed as strict JSON directly (the placeholder alone makes them invalid). Since we own the code that reads them, added the license as a plain block comment and stripped it in that same code before the result gets written as real JSON. See the reply above.
…r check" This reverts commit 41084fc.
…Gradle (PR review) Switch RangerPolarisAuthorizerTest from EmbeddedResourcePolicySource to LocalFolderPolicySource, and have RangerTestUtils splice the shipped serviceDef into the template at test-run time, writing into a JUnit-managed @tempdir. This keeps the fixture generation next to the test that consumes it instead of a build-time Gradle task; the intTest fixture stays Gradle-generated since it configures a separate server process that starts before any test code runs. Also add the ASF license header to both dev_polaris.json.template files (stripped by the generating code before use) instead of excluding them from the rat check, since JSON's lack of comment support doesn't apply here - these were never valid JSON to begin with.
|
FYI: @sneethiraj @mneethiraj |
|
@obelix74 : The commits here have Claude as a formal co-author. Would you mind converting them to personal commits? |
7774562 to
0b83034
Compare
flyingImer
left a comment
There was a problem hiding this comment.
LGTM. Reusing the shipped service definition in the authorization tests avoids maintaining separate copies, love it!
| ``` | ||
|
|
||
| 3. Run or restart Polaris to see that all accesses are authorized by Ranger policies, with access audit records available in Apache Ranger console. | ||
| 3. Register the Polaris service type with Ranger Admin using the service definition shipped at |
There was a problem hiding this comment.
Service-def for Polaris is implicitly registered starting with Ranger 2.9.0. So, in environments using 2.9.0 or above, step 3 is not necessary.
Summary
Ranger validates policy access types against the registered service definition, but until now the
serviceDefonly existed inside theRangerPolarisAuthorizerTestunit-test fixture — operators had no artifact to actually register with Ranger Admin.serviceDefintoextensions/auth/ranger/src/main/resources/polaris-ranger-servicedef.json, the artifact operators register with Ranger Admin (documented in the updated README).dev_polaris.jsonauthz fixtures from this shipped artifact at build time (generateAuthzTestFixture/generateAuthzItTestFixtureGradle tasks), rather than checking in a byte-for-byte copy, so the shipped artifact and theserviceDefthe tests actually exercise can no longer drift apart.Split out of #4115 per review feedback — this part is unrelated to the metrics query API and stands on its own; #4115 will be rebased on top of this once merged, carrying only the metrics-specific
table-metrics-readdelta.Test plan
./gradlew :polaris-extensions-auth-ranger:testpasses (11/11)🤖 Generated with Claude Code