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

[improve][io] Debezium sources: Support loading config from secrets #19004

Merged
merged 1 commit into from
Dec 23, 2022

Conversation

alpreu
Copy link
Contributor

@alpreu alpreu commented Dec 20, 2022

Motivation

Currently the Debezium sources don't support loading sensitive configuration from secrets.

Modifications

Most sources can rely on IOConfigUtils.loadWithSecrets because they provide a specific Config class with FieldDoc annotations. The Debezium sources work differently because there is no specific Config class but rather just a Map with some basic validation that is passed to Debezium. Therefore we need to directly get the secrets from the sourceContext.

Most of the Debezium sources rely on the same config properties (database.user, database.password) to configure the connector so I decided to move the secret-handling code into the abstract DebeziumSource class that all the individual connectors inherit from. There are exceptions to this, such as the MongoDB source, which uses another set of config properties (e.g. mongodb.user instead of database.user), these sources will have to call the secret-handling code by overriding the open method.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

I would be open to adding a unit test as well, but I feel it does not add a lot of value because the sourceContext already has unit tests for this functionality.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: alpreu#4

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 20, 2022
@Technoboy- Technoboy- added this to the 2.12.0 milestone Dec 21, 2022
@alpreu alpreu force-pushed the io-debezium-config-secrets branch from 58617b1 to 9d5a50c Compare December 21, 2022 13:26
@nicoloboschi
Copy link
Contributor

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2022

Codecov Report

Merging #19004 (9d5a50c) into master (feb3cb4) will increase coverage by 0.55%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19004      +/-   ##
============================================
+ Coverage     46.35%   46.91%   +0.55%     
- Complexity     8939    10548    +1609     
============================================
  Files           597      709     +112     
  Lines         56858    69363   +12505     
  Branches       5905     7441    +1536     
============================================
+ Hits          26357    32542    +6185     
- Misses        27616    33165    +5549     
- Partials       2885     3656     +771     
Flag Coverage Δ
unittests 46.91% <ø> (+0.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../pulsar/broker/service/SharedConsumerAssignor.java 3.70% <0.00%> (-64.82%) ⬇️
...apache/pulsar/broker/service/EntryAndMetadata.java 0.00% <0.00%> (-40.75%) ⬇️
...lsar/broker/loadbalance/impl/ThresholdShedder.java 3.27% <0.00%> (-27.05%) ⬇️
...che/pulsar/broker/intercept/BrokerInterceptor.java 0.00% <0.00%> (-13.05%) ⬇️
...tent/NonPersistentDispatcherMultipleConsumers.java 40.74% <0.00%> (-12.35%) ⬇️
...apache/pulsar/broker/service/TopicListService.java 40.80% <0.00%> (-12.00%) ⬇️
...sistent/PersistentDispatcherMultipleConsumers.java 49.47% <0.00%> (-8.51%) ⬇️
...org/apache/pulsar/broker/loadbalance/LoadData.java 58.33% <0.00%> (-8.34%) ⬇️
...ction/buffer/impl/TransactionBufferClientImpl.java 76.74% <0.00%> (-4.66%) ⬇️
.../buffer/impl/TransactionBufferClientStatsImpl.java 82.00% <0.00%> (-4.00%) ⬇️
... and 159 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants