Check secret existence #14865
Check secret existence #14865
Conversation
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| @@ -1262,6 +1294,11 @@ TRestoreResult TRestoreClient::RestoreExternalDataSource( | |||
| } | |||
|
|
|||
| TString query = ReadExternalDataSourceQuery(fsPath, Log.get()); | |||
| if (const auto secretName = GetSecretName(query)) { | |||
There was a problem hiding this comment.
External data sources can depend on multiple secrets, see TAuth message describing the authorization for a particular data source
| return GetToken(query, R"(-- database: ")"); | ||
| } | ||
|
|
||
| TString GetSecretName(const TString& query) { |
There was a problem hiding this comment.
This function has very limited functionality. I guess it is enough to check restorability of an external data source that uses static authorization (login + password), or an authorization by token. However, there are a lot more supported authorization methods, which require different secret names.
Moreover, why even bother detecting missing secrets, if an external data source (and I guess an async replication too) will detect the missing secrets themselves on the creation attempt?
| @@ -808,7 +808,7 @@ Ydb::Table::DescribeExternalDataSourceResult DescribeExternalDataSource(TDriver | |||
|
|
|||
| std::string ToString(std::string_view key, std::string_view value) { | |||
| // indented to follow the default YQL formatting | |||
| return std::format(R"( {} = "{}")", key, value); | |||
| return std::format(R"( {} = '{}')", key, value); | |||
There was a problem hiding this comment.
Nitpick: why 2 spaces and not 4? Isn't the 4 spaces the default formatting?
| @@ -1110,6 +1111,32 @@ TRestoreResult TRestoreClient::RestoreTopic( | |||
| return Result<TRestoreResult>(dbPath, std::move(result)); | |||
| } | |||
|
|
|||
| TRestoreResult TRestoreClient::CheckSecretExistence(const TString& secretName) { | |||
There was a problem hiding this comment.
I would like to see a little comment explaining how this is the only way to check the secret existence at the moment, because this approach is very counter intuitive. Possibly in the later PRs
Changelog entry
...
Changelog category
Description for reviewers
...