You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by "vyommani (via GitHub)" <gi...@apache.org> on 2026/10/22 08:19:01 UTC

[PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

vyommani opened a new pull request, #1327:
URL: https://github.com/apache/ranger/pull/1327

   ## What changes were proposed in this pull request?
   
   RANGER-5851: store `constraintDefs` (added to `RangerServiceDef` in RANGER-5838) in Ranger Admin. Without this change, constraintDefs are dropped when a service def is created or updated.
   
   **DB**
   - New table `x_constraint_def`, following the existing service-def child tables (`x_policy_condition_def`, `x_context_enricher_def`).
   - Columns:
     - `def_id` (FK to `x_service_def`), `item_id`;
     - name, label, description and rb keys;
     - `resources`, `access_types`, `parameters` and `options` as JSON text;
     - `sort_order`.
   - Index on `def_id`.
   - Added to the current schema for MySQL, Oracle, Postgres, SQL Server and SQL Anywhere, with upgrade patch `079-add-x_constraint_def.sql` for each.
   - The patches create the table and index only if they don't exist, so they are safe to re-run.
   
   **Admin**
   - `XXConstraintDef` entity and `XXConstraintDefDao`, with find-by-def-id and find-by-def-id-and-name queries.
   - `RangerServiceDefServiceBase`: maps constraintDefs between the model and the entity, and loads them when a service def is read.
   - `ServiceDBStore`:
     - service def create/update stores constraintDefs, matching them by `itemId`; constraintDefs left out of an update are removed;
     - service def delete removes its constraintDefs.
   - `XPortalUserService`: clears user references on user delete, as for other service-def tables.
   
   **Validation (`RangerServiceDefValidator`)**
   - Constraint def itemId and name are unique and not empty.
   - `resources` and `accessTypes` exist in the service def.
   - Each listed resource's ancestors are also listed, so the constraint maps to complete resource hierarchies.
   - Parameter names are unique, and parameter `type` is one of `string, int, bool, enum, user, group, role`.
   - `enum` parameters need a `subType` naming an enum in the service def; other types must not set `subType`.
   - New error codes 2026–2032.
   
   **Not in this change**
   - Constraint policies (policyType 4) in Admin, including a policy-ref table that blocks removing a constraintDef while policies use it: RANGER-5852 (TODO in `ServiceDBStore`).
   - UI.
   - constraintDefs in shipped service-def JSONs.
   
   ## How was this patch tested?
   
   - `TestRangerServiceDefValidator`:
     - valid constraintDefs (user/group/role/enum parameters, empty resources);
     - every failure case;
     - `isValid()` rejecting a service def with an invalid constraintDef.
   - `TestServiceDBStore`: create stores constraintDefs; update adds, changes and removes them; delete removes them.
   - `TestRangerServiceDefServiceBase` / `TestRangerServiceDefService`: model ↔ entity round-trip, null lists stored as NULL, and constraintDefs loaded when a service def is read.


-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #1327:
URL: https://github.com/apache/ranger/pull/1327#discussion_r4356981168


##########
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java:
##########
@@ -698,6 +700,8 @@ public RangerServiceDef createServiceDef(RangerServiceDef serviceDef) throws Exc
             }
         }
 
+        updateServiceDefText(createdSvcDef, constraintDefs);

Review Comment:
   constraintDefs are stored only in def_text, so XXServiceDef has no field for them. buildDefText rebuilds the JSON from the row columns and child rows, then adds this list. Create and update take the list from the request before the row is reloaded (in update, serviceDefService.update() returns an object read from the old def_text). 
   
   If you prefer that shape, I can add a transient list on XXServiceDef and drop the argument
   createdSvcDef.setConstraintDefs(constraintDefs);
   updateServiceDefText(createdSvcDef);
   



-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on PR #1327:
URL: https://github.com/apache/ranger/pull/1327#issuecomment-6349071843

   @vyommani  - instead of introducing a new table, `x_constraint_def`, how about persisting entire service-def json in a new column `x_service_def.def_text` - similar to use of `x_policy.policy_text` and `x_service_resource.service_resource_elements_text`? This will make loading and persisting of service-def more efficient, similar to the improvement for policies in RANGER-2203.


-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #1327:
URL: https://github.com/apache/ranger/pull/1327#discussion_r4360343451


