Skip to content

Conversation

@alpreu
Copy link
Contributor

@alpreu alpreu commented Dec 19, 2022

Motivation

The Elasticsearch sink does currently not support loading config properties such as credentials from secrets.

Modifications

Use IOConfigUtils.loadWithSecrets to enable loading sensitive sink configurations from secrets too.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • added unit test for loading the config from secrets

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
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: alpreu#3

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 19, 2022
@alpreu alpreu force-pushed the io-elasticsearch-config-secrets branch from 77cdee5 to 984801b Compare December 20, 2022 08:33
@alpreu alpreu force-pushed the io-elasticsearch-config-secrets branch from 984801b to cad39c3 Compare December 20, 2022 08:46
Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

+1

@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.91%. Comparing base (feb3cb4) to head (cad39c3).
Report is 2047 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18986      +/-   ##
============================================
+ Coverage     46.35%   46.91%   +0.55%     
- Complexity     8939    10563    +1624     
============================================
  Files           597      706     +109     
  Lines         56858    69005   +12147     
  Branches       5905     7391    +1486     
============================================
+ Hits          26357    32372    +6015     
- Misses        27616    33032    +5416     
- Partials       2885     3601     +716     
Flag Coverage Δ
unittests 46.91% <ø> (+0.55%) ⬆️

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

see 165 files with indirect coverage changes

@alpreu
Copy link
Contributor Author

alpreu commented Jan 2, 2023

/pulsarbot rerun-failure-checks

@alpreu
Copy link
Contributor Author

alpreu commented Jan 3, 2023

Can we merge this?

@nicoloboschi
Copy link
Contributor

/pulsarbot rerun-failure-checks

@alpreu
Copy link
Contributor Author

alpreu commented Jan 5, 2023

@nicoloboschi The failing OWASP dependency check is independent of this PR, we cannot get it to turn green, it has to be fixed by itself. Can we go ahead with merging this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connector 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.

3 participants