Skip to content

Commit a6e644a

Browse files
xeniapexeniape
authored
feat: Add access-control.properties overrides (#721)
* feat: Add access-control.properties overrides * only create access-control.properties entry in cm when data present * add changelog * adjust comments
1 parent bc6af37 commit a6e644a

File tree

5 files changed

+152
-66
lines changed

5 files changed

+152
-66
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file.
1212
- Support configuring JVM arguments ([#677]).
1313
- Aggregate emitted Kubernetes events on the CustomResources ([#677]).
1414
- Support for Trino 470 ([#705]).
15+
- Support `access-control.properties` in configOverrides ([#721]).
1516

1617
### Changed
1718

@@ -39,6 +40,7 @@ All notable changes to this project will be documented in this file.
3940
[#705]: https://github.com/stackabletech/trino-operator/pull/705
4041
[#715]: https://github.com/stackabletech/trino-operator/pull/715
4142
[#717]: https://github.com/stackabletech/trino-operator/pull/717
43+
[#721]: https://github.com/stackabletech/trino-operator/pull/721
4244

4345
## [24.11.1] - 2025-01-10
4446

‎docs/modules/trino/pages/usage-guide/configuration.adoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ This will lead to faulty installations.
1010

1111
For a role or role group, at the same level of `config`, you can specify `configOverrides` for:
1212

13+
* `access-control.properties`
1314
* `config.properties`
1415
* `node.properties`
15-
* `log.properties`
1616
* `password-authenticator.properties`
1717
* `security.properties`
1818

‎rust/operator-binary/src/authorization/opa.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,24 @@ use crate::crd::v1alpha1::TrinoCluster;
1010
pub struct TrinoOpaConfig {
1111
/// URI for OPA policies, e.g.
1212
/// `http://localhost:8081/v1/data/trino/allow`
13-
non_batched_connection_string: String,
13+
pub(crate) non_batched_connection_string: String,
1414
/// URI for Batch OPA policies, e.g.
1515
/// `http://localhost:8081/v1/data/trino/batch` - if not set, a
1616
/// single request will be sent for each entry on filtering methods
17-
batched_connection_string: String,
17+
pub(crate) batched_connection_string: String,
1818
/// URI for fetching row filters, e.g.
1919
/// `http://localhost:8081/v1/data/trino/rowFilters` - if not set,
2020
/// no row filtering will be applied
21-
row_filters_connection_string: Option<String>,
21+
pub(crate) row_filters_connection_string: Option<String>,
2222
/// URI for fetching column masks, e.g.
2323
/// `http://localhost:8081/v1/data/trino/columnMask` - if not set,
2424
/// no masking will be applied
25-
column_masking_connection_string: Option<String>,
25+
pub(crate) column_masking_connection_string: Option<String>,
2626
/// Whether to allow permission management (GRANT, DENY, ...) and
2727
/// role management operations - OPA will not be queried for any
2828
/// such operations, they will be bulk allowed or denied depending
2929
/// on this setting
30-
allow_permission_management_operations: bool,
30+
pub(crate) allow_permission_management_operations: bool,
3131
}
3232

3333
impl TrinoOpaConfig {

‎rust/operator-binary/src/controller.rs

Lines changed: 95 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::{
33
collections::{BTreeMap, HashMap},
44
convert::Infallible,
55
num::ParseIntError,
6-
ops::Div,
76
str::FromStr,
87
sync::Arc,
98
};
@@ -86,10 +85,9 @@ use crate::{
8685
v1alpha1, Container, TrinoRole, ACCESS_CONTROL_PROPERTIES, APP_NAME, CONFIG_DIR_NAME,
8786
CONFIG_PROPERTIES, DATA_DIR_NAME, DISCOVERY_URI, ENV_INTERNAL_SECRET, HTTPS_PORT,
8887
HTTPS_PORT_NAME, HTTP_PORT, HTTP_PORT_NAME, JVM_CONFIG, JVM_SECURITY_PROPERTIES,
89-
LOG_COMPRESSION, LOG_FORMAT, LOG_MAX_SIZE, LOG_MAX_TOTAL_SIZE, LOG_PATH, LOG_PROPERTIES,
90-
METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES, RW_CONFIG_DIR_NAME,
91-
STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
92-
STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR,
88+
LOG_PROPERTIES, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES,
89+
RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR,
90+
STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR,
9391
},
9492
operations::{
9593
add_graceful_shutdown_config, graceful_shutdown_config_properties, pdb::add_pdbs,
@@ -110,11 +108,6 @@ pub const TRINO_UID: i64 = 1000;
110108
pub const STACKABLE_LOG_DIR: &str = "/stackable/log";
111109
pub const STACKABLE_LOG_CONFIG_DIR: &str = "/stackable/log_config";
112110

113-
const LOG_FILE_COUNT: u32 = 2;
114-
pub const MAX_TRINO_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity {
115-
value: 10.0,
116-
unit: BinaryMultiple::Mebi,
117-
};
118111
pub const MAX_PREPARE_LOG_FILE_SIZE: MemoryQuantity = MemoryQuantity {
119112
value: 1.0,
120113
unit: BinaryMultiple::Mebi,
@@ -663,7 +656,7 @@ fn build_rolegroup_config_map(
663656
.next()
664657
.context(MissingCoordinatorPodsSnafu)?;
665658

666-
// Add additional config files fore authentication
659+
// Add additional config files for authentication
667660
cm_conf_data.extend(trino_authentication_config.config_files(trino_role));
668661

669662
for (property_name_kind, config) in config {
@@ -703,45 +696,6 @@ fn build_rolegroup_config_map(
703696
dynamic_resolved_config
704697
.extend(graceful_shutdown_config_properties(trino, trino_role));
705698

706-
// The log format used by Trino
707-
dynamic_resolved_config.insert(LOG_FORMAT.to_string(), Some("json".to_string()));
708-
// The path to the log file used by Trino
709-
dynamic_resolved_config.insert(
710-
LOG_PATH.to_string(),
711-
Some(format!(
712-
"{STACKABLE_LOG_DIR}/{container}/server.airlift.json",
713-
container = Container::Trino
714-
)),
715-
);
716-
// We do not compress. This will result in LOG_MAX_TOTAL_SIZE / LOG_MAX_SIZE files.
717-
dynamic_resolved_config
718-
.insert(LOG_COMPRESSION.to_string(), Some("none".to_string()));
719-
// The size of one log file
720-
dynamic_resolved_config.insert(
721-
LOG_MAX_SIZE.to_string(),
722-
Some(format!(
723-
// Trino uses the unit "MB" for MiB.
724-
"{}MB",
725-
MAX_TRINO_LOG_FILES_SIZE
726-
.scale_to(BinaryMultiple::Mebi)
727-
.div(LOG_FILE_COUNT as f32)
728-
.ceil()
729-
.value,
730-
)),
731-
);
732-
// The maximum size of all logfiles combined
733-
dynamic_resolved_config.insert(
734-
LOG_MAX_TOTAL_SIZE.to_string(),
735-
Some(format!(
736-
// Trino uses the unit "MB" for MiB.
737-
"{}MB",
738-
MAX_TRINO_LOG_FILES_SIZE
739-
.scale_to(BinaryMultiple::Mebi)
740-
.ceil()
741-
.value,
742-
)),
743-
);
744-
745699
// Add static properties and overrides
746700
dynamic_resolved_config.extend(transformed_config);
747701

@@ -784,19 +738,29 @@ fn build_rolegroup_config_map(
784738
);
785739
}
786740
}
741+
PropertyNameKind::File(file_name) if file_name == ACCESS_CONTROL_PROPERTIES => {
742+
if let Some(trino_opa_config) = trino_opa_config {
743+
dynamic_resolved_config.extend(trino_opa_config.as_config());
744+
}
745+
746+
// Add static properties and overrides
747+
dynamic_resolved_config.extend(transformed_config);
748+
749+
if !dynamic_resolved_config.is_empty() {
750+
let access_control_properties =
751+
product_config::writer::to_java_properties_string(
752+
dynamic_resolved_config.iter(),
753+
)
754+
.context(FailedToWriteJavaPropertiesSnafu)?;
755+
756+
cm_conf_data.insert(file_name.to_string(), access_control_properties);
757+
}
758+
}
787759
PropertyNameKind::File(file_name) if file_name == JVM_CONFIG => {}
788760
_ => {}
789761
}
790762
}
791763

792-
if let Some(trino_opa_config) = trino_opa_config {
793-
let config = trino_opa_config.as_config();
794-
let config_properties = product_config::writer::to_java_properties_string(config.iter())
795-
.context(FailedToWriteJavaPropertiesSnafu)?;
796-
797-
cm_conf_data.insert(ACCESS_CONTROL_PROPERTIES.to_string(), config_properties);
798-
}
799-
800764
cm_conf_data.insert(JVM_CONFIG.to_string(), jvm_config.to_string());
801765

802766
let jvm_sec_props: BTreeMap<String, Option<String>> = config
@@ -1333,6 +1297,7 @@ fn validated_product_config(
13331297
PropertyNameKind::File(JVM_CONFIG.to_string()),
13341298
PropertyNameKind::File(LOG_PROPERTIES.to_string()),
13351299
PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()),
1300+
PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()),
13361301
];
13371302

13381303
roles.insert(
@@ -1740,6 +1705,7 @@ mod tests {
17401705
assert!(cm.contains_key("security.properties"));
17411706
assert!(cm.contains_key("node.properties"));
17421707
assert!(cm.contains_key("log.properties"));
1708+
assert!(cm.contains_key("access-control.properties"));
17431709
}
17441710

17451711
fn build_config_map(trino_yaml: &str) -> ConfigMap {
@@ -1761,6 +1727,7 @@ mod tests {
17611727
PropertyNameKind::File(JVM_CONFIG.to_string()),
17621728
PropertyNameKind::File(LOG_PROPERTIES.to_string()),
17631729
PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()),
1730+
PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()),
17641731
];
17651732
let validated_config = validate_all_roles_and_groups_config(
17661733
// The Trino version is a single number like 396.
@@ -1807,6 +1774,23 @@ mod tests {
18071774
TrinoAuthenticationTypes::try_from(Vec::new()).unwrap(),
18081775
)
18091776
.unwrap();
1777+
let trino_opa_config = Some(TrinoOpaConfig {
1778+
non_batched_connection_string:
1779+
"http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/allow"
1780+
.to_string(),
1781+
batched_connection_string:
1782+
"http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batch"
1783+
.to_string(),
1784+
row_filters_connection_string: Some(
1785+
"http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/rowFilters"
1786+
.to_string(),
1787+
),
1788+
column_masking_connection_string: Some(
1789+
"http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/columnMask"
1790+
.to_string(),
1791+
),
1792+
allow_permission_management_operations: true,
1793+
});
18101794
let merged_config = trino
18111795
.merged_config(&trino_role, &rolegroup_ref, &[])
18121796
.unwrap();
@@ -1824,13 +1808,65 @@ mod tests {
18241808
.unwrap(),
18251809
&merged_config,
18261810
&trino_authentication_config,
1827-
&None,
1811+
&trino_opa_config,
18281812
None,
18291813
&cluster_info,
18301814
)
18311815
.unwrap()
18321816
}
18331817

1818+
#[test]
1819+
fn test_access_control_overrides() {
1820+
let trino_yaml = r#"
1821+
apiVersion: trino.stackable.tech/v1alpha1
1822+
kind: TrinoCluster
1823+
metadata:
1824+
name: trino
1825+
spec:
1826+
image:
1827+
productVersion: "470"
1828+
clusterConfig:
1829+
catalogLabelSelector:
1830+
matchLabels:
1831+
trino: simple-trino
1832+
authorization:
1833+
opa:
1834+
configMapName: simple-opa
1835+
package: my-product
1836+
coordinators:
1837+
configOverrides:
1838+
access-control.properties:
1839+
hello-from-role: "true" # only defined here at role level
1840+
foo.bar: "false" # overriden by role group below
1841+
opa.allow-permission-management-operations: "false" # override value from config
1842+
roleGroups:
1843+
default:
1844+
configOverrides:
1845+
access-control.properties:
1846+
hello-from-role-group: "true" # only defined here at group level
1847+
foo.bar: "true" # overrides role value
1848+
opa.policy.batched-uri: "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batch-new" # override value from config
1849+
replicas: 1
1850+
workers:
1851+
roleGroups:
1852+
default:
1853+
replicas: 1
1854+
"#;
1855+
1856+
let cm = build_config_map(trino_yaml).data.unwrap();
1857+
let access_control_config = cm.get("access-control.properties").unwrap();
1858+
1859+
assert!(access_control_config.contains("access-control.name=opa"));
1860+
assert!(access_control_config.contains("hello-from-role=true"));
1861+
assert!(access_control_config.contains("hello-from-role-group=true"));
1862+
assert!(access_control_config.contains("foo.bar=true"));
1863+
assert!(access_control_config.contains("opa.allow-permission-management-operations=false"));
1864+
assert!(access_control_config.contains(r#"opa.policy.batched-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batch-new"#));
1865+
assert!(access_control_config.contains(r#"opa.policy.column-masking-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/columnMask"#));
1866+
assert!(access_control_config.contains(r#"opa.policy.row-filters-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/rowFilters"#));
1867+
assert!(access_control_config.contains(r#"opa.policy.uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/allow"#));
1868+
}
1869+
18341870
#[test]
18351871
fn test_env_overrides() {
18361872
let trino_yaml = r#"

‎rust/operator-binary/src/crd/mod.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ pub mod authentication;
33
pub mod catalog;
44
pub mod discovery;
55

6-
use std::{collections::BTreeMap, str::FromStr};
6+
use std::{collections::BTreeMap, ops::Div, str::FromStr};
77

88
use affinity::get_affinity;
99
use serde::{Deserialize, Serialize};
@@ -26,6 +26,7 @@ use stackable_operator::{
2626
},
2727
k8s_openapi::apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::LabelSelector},
2828
kube::{runtime::reflector::ObjectRef, CustomResource, ResourceExt},
29+
memory::{BinaryMultiple, MemoryQuantity},
2930
product_config_utils::{Configuration, Error as ConfigError},
3031
product_logging::{self, spec::Logging},
3132
role_utils::{GenericRoleConfig, JavaCommonConfig, Role, RoleGroup, RoleGroupRef},
@@ -91,6 +92,7 @@ pub const DATA_DIR_NAME: &str = "/stackable/data";
9192
pub const STACKABLE_SERVER_TLS_DIR: &str = "/stackable/server_tls";
9293
pub const STACKABLE_CLIENT_TLS_DIR: &str = "/stackable/client_tls";
9394
pub const STACKABLE_INTERNAL_TLS_DIR: &str = "/stackable/internal_tls";
95+
pub const STACKABLE_LOG_DIR: &str = "/stackable/log";
9496
pub const STACKABLE_MOUNT_SERVER_TLS_DIR: &str = "/stackable/mount_server_tls";
9597
pub const STACKABLE_MOUNT_INTERNAL_TLS_DIR: &str = "/stackable/mount_internal_tls";
9698
pub const SYSTEM_TRUST_STORE: &str = "/etc/pki/java/cacerts";
@@ -107,6 +109,11 @@ pub const LOG_PATH: &str = "log.path";
107109
pub const LOG_COMPRESSION: &str = "log.compression";
108110
pub const LOG_MAX_SIZE: &str = "log.max-size";
109111
pub const LOG_MAX_TOTAL_SIZE: &str = "log.max-total-size";
112+
const LOG_FILE_COUNT: u32 = 2;
113+
pub const MAX_TRINO_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity {
114+
value: 10.0,
115+
unit: BinaryMultiple::Mebi,
116+
};
110117

111118
pub const JVM_HEAP_FACTOR: f32 = 0.8;
112119

@@ -552,6 +559,46 @@ impl Configuration for v1alpha1::TrinoConfigFragment {
552559
);
553560
}
554561

562+
// The log format used by Trino
563+
result.insert(LOG_FORMAT.to_string(), Some("json".to_string()));
564+
// The path to the log file used by Trino
565+
result.insert(
566+
LOG_PATH.to_string(),
567+
Some(format!(
568+
"{STACKABLE_LOG_DIR}/{container}/server.airlift.json",
569+
container = Container::Trino
570+
)),
571+
);
572+
573+
// We do not compress. This will result in LOG_MAX_TOTAL_SIZE / LOG_MAX_SIZE files.
574+
result.insert(LOG_COMPRESSION.to_string(), Some("none".to_string()));
575+
576+
// The size of one log file
577+
result.insert(
578+
LOG_MAX_SIZE.to_string(),
579+
Some(format!(
580+
// Trino uses the unit "MB" for MiB.
581+
"{}MB",
582+
MAX_TRINO_LOG_FILES_SIZE
583+
.scale_to(BinaryMultiple::Mebi)
584+
.div(LOG_FILE_COUNT as f32)
585+
.ceil()
586+
.value,
587+
)),
588+
);
589+
// The maximum size of all logfiles combined
590+
result.insert(
591+
LOG_MAX_TOTAL_SIZE.to_string(),
592+
Some(format!(
593+
// Trino uses the unit "MB" for MiB.
594+
"{}MB",
595+
MAX_TRINO_LOG_FILES_SIZE
596+
.scale_to(BinaryMultiple::Mebi)
597+
.ceil()
598+
.value,
599+
)),
600+
);
601+
555602
// disable http-request logs
556603
result.insert(
557604
"http-server.log.enabled".to_string(),
@@ -646,6 +693,7 @@ impl Configuration for v1alpha1::TrinoConfigFragment {
646693
}
647694
}
648695
LOG_PROPERTIES => {}
696+
ACCESS_CONTROL_PROPERTIES => {}
649697
_ => {}
650698
}
651699

0 commit comments

Comments
 (0)