roryqi commented on code in PR #13478:
URL: https://github.com/apache/gravitino/pull/13478#discussion_r4248172329
##########
design-docs/tag-based-read-restrictions-for-iceberg-rest.md:
##########
@@ -397,16 +404,18 @@ is lowered to:
```text
(condition1 and result1)
-or (not condition1 and condition2 and result2)
-or (not condition1 and not condition2 and fallback)
+or (not condition1 and (
+ (condition2 and result2)
+ or (not condition2 and fallback)
+))
```
Request-context-only conditions are folded before the final predicate is
built, but every branch
must still parse and type-check. For a column mask, all conditions are
context-only: the resolver
evaluates them in order and selects the first action whose condition is true,
or the final fallback
action. The conditional assignment, `if`, and `else` nodes never appear in the
Iceberg wire
expression or projection. This preserves the first-matching-branch semantics
of Rego's `else`
-chain while producing the one closed Boolean predicate or one column action
required by Iceberg.
+The nested form grows linearly rather than repeating every earlier condition
in every later branch.
Review Comment:
Fixed in 94563dacbc. I restored the missing continuation about producing one
closed Boolean predicate or column action, before the linear-lowering sentence.
##########
docs/policies.md:
##########
@@ -47,6 +47,120 @@ The row-filter and column-mask policy types can be created
and associated with t
does not enforce them yet. Until enforcement is available, these policy types
do not restrict data
access.
+### Read-Restriction Expressions
+
+Row-filter and column-mask expressions use `restricted-rego-v1`, a small,
fail-closed Rego-like
+syntax owned by Gravitino. Each expression is one complete rule, not an
arbitrary Rego module. A
+row filter starts with `filter :=` and produces a Boolean predicate. A column
mask starts with
+`mask := action("...")` and produces an allowlisted masking action.
+
+The complete grammar is:
+
+```text
+program := filterRule | maskRule
+filterRule := unconditionalFilter | conditionalFilter
+unconditionalFilter := "filter" ":=" expr
+conditionalFilter := "filter" ":=" expr "if" expr filterElse* filterFallback
+filterElse := "else" ":=" expr "if" expr
+filterFallback := "else" ":=" expr
+maskRule := unconditionalMask | conditionalMask
+unconditionalMask := "mask" ":=" maskAction
+conditionalMask := "mask" ":=" maskAction "if" contextExpr maskElse*
maskFallback
+maskElse := "else" ":=" maskAction "if" contextExpr
+maskFallback := "else" ":=" maskAction
+maskAction := "action" "(" string ")"
+contextExpr := expr
+expr := orExpr
+orExpr := andExpr ("or" andExpr)*
+andExpr := notExpr ("and" notExpr)*
+notExpr := "not" notExpr | compareExpr
+compareExpr := primary (("==" | "!=" | "<" | "<=" | ">" | ">=" | "in")
primary)?
+primary := colRef | sessionUser | groupMember | literal | array | "(" expr
")"
+colRef := "col" "(" string ")"
+sessionUser := "session_user" "(" ")"
+groupMember := "is_group_member" "(" string ")"
+literal := string | number | boolean | null
+array := "[" literal ("," literal)* "]"
+boolean := "true" | "false"
+null := "null"
+number := "-"? ("0" | nonZeroDigit digit*) ("." digit+)?
+digit := "0" | nonZeroDigit
+nonZeroDigit := "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9"
+```
+
+For example, a row-filter program can select the first matching predicate:
+
+```restricted-rego-v1
+filter := col("region") == "US" if is_group_member("auditors")
+else := col("owner") == session_user()
+```
+
+This column-mask program selects a masking action in the same way:
+
+```restricted-rego-v1
+mask := action("show-last-4") if is_group_member("support")
+else := action("replace-with-null")
+```
+
+Conditional branches are evaluated from left to right. The first true
condition selects its result,
+and every conditional rule must end with an unconditional `else`. Row-filter
conditions and results
+must be Boolean. Column-mask conditions may use only request context; they
cannot reference columns.
+
+The syntax supports these constructs:
+
+- `col("name")`, `session_user()`, and `is_group_member("group")`;
+- JSON string literals, exact decimal numbers, `true`, `false`, `null`, and
homogeneous non-null
+ literal arrays;
+- comparisons `==`, `!=`, `<`, `<=`, `>`, `>=`, and `in`; and
+- Boolean operators `and`, `or`, and `not`.
+
+Keywords and built-in function names are case-sensitive. For example, only
lowercase `in` is an
+operator; `In`, `IN`, and `iN` are invalid.
+
+Strings use JSON double-quoted syntax and are decoded exactly once after the
enclosing HTTP JSON is
+decoded. Raw control characters, NUL, isolated Unicode surrogates,
single-quoted strings, and invalid
+JSON escapes are rejected. Gravitino does not normalize, trim, or case-fold
decoded column names,
+group names, or values. Numbers are exact decimals; exponent notation, a
leading `+`, leading zeroes,
+and a trailing decimal point are invalid.
+
+Supported operand shapes are:
+
+| Form | Operators | Requirements |
+| --- | --- | --- |
+| Boolean predicates | `and`, `or`, `not` | Every operand must be Boolean. |
+| Column and non-null literal | `==`, `!=`, `<`, `<=`, `>`, `>=` | Either
operand order; types must be compatible. Boolean literals support only `==` and
`!=`. |
+| String column and `session_user()` | `==`, `!=` | Either operand order. |
+| `session_user()` and string literal | `==`, `!=` | Either operand order. |
+| Column and `null` | `==`, `!=` | Either operand order. |
+| Column and literal array | `in` | Column on the left; array must be
non-empty and homogeneous. |
+| `session_user()` and string array | `in` | `session_user()` on the left;
array must be non-empty. |
+| `is_group_member("group")` | none | Produces a Boolean request-context
predicate. |
+
+Column-to-column and literal-to-literal comparisons, ordering on
`session_user()`, null array
+elements, nested arrays, and comparisons on `is_group_member(...)` are invalid.
+
+Column-mask actions are `mask-alphanum`, `mask-to-fixed-value`,
`replace-with-null`, `show-first-4`,
+`show-last-4`, `truncate-to-year`, `truncate-to-month`, `sha-256-global`, and
+`sha-256-query-local`.
+
+Validation applies these limits when a policy is created or its content is
updated:
+
+- 16 KiB of UTF-8 source;
+- operation depth 8 for the source AST;
+- 256 nodes for the source AST;
+- 4 KiB of UTF-8 for each decoded string literal; and
+- 256 bytes for each numeric literal; and
+- 256 array elements in total in the source.
Review Comment:
Fixed in 94563dacbc. I documented both the per-array 256-element limit and
the 256-level parenthesis nesting limit in the user and design docs, and
cleaned up the list punctuation.
##########
core/src/main/java/org/apache/gravitino/policy/rego/RestrictedRegoProgram.java:
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://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.apache.gravitino.policy.rego;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+/** A parsed, unresolved program in the {@code restricted-rego-v1} source
dialect. */
+public interface RestrictedRegoProgram {
+
+ /**
+ * Returns the rule type declared by this program.
+ *
+ * @return rule type
+ */
+ RuleType ruleType();
+
+ /**
+ * Validates the rule and every expression it contains.
+ *
+ * @throws IllegalArgumentException if the rule is malformed
+ */
+ void validate() throws IllegalArgumentException;
+
+ /** Rule heads supported by {@code restricted-rego-v1}. */
+ enum RuleType {
+ /** A row-filter rule. */
+ FILTER,
+ /** A column-mask rule. */
+ MASK
+ }
+
+ /** Logical column-mask actions supported by the first restricted profile. */
+ enum MaskAction {
+ /** Mask alphanumeric characters. */
+ MASK_ALPHANUM("mask-alphanum"),
+ /** Replace the value with the action's fixed value. */
+ MASK_TO_FIXED_VALUE("mask-to-fixed-value"),
+ /** Replace the value with null. */
+ REPLACE_WITH_NULL("replace-with-null"),
+ /** Preserve only the first four characters. */
+ SHOW_FIRST_4("show-first-4"),
+ /** Preserve only the last four characters. */
+ SHOW_LAST_4("show-last-4"),
+ /** Truncate a temporal value to its year. */
+ TRUNCATE_TO_YEAR("truncate-to-year"),
+ /** Truncate a temporal value to its month. */
+ TRUNCATE_TO_MONTH("truncate-to-month"),
+ /** Apply a global SHA-256 mask. */
+ SHA_256_GLOBAL("sha-256-global"),
+ /** Apply a query-local SHA-256 mask. */
+ SHA_256_QUERY_LOCAL("sha-256-query-local");
+
+ private final String value;
+
+ MaskAction(String value) {
+ this.value = value;
+ }
+
+ /**
+ * Parses a mask-action value.
+ *
+ * @param value source action value
+ * @return parsed action
+ * @throws IllegalArgumentException if the action is not allowlisted
+ */
+ public static MaskAction fromValue(String value) {
+ Preconditions.checkArgument(value != null && !value.isEmpty(), "mask
action cannot be empty");
+ for (MaskAction action : values()) {
+ if (action.value.equals(value)) {
+ return action;
+ }
+ }
+ throw new IllegalArgumentException("Unsupported restricted-rego-v1 mask
action: " + value);
+ }
+
+ /**
+ * Returns the source action value.
+ *
+ * @return source action value
+ */
+ public String value() {
+ return value;
+ }
+ }
+
+ /** One guarded result in a conditional row-filter rule. */
+ final class FilterBranch {
+ private final CanonicalExpression result;
+ private final CanonicalExpression condition;
+
+ FilterBranch(CanonicalExpression result, CanonicalExpression condition) {
+ this.result = result;
+ this.condition = condition;
+ }
+
+ /**
+ * Returns the Boolean result selected by this branch.
+ *
+ * @return unresolved Boolean result
+ */
+ public CanonicalExpression result() {
+ return result;
+ }
+
+ /**
+ * Returns this branch's Boolean condition.
+ *
+ * @return unresolved Boolean condition
+ */
+ public CanonicalExpression condition() {
+ return condition;
+ }
+
+ /**
+ * Validates the branch.
+ *
+ * @throws IllegalArgumentException if its result or condition is not
Boolean
+ */
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(result != null, "filter branch result cannot
be null");
+ Preconditions.checkArgument(condition != null, "filter branch condition
cannot be null");
+ result.validate();
+ condition.validate();
+ Preconditions.checkArgument(
+ ExpressionValidator.isPredicate(result), "filter branch result must
be Boolean");
+ Preconditions.checkArgument(
+ ExpressionValidator.isPredicate(condition), "filter branch condition
must be Boolean");
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof FilterBranch)) {
+ return false;
+ }
+ FilterBranch that = (FilterBranch) other;
+ return Objects.equals(result, that.result) && Objects.equals(condition,
that.condition);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(result, condition);
+ }
+
+ @Override
+ public String toString() {
+ return "FilterBranch{" + "result=" + result + ", condition=" + condition
+ '}';
+ }
+ }
+
+ /** A complete {@code filter := ...} row-filter rule. */
+ final class RowFilter implements RestrictedRegoProgram {
+ private final List<FilterBranch> branches;
+ private final CanonicalExpression fallback;
+
+ RowFilter(List<FilterBranch> branches, CanonicalExpression fallback) {
+ this.branches =
+ branches == null ? null : Collections.unmodifiableList(new
ArrayList<>(branches));
+ this.fallback = fallback;
+ }
+
+ @Override
+ public RuleType ruleType() {
+ return RuleType.FILTER;
+ }
+
+ /**
+ * Returns the guarded results in source order.
+ *
+ * @return immutable branches, or an empty list for an unconditional rule
+ */
+ public List<FilterBranch> branches() {
+ return branches;
+ }
+
+ /**
+ * Returns the unconditional result or final {@code else} result.
+ *
+ * @return unresolved Boolean fallback
+ */
+ public CanonicalExpression fallback() {
+ return fallback;
+ }
+
+ /**
+ * Returns whether this rule contains at least one conditional branch.
+ *
+ * @return {@code true} for a conditional rule
+ */
+ public boolean isConditional() {
+ return branches != null && !branches.isEmpty();
+ }
+
+ /**
+ * Lowers the ordered branches to one Boolean predicate with first-match
semantics.
+ *
+ * <p>The returned predicate remains unresolved. Context and column
binding can be applied to it
+ * by a later resolver.
+ *
+ * @return lowered unresolved predicate
+ */
+ public CanonicalExpression lower() {
+ validate();
+ if (branches.isEmpty()) {
+ return fallback;
+ }
+
+ CanonicalExpression lowered = fallback;
+ for (int index = branches.size() - 1; index >= 0; index--) {
+ FilterBranch branch = branches.get(index);
+ CanonicalExpression selected =
+ LogicalExpression.of(LogicalOperator.AND,
List.of(branch.condition(), branch.result()));
+ CanonicalExpression remaining =
+ LogicalExpression.of(
+ LogicalOperator.AND, List.of(Not.negate(branch.condition()),
lowered));
+ lowered = LogicalExpression.of(LogicalOperator.OR, List.of(selected,
remaining));
Review Comment:
Fixed in 94563dacbc with the boundary-test option, without adding a new
post-lowering rejection. The test pins the largest minimal branch chain allowed
by the source-node budget: 127 branches lower to depth 256 and 890 node
occurrences. The same derived bound is now documented for downstream
resolver/serializer implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
