feat: Batch OPA column mask calls #827
Conversation
…ator to use new batched endpoint
| Some("columnMask"), | ||
| OpaApiVersion::V1, | ||
|
|
||
| let batched_column_masking_connection_string = if trino.column_masking_enabled() { |
There was a problem hiding this comment.
| let batched_column_masking_connection_string = if trino.column_masking_enabled() { | |
| let batched_column_masking_connection_string = if opa_config.enable_column_masking { |
... and delete the function colum_masking_enabled().
| /// URI for fetching columns masks in batches, e.g. | ||
| /// `http://localhost:8081/v1/data/trino/batchColumnMasks` - if not set, | ||
| /// column-masking-uri will be used for getting column masks in parallel |
There was a problem hiding this comment.
column-masking-uri is not set, so it will not be used.
| opa: | ||
| configMapName: opa | ||
| package: trino | ||
| {% if test_scenario['values']['trino'] == "451" %} |
There was a problem hiding this comment.
The supported Trino versions in SDP 26.3 are 477 and 479. I would just remove version 451 now from test-definition.yaml. Otherwise, removing these checks could be forgotten.
| column_masks := trino.batchColumnMasks with input as request | ||
| with data.trino_policies.policies as policies | ||
|
|
||
| count(column_masks) == 0 |
There was a problem hiding this comment.
| count(column_masks) == 0 | |
| column_masks == set() |
column_masks must be an empty set and not an empty array, object or string.
| #[serde(flatten)] | ||
| pub opa: OpaConfig, | ||
|
|
||
| /// Set the OPA batched column masking uri for Trino queries or not. Defaults to true. |
There was a problem hiding this comment.
I am not sure if this is better:
| /// Set the OPA batched column masking uri for Trino queries or not. Defaults to true. | |
| /// Whether to set the OPA batched column masking URI for Trino queries; defaults to true |
| enableColumnMasking: true # default | ||
| ---- | ||
|
|
||
| <1> Redundant fields for column masking reference configuration are omitted |
There was a problem hiding this comment.
Which fields do you mean? All other TrinoCluster fields? If yes, then this comment is not necessary because almost all code snippets in this documentation are incomplete or we would have to add such a comment to all other snippets for consistency.
|
|
||
| ===== Result | ||
|
|
||
| In `access-control.properties` the following value is set, when `enableColumnMasking` is set to `true`: |
There was a problem hiding this comment.
| In `access-control.properties` the following value is set, when `enableColumnMasking` is set to `true`: | |
| In the `access-control.properties` file, the following value is set when `enableColumnMasking` is set to `true`: |
(or just fix the position of the comma)
| ===== Considerations | ||
|
|
||
| The default setting for `enableColumnMasking` assumes a `batchColumnMasks` rule is defined in the Rego rules for the TrinoCluster. | ||
| If there is no such rule defined, Trino queries, using that column masking endpoint, will fail. |
There was a problem hiding this comment.
| If there is no such rule defined, Trino queries, using that column masking endpoint, will fail. | |
| If no such rule is defined, Trino queries that utilize the column masking endpoint will fail. |
(or just fix the commas)
| assert!(access_control_config.contains("opa.allow-permission-management-operations=false")); | ||
| assert!(access_control_config.contains(r#"opa.policy.batched-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batch-new"#)); | ||
| assert!(access_control_config.contains(r#"opa.policy.column-masking-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/columnMask"#)); | ||
| assert!(access_control_config.contains(r#"opa.policy.batch-column-masking-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batchColumnMasks"#)); |
There was a problem hiding this comment.
This test case tests configOverrides. Perhaps you could override the batch-column-masking-uri?
| * `access-control.properties` | ||
| * `config.properties` | ||
| * `node.properties` | ||
| * `password-authenticator.properties` |
There was a problem hiding this comment.
Does the file password-authenticator.properties exist and can it really be overridden?
Description
Implementation for #814
This PR changes the previous behavior of setting
opa.policy.column-masking-urito now settingopa.policy.batch-column-masking-uriinaccess-control.propertiesinstead, for better performance. Consequently the Trino rules are expanded with thebatchColumnMasksrules and schema.It also adds the new field
enableColumnMaskingto theopaconfiguration, to make it possible for the user to disable the default behavior of settingopa.policy.batch-column-masking-uriat all and adds accompanying reference documentation.Additionally, as noted in https://github.com/stackabletech/decisions/issues/71#issuecomment-3718891949,
opachanged to a mandatory enum variant instead of being optional.Integrationtests
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker