Remove Watcher Account "unsecure" settings #36736
Conversation
|
Pinging @elastic/es-core-features |
| throw requiredSettingException(accountName, settingName); | ||
| } | ||
| value = secureString.toString(); | ||
| private static String getSetting(String accountName, Settings settings, Setting<SecureString> secureSetting) { |
There was a problem hiding this comment.
This seems like something we could put in Account
There was a problem hiding this comment.
Agreed, but I think that's a job for another PR (see prev comment)
| } else { | ||
| return new SecureString(value.toCharArray()); | ||
| return null; |
There was a problem hiding this comment.
this seems a bit different from the other getSecureSettings that are in the other *Account classes, and somewhat trappy. Why would we be returning null instead of throwing up like we do in the others? I ask because I hope we can consolidate the functionality into only Account, if thats at all possible, instead of having a method that does this validation in every *Account class.
There was a problem hiding this comment.
@hub-cap There is no Account base class. Also I tried to touch as less as possible to make reviewing easier. IMO we should be using the SecureSetting from the corresponding Service (EmailService in this case). But I prefer to leave this for another time. WDYT?
|
test this please |
1 similar comment
|
test this please |
|
@hub-cap is this LGTMd or you still need to look into it? |
|
@elasticmachine run elasticsearch-ci-1 |
|
run gradle build test 1 |
|
run gradle build tests 1 |
|
run gradle build tests 2 |
|
run gradle build tests 1 |
|
run gradle build tests 2 |
|
run gradle build tests 1 |
|
Hey @hub-cap , Apparently I've not adapted the tests, waiting for the reviews (a bad habit I got into), and then forgot about it. I have done it now. It was just grunt work but it was more extensive than I thought it would be. May I please ask you to cast an eye, maybe it rings some bell, just to be surer I haven't subtly messed smth up. Nevertheless, I will be crawling through CI... |
|
run gradle build tests 2 |
* elastic/master: Remove Watcher Account "unsecure" settings (elastic#36736) Add cache cleaning task for ML snapshot (elastic#37505) Update jdk used by the docker builds (elastic#37621) Remove an unused constant in PutMappingRequest. Update get users to allow unknown fields (elastic#37593) Do not add index event listener if CCR disabled (elastic#37432) Add local session timeouts to leader node (elastic#37438)
* elastic/master: (104 commits) Permission for restricted indices (elastic#37577) Remove Watcher Account "unsecure" settings (elastic#36736) Add cache cleaning task for ML snapshot (elastic#37505) Update jdk used by the docker builds (elastic#37621) Remove an unused constant in PutMappingRequest. Update get users to allow unknown fields (elastic#37593) Do not add index event listener if CCR disabled (elastic#37432) Add local session timeouts to leader node (elastic#37438) Add some deprecation optimizations (elastic#37597) refactor inner geogrid classes to own class files (elastic#37596) Remove obsolete deprecation checks (elastic#37510) ML: Add support for single bucket aggs in Datafeeds (elastic#37544) ML: creating ML State write alias and pointing writes there (elastic#37483) Deprecate types in the put mapping API. (elastic#37280) [ILM] Add unfollow action (elastic#36970) Packaging: Update marker used to allow ELASTIC_PASSWORD (elastic#37243) Fix setting openldap realm ssl config Document the need for JAVA11_HOME (elastic#37589) SQL: fix object extraction from sources (elastic#37502) Nit in settings.gradle for Eclipse ...
Removes all the settings for watcher notifications accounts that are dynamic cluster filtered, and have a corresponding secure sibling.
They are deprecated in favor of the sibling that is stored in the keystore.
Some documentation is still pending.Follow-up on #36403