Skip to content

Commit

Permalink
Merge branch 'main' into update-release-branch
Browse files Browse the repository at this point in the history
  • Loading branch information
siegfriedweber committed Nov 26, 2024
2 parents cbbcc17 + f3eb0af commit c311ba7
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 38 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
- BREAKING: Use distinct ServiceAccounts for the Stacklets, so that multiple Stacklets can be
deployed in one namespace. Existing Stacklets will use the newly created ServiceAccounts after
restart ([#545]).
- Fix OIDC endpoint construction in case the `rootPath` does not have a trailing slash ([#547]).

[#545]: https://github.com/stackabletech/airflow-operator/pull/545
[#547]: https://github.com/stackabletech/airflow-operator/pull/547

## [24.11.0] - 2024-11-18

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ tracing = "0.1"

# [patch."https://github.com/stackabletech/operator-rs.git"]
# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" }
# stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
2 changes: 1 addition & 1 deletion rust/operator-binary/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ tracing.workspace = true
indoc.workspace = true

[build-dependencies]

built.workspace = true

[dev-dependencies]
rstest.workspace = true
serde_yaml.workspace = true
103 changes: 66 additions & 37 deletions rust/operator-binary/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ pub enum Error {
FailedToCreateLdapEndpointUrl {
source: stackable_operator::commons::authentication::ldap::Error,
},

#[snafu(display("invalid OIDC endpoint"))]
InvalidOidcEndpoint {
source: stackable_operator::commons::authentication::oidc::Error,
},

#[snafu(display("invalid well-known OIDC configuration URL"))]
InvalidWellKnownConfigUrl {
source: stackable_operator::commons::authentication::oidc::Error,
},
}

pub fn add_airflow_config(
Expand Down Expand Up @@ -78,7 +88,7 @@ fn append_authentication_config(
}

if !oidc_providers.is_empty() {
append_oidc_config(config, &oidc_providers);
append_oidc_config(config, &oidc_providers)?;
}

config.insert(
Expand Down Expand Up @@ -192,7 +202,7 @@ fn append_oidc_config(
&oidc::AuthenticationProvider,
&oidc::ClientAuthenticationOptions<()>,
)],
) {
) -> Result<(), Error> {
// Debatable: AUTH_OAUTH or AUTH_OID
// Additionally can be set via config
config.insert(
Expand All @@ -217,6 +227,13 @@ fn append_oidc_config(

let oauth_providers_config_entry = match oidc_provider {
oidc::IdentityProviderHint::Keycloak => {
let endpoint_url = oidc.endpoint_url().context(InvalidOidcEndpointSnafu)?;
let mut api_base_url = endpoint_url.as_str().trim_end_matches('/').to_owned();
api_base_url.push_str("/protocol/");
let well_known_config_url = oidc
.well_known_config_url()
.context(InvalidWellKnownConfigUrlSnafu)?;

formatdoc!(
"
{{ 'name': 'keycloak',
Expand All @@ -228,11 +245,10 @@ fn append_oidc_config(
'client_kwargs': {{
'scope': '{scopes}'
}},
'api_base_url': '{url}/protocol/',
'server_metadata_url': '{url}/.well-known/openid-configuration',
'api_base_url': '{api_base_url}',
'server_metadata_url': '{well_known_config_url}',
}},
}}",
url = oidc.endpoint_url().unwrap(),
scopes = scopes.join(" "),
)
}
Expand All @@ -251,12 +267,15 @@ fn append_oidc_config(
joined_oauth_providers_config = oauth_providers_config.join(",\n")
),
);

Ok(())
}

#[cfg(test)]
mod tests {
use crate::config::add_airflow_config;
use indoc::indoc;
use indoc::formatdoc;
use rstest::rstest;
use stackable_airflow_crd::authentication::{
default_sync_roles_at, default_user_registration, AirflowAuthenticationClassResolved,
AirflowClientAuthenticationDetailsResolved, FlaskRolesSyncMoment,
Expand Down Expand Up @@ -339,12 +358,15 @@ mod tests {
]), result);
}

#[test]
fn test_oidc_config() {
let oidc_provider_yaml1 = r#"
hostname: my.keycloak1.server
#[rstest]
#[case("/realms/sdp")]
#[case("/realms/sdp/")]
#[case("/realms/sdp/////")]
fn test_oidc_config(#[case] root_path: &str) {
let oidc_provider_yaml1 = formatdoc!(
"hostname: my.keycloak1.server
port: 12345
rootPath: my-root-path
rootPath: {root_path}
tls:
verification:
server:
Expand All @@ -356,8 +378,9 @@ mod tests {
- email
- profile
provider_hint: Keycloak
"#;
let deserializer = serde_yaml::Deserializer::from_str(oidc_provider_yaml1);
"
);
let deserializer = serde_yaml::Deserializer::from_str(&oidc_provider_yaml1);
let oidc_provider1: oidc::AuthenticationProvider =
serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap();

Expand Down Expand Up @@ -399,41 +422,47 @@ mod tests {
let mut result = BTreeMap::new();
add_airflow_config(&mut result, &authentication_config).expect("Ok");

assert_eq!(BTreeMap::from([
("AUTH_ROLES_SYNC_AT_LOGIN".into(), "false".into()),
("AUTH_TYPE".into(), "AUTH_OAUTH".into()),
("AUTH_USER_REGISTRATION".into(), "true".into()),
("AUTH_USER_REGISTRATION_ROLE".into(), "Admin".into()),
("OAUTH_PROVIDERS".into(), indoc!("
assert_eq!(
BTreeMap::from([
("AUTH_ROLES_SYNC_AT_LOGIN".into(), "false".into()),
("AUTH_TYPE".into(), "AUTH_OAUTH".into()),
("AUTH_USER_REGISTRATION".into(), "true".into()),
("AUTH_USER_REGISTRATION_ROLE".into(), "Admin".into()),
(
"OAUTH_PROVIDERS".into(),
formatdoc! {"
[
{ 'name': 'keycloak',
{{ 'name': 'keycloak',
'icon': 'fa-key',
'token_key': 'access_token',
'remote_app': {
'remote_app': {{
'client_id': os.environ.get('OIDC_A96BCC4FA49835D2_CLIENT_ID'),
'client_secret': os.environ.get('OIDC_A96BCC4FA49835D2_CLIENT_SECRET'),
'client_kwargs': {
'client_kwargs': {{
'scope': 'openid email profile roles'
},
'api_base_url': 'https://my.keycloak1.server:12345/my-root-path/protocol/',
'server_metadata_url': 'https://my.keycloak1.server:12345/my-root-path/.well-known/openid-configuration',
},
},
{ 'name': 'keycloak',
}},
'api_base_url': 'https://my.keycloak1.server:12345/realms/sdp/protocol/',
'server_metadata_url': 'https://my.keycloak1.server:12345/realms/sdp/.well-known/openid-configuration',
}},
}},
{{ 'name': 'keycloak',
'icon': 'fa-key',
'token_key': 'access_token',
'remote_app': {
'remote_app': {{
'client_id': os.environ.get('OIDC_3A305E38C3B561F3_CLIENT_ID'),
'client_secret': os.environ.get('OIDC_3A305E38C3B561F3_CLIENT_SECRET'),
'client_kwargs': {
'client_kwargs': {{
'scope': 'openid'
},
'api_base_url': 'http://my.keycloak2.server//protocol/',
'server_metadata_url': 'http://my.keycloak2.server//.well-known/openid-configuration',
},
}
}},
'api_base_url': 'http://my.keycloak2.server/protocol/',
'server_metadata_url': 'http://my.keycloak2.server/.well-known/openid-configuration',
}},
}}
]
").into())
]), result);
"}
)
]),
result
);
}
}

0 comments on commit c311ba7

Please sign in to comment.