Skip to content

Commit cc6fab6

Browse files
committed
datalake: add iceberg_rest_catalog_credentials_host config
When cloud_storage_credentials_host is set (e.g. to an STS endpoint), it was being applied to Iceberg REST catalog credential requests too, regardless of the catalog's credentials source. This caused failures when the catalog uses aws_instance_metadata (which expects 169.254.169.254) but the cloud storage host override points elsewhere. Add iceberg_rest_catalog_credentials_host to allow the catalog credentials host to be set independently. Thread host_override through build_refresh_credentials_source and s3_compat_config down to make_refresh_credentials, replacing the direct config read in the template with an explicit parameter.
1 parent 2e0ec2a commit cc6fab6

10 files changed

Lines changed: 43 additions & 12 deletions

File tree

src/v/cloud_roles/auth_refresh_bg_op.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ void auth_refresh_bg_op::do_start_auth_refresh_op(
5757
try {
5858
cloud_roles::aws_service_name service_name;
5959
cloud_roles::aws_region_name region_name;
60+
std::optional<ss::sstring> host_override;
6061

6162
ss::visit(
6263
_source_config,
@@ -69,6 +70,7 @@ void auth_refresh_bg_op::do_start_auth_refresh_op(
6970
s3_cfg) {
7071
service_name = s3_cfg.service;
7172
region_name = s3_cfg.region;
73+
host_override = s3_cfg.host;
7274
},
7375
[&](const cloud_roles::auth_refresh_bg_op::abs_config&) {});
7476

@@ -80,6 +82,7 @@ void auth_refresh_bg_op::do_start_auth_refresh_op(
8082
service_name,
8183
region_name,
8284
std::nullopt,
85+
std::move(host_override),
8386
cloud_roles::default_retry_params,
8487
std::move(metrics_tag)));
8588

src/v/cloud_roles/auth_refresh_bg_op.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class auth_refresh_bg_op {
2929
struct s3_compat_config {
3030
cloud_roles::aws_service_name service;
3131
cloud_roles::aws_region_name region;
32+
std::optional<ss::sstring> host;
3233
};
3334
struct abs_config {};
3435

src/v/cloud_roles/refresh_credentials.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@ refresh_credentials make_refresh_credentials(
406406
aws_service_name service,
407407
aws_region_name region,
408408
std::optional<net::unresolved_address> endpoint,
409+
std::optional<ss::sstring> host_override,
409410
retry_params retry_params,
410411
ss::sstring metrics_tag) {
411412
switch (cloud_credentials_source) {
@@ -423,6 +424,7 @@ refresh_credentials make_refresh_credentials(
423424
std::move(service),
424425
std::move(region),
425426
std::move(endpoint),
427+
std::move(host_override),
426428
retry_params,
427429
std::move(metrics_tag));
428430
case model::cloud_credentials_source::sts:
@@ -432,6 +434,7 @@ refresh_credentials make_refresh_credentials(
432434
std::move(service),
433435
std::move(region),
434436
std::move(endpoint),
437+
std::move(host_override),
435438
retry_params,
436439
std::move(metrics_tag));
437440
case model::cloud_credentials_source::gcp_instance_metadata:
@@ -441,6 +444,7 @@ refresh_credentials make_refresh_credentials(
441444
std::move(service),
442445
std::move(region),
443446
std::move(endpoint),
447+
std::move(host_override),
444448
retry_params,
445449
std::move(metrics_tag));
446450
case model::cloud_credentials_source::azure_aks_oidc_federation:
@@ -450,6 +454,7 @@ refresh_credentials make_refresh_credentials(
450454
std::move(service),
451455
std::move(region),
452456
std::move(endpoint),
457+
std::move(host_override),
453458
retry_params,
454459
std::move(metrics_tag));
455460
case model::cloud_credentials_source::azure_vm_instance_metadata:
@@ -459,6 +464,7 @@ refresh_credentials make_refresh_credentials(
459464
std::move(service),
460465
std::move(region),
461466
std::move(endpoint),
467+
std::move(host_override),
462468
retry_params,
463469
std::move(metrics_tag));
464470
}

src/v/cloud_roles/refresh_credentials.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ refresh_credentials make_refresh_credentials(
188188
aws_service_name service,
189189
aws_region_name region,
190190
std::optional<net::unresolved_address> endpoint = std::nullopt,
191+
std::optional<ss::sstring> host_override = std::nullopt,
191192
retry_params retry_params = default_retry_params,
192193
ss::sstring metrics_tag = "") {
193194
ss::sstring host = {
@@ -196,17 +197,14 @@ refresh_credentials make_refresh_credentials(
196197
if (endpoint) {
197198
host = endpoint->host();
198199
}
199-
if (
200-
auto cfg_host
201-
= config::shard_local_cfg().cloud_storage_credentials_host();
202-
cfg_host.has_value()) {
200+
if (host_override.has_value()) {
203201
vlog(
204202
clrl_log.info,
205203
"overriding default cloud roles credentials host {} with {} set "
206204
"in configuration.",
207205
host,
208-
cfg_host.value());
209-
host = cfg_host.value();
206+
host_override.value());
207+
host = host_override.value();
210208
}
211209
auto port = endpoint ? endpoint->port() : CredentialsProvider::default_port;
212210
auto impl = std::make_unique<CredentialsProvider>(
@@ -232,6 +230,7 @@ refresh_credentials make_refresh_credentials(
232230
aws_service_name service,
233231
aws_region_name region,
234232
std::optional<net::unresolved_address> endpoint = std::nullopt,
233+
std::optional<ss::sstring> host_override = std::nullopt,
235234
retry_params retry_params = default_retry_params,
236235
ss::sstring metrics_tag = "");
237236

src/v/cloud_storage_clients/configuration.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,8 @@ std::ostream& operator<<(std::ostream& o, const client_configuration& c) {
342342
cloud_roles::auth_refresh_bg_op::credentials_source_config
343343
build_refresh_credentials_source(
344344
const client_configuration& config,
345-
model::cloud_credentials_source cloud_credentials_source) {
345+
model::cloud_credentials_source cloud_credentials_source,
346+
std::optional<ss::sstring> host_override) {
346347
if (
347348
cloud_credentials_source
348349
== model::cloud_credentials_source::config_file) {
@@ -366,10 +367,12 @@ build_refresh_credentials_source(
366367
} else {
367368
return ss::visit(
368369
config,
369-
[](const cloud_storage_clients::s3_configuration& s3_cfg)
370+
[&](const cloud_storage_clients::s3_configuration& s3_cfg)
370371
-> cloud_roles::auth_refresh_bg_op::credentials_source_config {
371372
return cloud_roles::auth_refresh_bg_op::s3_compat_config{
372-
.service = s3_cfg.service, .region = s3_cfg.region};
373+
.service = s3_cfg.service,
374+
.region = s3_cfg.region,
375+
.host = host_override};
373376
},
374377
[](const cloud_storage_clients::abs_configuration&)
375378
-> cloud_roles::auth_refresh_bg_op::credentials_source_config {

src/v/cloud_storage_clients/configuration.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ model::cloud_storage_backend infer_backend_from_configuration(
239239
cloud_roles::auth_refresh_bg_op::credentials_source_config
240240
build_refresh_credentials_source(
241241
const client_configuration& config,
242-
model::cloud_credentials_source cloud_credentials_source);
242+
model::cloud_credentials_source cloud_credentials_source,
243+
std::optional<ss::sstring> host_override = std::nullopt);
243244

244245
ss::future<ss::shared_ptr<ss::tls::certificate_credentials>>
245246
build_tls_credentials(const client_configuration& config);

src/v/cloud_storage_clients/credential_manager.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ credential_manager::credential_manager(
2121
model::cloud_credentials_source cloud_credentials_source)
2222
: _upstream(upstream)
2323
, _client_conf(std::move(conf))
24-
, _auth_refresh_bg_op{pool_log, _gate, _as, cloud_credentials_source, cloud_storage_clients::build_refresh_credentials_source(_client_conf, cloud_credentials_source)}
24+
, _auth_refresh_bg_op{pool_log, _gate, _as, cloud_credentials_source, cloud_storage_clients::build_refresh_credentials_source(_client_conf, cloud_credentials_source, config::shard_local_cfg().cloud_storage_credentials_host())}
2525
, _azure_shared_key_binding(
2626
config::shard_local_cfg().cloud_storage_azure_shared_key.bind()) {}
2727

src/v/config/configuration.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4316,6 +4316,21 @@ configuration::configuration()
43164316
{.needs_restart = needs_restart::yes, .visibility = visibility::user},
43174317
std::nullopt,
43184318
&validate_non_empty_string_opt)
4319+
, iceberg_rest_catalog_credentials_host(
4320+
*this,
4321+
"iceberg_rest_catalog_credentials_host",
4322+
"The hostname to connect to for retrieving role based credentials for "
4323+
"the Iceberg REST catalog. May be required when the REST catalog uses a "
4324+
"different credentials source than cloud storage -- for example, if "
4325+
"cloud_storage_credentials_source is set to sts with a custom "
4326+
"credentials "
4327+
"host, but the REST catalog uses aws_instance_metadata, this should be "
4328+
"set to the EC2 instance metadata address (169.254.169.254) to prevent "
4329+
"the cloud storage host override from applying to catalog credential "
4330+
"requests.",
4331+
{.needs_restart = needs_restart::yes, .visibility = visibility::user},
4332+
std::nullopt,
4333+
&validate_non_empty_string_opt)
43194334
, iceberg_backlog_controller_p_coeff(
43204335
*this,
43214336
"iceberg_backlog_controller_p_coeff",

src/v/config/configuration.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,7 @@ struct configuration final : public config_store {
743743
enum_property<std::optional<model::cloud_credentials_source>>
744744
iceberg_rest_catalog_aws_credentials_source;
745745
property<std::optional<ss::sstring>> iceberg_rest_catalog_gcp_user_project;
746+
property<std::optional<ss::sstring>> iceberg_rest_catalog_credentials_host;
746747
property<double> iceberg_backlog_controller_p_coeff;
747748
property<double> iceberg_backlog_controller_i_coeff;
748749
bounded_property<uint32_t> iceberg_target_backlog_size;

src/v/datalake/credential_manager.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,9 @@ void credential_manager::start_auth_refresh_if_needed() {
235235

236236
auto config_source
237237
= cloud_storage_clients::build_refresh_credentials_source(
238-
*client_config, get_credentials_source(cfg));
238+
*client_config,
239+
get_credentials_source(cfg),
240+
cfg.iceberg_rest_catalog_credentials_host());
239241

240242
auth_refresh_bg_op_.emplace(
241243
datalake_log,

0 commit comments

Comments
 (0)