Skip to content

RANGER-5851: Admin: persist constraintDefs of service definitions - #1327

Open
vyommani wants to merge 3 commits into
apache:masterfrom
vyommani:RANGER-5851
Open

vyommani wants to merge 3 commits into
apache:masterfrom
vyommani:RANGER-5851

Conversation

@vyommani

Copy link
Copy Markdown
Contributor

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.

@mneethiraj

Copy link
Copy Markdown
Contributor

@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.

@vyommani

Copy link
Copy Markdown
Contributor Author

@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.

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.


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

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.

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

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.

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.

}
}

updateServiceDefText(createdSvcDef, constraintDefs);

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.

Instead of sending constraintDefs as an argument, consider the following:

createdSvcDef.setContraintDefs(serviceDef.getConstraintDefs());
updateServiceDefText(createdSvcDef);

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.

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 branch has not been deployed

No deployments
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.

2 participants