Skip to content

fix(ranger): ship operator-consumable Ranger service def - #5494

Open
obelix74 wants to merge 5 commits into
apache:mainfrom
obelix74:ranger-servicedef-base
Open

obelix74 wants to merge 5 commits into
apache:mainfrom
obelix74:ranger-servicedef-base

Conversation

@obelix74

Copy link
Copy Markdown
Contributor

Summary

Ranger validates policy access types against the registered service definition, but until now the serviceDef only existed inside the RangerPolarisAuthorizerTest unit-test fixture — operators had no artifact to actually register with Ranger Admin.

  • Extract the serviceDef into extensions/auth/ranger/src/main/resources/polaris-ranger-servicedef.json, the artifact operators register with Ranger Admin (documented in the updated README).
  • Generate the unit-test and intTest dev_polaris.json authz fixtures from this shipped artifact at build time (generateAuthzTestFixture/generateAuthzItTestFixture Gradle tasks), rather than checking in a byte-for-byte copy, so the shipped artifact and the serviceDef the 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-read delta.

Test plan

  • ./gradlew :polaris-extensions-auth-ranger:test passes (11/11)
  • Generated fixtures verified byte-for-byte JSON-equivalent to the fixtures they replace

🤖 Generated with Claude Code

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.

@dimas-b dimas-b left a comment

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.

Thanks for extracting this change into a dedicated PR, @obelix74 !

Just one minor comment below.

Comment thread extensions/auth/ranger/build.gradle.kts Outdated
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)

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.

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.

"org.apache.ranger.admin.client.EmbeddedResourcePolicySource");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment thread build.gradle.kts Outdated

// 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")

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.

Since we process these files in our code, we could add the header to the source and ignore it in runtime... WDYT?

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.

... but JSON supports comments, does it not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Anand Kumar Sankaran added 2 commits September 11, 2026 08:29
…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.

@dimas-b dimas-b left a comment

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.

Thanks, @obelix74 !

@github-project-automation github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Sep 11, 2026
@dimas-b
dimas-b requested review from flyingImer and snazy September 11, 2026 16:19

@vigneshio vigneshio left a comment

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.

LGTM.. Thks @obelix74

@dimas-b

dimas-b commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

FYI: @sneethiraj @mneethiraj

@dimas-b

dimas-b commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@obelix74 : The commits here have Claude as a formal co-author. Would you mind converting them to personal commits?

https://polaris.apache.org/community/contributing-guidelines/#guidelines-for-ai-assisted-contributions

@obelix74
obelix74 force-pushed the ranger-servicedef-base branch from 7774562 to 0b83034 Compare September 16, 2026 14:49

@flyingImer flyingImer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Reusing the shipped service definition in the authorization tests avoids maintaining separate copies, love it!

@mneethiraj mneethiraj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The updates look good. Thanks!

```

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants