Repository navigation
Conversation
|
@vyommani - instead of introducing a new table, |
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"); |
There was a problem hiding this comment.
"user", "group", "role" - these types don't seem to be relevant as constraint parameter. I suggest removing these.
There was a problem hiding this 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.
| } | ||
| } | ||
|
|
||
| updateServiceDefText(createdSvcDef, constraintDefs); |
There was a problem hiding this comment.
Instead of sending constraintDefs as an argument, consider the following:
createdSvcDef.setContraintDefs(serviceDef.getConstraintDefs());
updateServiceDefText(createdSvcDef);
There was a problem hiding this 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);
What changes were proposed in this pull request?
RANGER-5851: store
constraintDefs(added toRangerServiceDefin RANGER-5838) in Ranger Admin. Without this change, constraintDefs are dropped when a service def is created or updated.DB
x_constraint_def, following the existing service-def child tables (x_policy_condition_def,x_context_enricher_def).def_id(FK tox_service_def),item_id;resources,access_types,parametersandoptionsas JSON text;sort_order.def_id.079-add-x_constraint_def.sqlfor each.Admin
XXConstraintDefentity andXXConstraintDefDao, 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:itemId; constraintDefs left out of an update are removed;XPortalUserService: clears user references on user delete, as for other service-def tables.Validation (
RangerServiceDefValidator)resourcesandaccessTypesexist in the service def.typeis one ofstring, int, bool, enum, user, group, role.enumparameters need asubTypenaming an enum in the service def; other types must not setsubType.Not in this change
ServiceDBStore).How was this patch tested?
TestRangerServiceDefValidator: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.