Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: add iceberg_rest_catalog_authentication_mode #24986

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

WillemKauf
Copy link
Contributor

@WillemKauf WillemKauf commented Jan 30, 2025

Add iceberg_rest_catalog_authentication_mode and iceberg_rest_catalog_oauth2_server_uri configuration options.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

@WillemKauf WillemKauf requested a review from a team as a code owner January 30, 2025 22:17
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 31, 2025

Retry command for Build#61413

please wait until all jobs are finished before running the slash command


/ci-repeat 1
tests/rptest/tests/polaris_catalog_smoke_test.py::PolarisCatalogSmokeTest.test_connecting_to_catalog@{"cloud_storage_type":1,"with_tls":false}
tests/rptest/tests/polaris_catalog_smoke_test.py::PolarisCatalogSmokeTest.test_connecting_to_catalog@{"cloud_storage_type":1,"with_tls":true}

@WillemKauf WillemKauf force-pushed the iceberg_authentication_properties branch 2 times, most recently from f1ecafb to 819b3f9 Compare January 31, 2025 02:24
@WillemKauf WillemKauf force-pushed the iceberg_authentication_properties branch from 819b3f9 to 2b38389 Compare January 31, 2025 02:56
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 31, 2025

CI test results

test results on build#61423
test_id test_kind job_url test_status passed
gtest_raft_rpunit.gtest_raft_rpunit unit https://buildkite.com/redpanda/redpanda/builds/61423#0194ba4c-385f-48a1-bd80-40a25c387208 FLAKY 1/2
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery ducktape https://buildkite.com/redpanda/redpanda/builds/61423#0194baa8-73b0-4829-b1fe-6bd344b0b466 FLAKY 1/2
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery ducktape https://buildkite.com/redpanda/redpanda/builds/61423#0194baac-12d6-4de7-be47-f79878594dbc FLAKY 1/3
rptest.tests.datalake.compaction_test.CompactionGapsTest.test_translation_no_gaps.cloud_storage_type=CloudStorageType.S3 ducktape https://buildkite.com/redpanda/redpanda/builds/61423#0194baac-12d7-4f9b-b2b5-f68739b0ee24 FLAKY 1/2
rptest.tests.partition_force_reconfiguration_test.NodeWiseRecoveryTest.test_node_wise_recovery.dead_node_count=1 ducktape https://buildkite.com/redpanda/redpanda/builds/61423#0194baa8-73b0-4829-b1fe-6bd344b0b466 FLAKY 1/2
test results on build#61438
test_id test_kind job_url test_status passed
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery ducktape https://buildkite.com/redpanda/redpanda/builds/61438#0194bce7-8113-4b07-bac1-0261019fff89 FLAKY 1/2
rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance ducktape https://buildkite.com/redpanda/redpanda/builds/61438#0194bce7-8115-4648-bc9a-a9632dc84135 FLAKY 1/2
rptest.tests.datalake.simple_connect_test.RedpandaConnectIcebergTest.test_translating_avro_serialized_records.cloud_storage_type=CloudStorageType.S3.scenario=simple ducktape https://buildkite.com/redpanda/redpanda/builds/61438#0194bce3-b648-4de6-81d6-0a94cf40d184 FLAKY 1/2
rptest.tests.internal_topic_protection_test.InternalTopicProtectionLargeClusterTest.test_consumer_offset_topic ducktape https://buildkite.com/redpanda/redpanda/builds/61438#0194bce7-8115-4648-bc9a-a9632dc84135 FLAKY 1/2
rptest.tests.partition_movement_test.SIPartitionMovementTest.test_shadow_indexing.num_to_upgrade=0.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/61438#0194bce7-8115-4648-bc9a-a9632dc84135 FLAKY 1/2
test results on build#61474
test_id test_kind job_url test_status passed
rptest.tests.compaction_recovery_test.CompactionRecoveryUpgradeTest.test_index_recovery_after_upgrade ducktape https://buildkite.com/redpanda/redpanda/builds/61474#0194bf68-19f3-443a-aa71-3c6360d84182 FLAKY 1/2
rptest.tests.datalake.datalake_e2e_test.DatalakeE2ETests.test_topic_lifecycle.cloud_storage_type=CloudStorageType.S3.filesystem_catalog_mode=True ducktape https://buildkite.com/redpanda/redpanda/builds/61474#0194bf71-acf6-446d-aebf-c791682c9ad4 FLAKY 1/2

@WillemKauf WillemKauf force-pushed the iceberg_authentication_properties branch 2 times, most recently from 1a892e9 to bb87125 Compare January 31, 2025 13:42
src/v/config/configuration.cc Show resolved Hide resolved
Comment on lines 142 to 143
std::optional<iceberg::rest_client::credentials>& credentials,
std::optional<iceberg::rest_client::oauth_token>& token) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just return a struct or pair?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, happy to change this from in/out parameters to a return type if that would be more ergonomic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we generally don't use out-parameters unless the isn't a good alternative. struct credentials_or_token { ... };

src/v/iceberg/tests/rest_catalog_test.cc Show resolved Hide resolved
For allowing the user to specify a uri to an OAuth2 server
for use in authenticating requests made by the `catalog_client`
to the catalog.
For configuring the authentication method used for the `catalog_client`
requests made to the catalog.

There are three possible values for this `enum`:

  1. `::none`, where no authentication is attempted.
  2. `::bearer`, where the token provided via `iceberg_rest_catalog_token`
      is used for authentication.
  3. `::oauth2`, where the client credentials defined via
     `iceberg_rest_catalog_client_id/secret` are used to retrieve
      an access token for future use.
@WillemKauf WillemKauf force-pushed the iceberg_authentication_properties branch from bb87125 to a5b738a Compare January 31, 2025 21:33
@WillemKauf
Copy link
Contributor Author

Force push to:

  • Move commit history around for config properties
  • Squash commit for rest_catalog_test

Depending on authentication settings, we may either construct
user credentials for use with the OAuth2 endpoint for retrieving a
token, construct the token directly, or do nothing at all.
@WillemKauf WillemKauf force-pushed the iceberg_authentication_properties branch from a5b738a to 84e8449 Compare February 1, 2025 00:32
@WillemKauf
Copy link
Contributor Author

Force push to:

  • change maybe_make_credentials_or_token() to make_credentials_and_token()

@WillemKauf WillemKauf force-pushed the iceberg_authentication_properties branch from 84e8449 to 493a2a2 Compare February 1, 2025 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants