Skip to content

Commit 9266f48

Browse files
committed
fix: consolidate env vars
1 parent ef6a0bd commit 9266f48

3 files changed

Lines changed: 24 additions & 11 deletions

File tree

rust/operator-binary/src/controller/build/resource/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,10 @@ pub(crate) fn build_superset_container_builder(
174174
);
175175
}
176176

177-
// SECRET_KEY from auto-generated secret
177+
// The Flask `SECRET_KEY` env var is sourced from the auto-generated Secret. Superset requires the
178+
// env var name to equal the Secret data key, so both use `INTERNAL_SECRET_SECRET_KEY`.
178179
superset_cb.add_env_var_from_secret(
179-
"SECRET_KEY",
180+
INTERNAL_SECRET_SECRET_KEY,
180181
validated.cluster_config.secret_key_secret_name.clone(),
181182
INTERNAL_SECRET_SECRET_KEY,
182183
);

rust/operator-binary/src/crd/authentication.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ pub fn default_user_registration() -> bool {
154154
/// Default Superset role assigned to users on registration.
155155
pub const DEFAULT_USER_REGISTRATION_ROLE: &str = "Public";
156156

157+
/// The OIDC principal claim Superset enforces for the Keycloak provider, due to the Flask-AppBuilder
158+
/// implementation (see <https://github.com/dpgaspar/Flask-AppBuilder/blob/6d44e6d581433dcea475764c4bb1270c24bbd6de/flask_appbuilder/security/manager.py#L719>).
159+
pub const ENFORCED_PRINCIPAL_CLAIM: &str = "preferred_username";
160+
157161
pub fn default_user_registration_role() -> String {
158162
DEFAULT_USER_REGISTRATION_ROLE.to_string()
159163
}
@@ -310,15 +314,14 @@ impl SupersetClientAuthenticationDetailsResolved {
310314
}
311315
);
312316

313-
// We have to enforce preferred_username here due to the flask implementation
314-
// https://github.com/dpgaspar/Flask-AppBuilder/blob/6d44e6d581433dcea475764c4bb1270c24bbd6de/flask_appbuilder/security/manager.py#L719
317+
// We have to enforce `ENFORCED_PRINCIPAL_CLAIM` here due to the Flask-AppBuilder implementation.
315318
match oidc_provider {
316319
oidc::v1alpha1::IdentityProviderHint::Keycloak => {
317320
ensure!(
318-
&provider.principal_claim == "preferred_username",
321+
provider.principal_claim == ENFORCED_PRINCIPAL_CLAIM,
319322
OidcPrincipalClaimNotSupportedSnafu {
320323
configured: provider.principal_claim.clone(),
321-
supported: "preferred_username".to_owned(),
324+
supported: ENFORCED_PRINCIPAL_CLAIM.to_owned(),
322325
auth_class_name,
323326
}
324327
);

rust/operator-binary/src/druid_connection_controller/mod.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ use strum::{EnumDiscriminants, IntoStaticStr};
2929
use crate::{
3030
APP_NAME, OPERATOR_NAME,
3131
built_info::PKG_VERSION,
32-
controller::{CONTAINER_IMAGE_BASE_NAME, build::properties::ConfigFileName},
32+
controller::{
33+
CONTAINER_IMAGE_BASE_NAME,
34+
build::{properties::ConfigFileName, resource::bash_wrapper_command},
35+
},
3336
crd::{
3437
INTERNAL_SECRET_SECRET_KEY, METADATA_DATABASE_ENV_PREFIX, PYTHONPATH, druidconnection,
3538
v1alpha1,
@@ -318,6 +321,10 @@ fn build_druid_db_yaml(druid_cluster_name: &str, sqlalchemy_str: &str) -> Result
318321
))
319322
}
320323

324+
/// Name of the env var (and the matching `superset_config.py` setting) holding the metadata
325+
/// database connection string for the import job.
326+
const SQLALCHEMY_DATABASE_URI_ENV: &str = "SQLALCHEMY_DATABASE_URI";
327+
321328
/// Builds the import job. When run it will import the druid connection into the database.
322329
async fn build_import_job(
323330
superset_cluster: &v1alpha1::SupersetCluster,
@@ -328,7 +335,9 @@ async fn build_import_job(
328335
) -> Result<Job> {
329336
let mut commands = vec![];
330337

331-
let config = "import os; SQLALCHEMY_DATABASE_URI = os.path.expandvars(os.environ.get('SQLALCHEMY_DATABASE_URI'))";
338+
let config = format!(
339+
"import os; {SQLALCHEMY_DATABASE_URI_ENV} = os.path.expandvars(os.environ.get('{SQLALCHEMY_DATABASE_URI_ENV}'))"
340+
);
332341
commands.push(format!("mkdir -p {PYTHONPATH}"));
333342
commands.push(format!(
334343
"echo \"{config}\" > {PYTHONPATH}/{config_file}",
@@ -356,10 +365,10 @@ async fn build_import_job(
356365
.expect("ContainerBuilder not created");
357366
container_builder
358367
.image_from_product_image(resolved_product_image)
359-
.command(vec!["/bin/bash".to_string()])
360-
.args(vec![String::from("-c"), commands.join("; ")])
368+
.command(bash_wrapper_command())
369+
.args(vec![commands.join("; ")])
361370
.add_env_var(
362-
"SQLALCHEMY_DATABASE_URI",
371+
SQLALCHEMY_DATABASE_URI_ENV,
363372
metadata_database_connection_details.url_template.clone(),
364373
)
365374
.add_env_var_from_secret(

0 commit comments

Comments
 (0)