##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java:
##########
@@ -50,7 +52,8 @@
 public class RangerServiceDefValidator extends RangerValidator {
     private static final Logger LOG = LoggerFactory.getLogger(RangerServiceDefValidator.class);
 
-    private static final Set<String> VALID_CONFIG_TYPES = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONFIG_TYPES           = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONSTRAINT_PARAM_TYPES = ImmutableSet.of("string", "int", "bool", "enum", "user", "group", "role");

Review Comment:
   Is support for user/group/role a requirement? If yes, does UI and backend validate the value? I suggest keeping it simple to start with.



-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #1327:
URL: https://github.com/apache/ranger/pull/1327#discussion_r4356658239


##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java:
##########
@@ -50,7 +52,8 @@
 public class RangerServiceDefValidator extends RangerValidator {
     private static final Logger LOG = LoggerFactory.getLogger(RangerServiceDefValidator.class);
 
-    private static final Set<String> VALID_CONFIG_TYPES = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONFIG_TYPES           = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONSTRAINT_PARAM_TYPES = ImmutableSet.of("string", "int", "bool", "enum", "user", "group", "role");

Review Comment:
   "user", "group", "role" - these types don't seem to be relevant as constraint parameter. I suggest removing these.



##########
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java:
##########
@@ -698,6 +700,8 @@ public RangerServiceDef createServiceDef(RangerServiceDef serviceDef) throws Exc
             }
         }
 
+        updateServiceDefText(createdSvcDef, constraintDefs);

Review Comment:
   Instead of sending `constraintDefs` as an argument, consider the following:
   
   ```
   createdSvcDef.setContraintDefs(serviceDef.getConstraintDefs());
   updateServiceDefText(createdSvcDef);
   ```



-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #1327:
URL: https://github.com/apache/ranger/pull/1327#discussion_r4365972882


##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java:
##########
@@ -50,7 +52,8 @@
 public class RangerServiceDefValidator extends RangerValidator {
     private static final Logger LOG = LoggerFactory.getLogger(RangerServiceDefValidator.class);
 
-    private static final Set<String> VALID_CONFIG_TYPES = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONFIG_TYPES           = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONSTRAINT_PARAM_TYPES = ImmutableSet.of("string", "int", "bool", "enum", "user", "group", "role");

Review Comment:
   It is not a requirement for this change. The value is stored as a string, and neither the UI nor Admin checks that it is an existing user, group, or role. I removed user, group, and role. The parameter types are string, int, bool, and enum.



-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on PR #1327:
URL: https://github.com/apache/ranger/pull/1327#issuecomment-6357551923

   > @vyommani - instead of introducing a new table, `x_constraint_def`, how about persisting entire service-def json in a new column `x_service_def.def_text` - similar to use of `x_policy.policy_text` and `x_service_resource.service_resource_elements_text`? This will make loading and persisting of service-def more efficient, similar to the improvement for policies in [RANGER-2203](https://issues.apache.org/jira/browse/RANGER-2203).
   
   Madhan, that works, and I’ll update this PR to match.
   
   I’ll drop x_constraint_def and store the service def in a new x_service_def.def_text column, the same way x_policy.policy_text works. Create and update will write the full service-def JSON there. Get will read that column when it’s set, and use the current child-table load when it’s null. A Java patch will fill def_text for existing service defs.
   
   The current child tables stay, because policy and zone ref updates look up resource, access-type and condition rows by id. constraintDefs will live only in def_text, so the ref for constraint policies in RANGER-5852 will use the constraint name.


-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #1327:
URL: https://github.com/apache/ranger/pull/1327#discussion_r4356961324


##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java:
##########
@@ -50,7 +52,8 @@
 public class RangerServiceDefValidator extends RangerValidator {
     private static final Logger LOG = LoggerFactory.getLogger(RangerServiceDefValidator.class);
 
-    private static final Set<String> VALID_CONFIG_TYPES = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONFIG_TYPES           = ImmutableSet.of("bool", "enum", "int", "string", "password", "path");
+    private static final Set<String> VALID_CONSTRAINT_PARAM_TYPES = ImmutableSet.of("string", "int", "bool", "enum", "user", "group", "role");

Review Comment:
   user, group, and role are for an approver-style parameter. The value stored on the policy is still a string, a user, group, or role name. The type tells the UI which picker to show. They are not service-config types: password and path stay rejected, and isValidConfigType rejects user, group, and role. requireApproval uses an approver of type user or group. I can drop them if a plain string is enough.



-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5851: Admin: persist constraintDefs of service definitions [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #1327:
URL: https://github.com/apache/ranger/pull/1327#discussion_r4360320325


##########
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java:
##########
@@ -698,6 +700,8 @@ public RangerServiceDef createServiceDef(RangerServiceDef serviceDef) throws Exc
             }
         }
 
+        updateServiceDefText(createdSvcDef, constraintDefs);

Review Comment:
   Ok. I meant to say `createdServiceDef` (which is initialized in the following line), instead of `createdSvcDef`.
   
   Anyway, the goal is to follow the same approach used to save/update RangerPolicy - in ServiceDBStore.createPolicy() and RangerPolicyServiceBase.mapViewToEntityBean(). Note that RangerPolicyServiceBase.mapEntityToViewBean() must also be updated to read service-def from XXServiceDef.defText field, if its not empty.



-- 
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: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org