Secrets manager integration test #127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yanw-bq
changed the title
justing-bq
reviewed
Mar 31, 2023
driver/connection_proxy.cc
Outdated
| // We disable this flag after fetching the custom message once | ||
| // so it does not obscure future proxy errors. | ||
| has_custom_error_message = false; | ||
|
|
There was a problem hiding this comment.
Can we get rid of non-changes like this?
There was a problem hiding this comment.
This just removes extra some spaces/tabs. Not sure how many I should add back.
There was a problem hiding this comment.
Just copy and paste what was there before your change.
driver/secrets_manager_proxy.cc
Outdated
| if (get_secret_value_outcome.IsSuccess()) { | ||
| secret_string = get_secret_value_outcome.GetResult().GetSecretString(); | ||
| } | ||
| else { | ||
| MYLOG_DBC_TRACE(dbc, get_secret_value_outcome.GetError().GetMessage().c_str()); | ||
| MYLOG_DBC_TRACE(dbc, "[SECRETS_MANAGER_PROXY] %s", get_secret_value_outcome.GetError().GetMessage().c_str()); |
There was a problem hiding this comment.
Can we store get_secret_value_outcome.GetError().GetMessage().c_str() in a variable rather than invoking it twice?
| @@ -300,6 +300,8 @@ SQLWCHAR *dsnparams[]= {W_DSN, W_DRIVER, W_DESCRIPTION, W_SERVER, | |||
| W_SSLMODE, W_NO_DATE_OVERFLOW, W_LOAD_DATA_LOCAL_DIR, | |||
| W_OCI_CONFIG_FILE, W_TLS_VERSIONS, | |||
| W_SSL_CRL, W_SSL_CRLPATH, | |||
| /* AWS Auth */ | |||
| W_AUTH_MODE, W_AUTH_REGION, W_AUTH_HOST, W_AUTH_PORT, W_AUTH_EXPIRATION, W_AUTH_SECRET_ID, | |||
There was a problem hiding this comment.
Oops. Not sure how this was originally missed. Thanks for catching it.
| }; | ||
|
|
||
| TEST_F(SecretsManagerIntegrationTest, EnableSecretsManager) { | ||
| connection_string = builder.withDSN(dsn).withServer(MYSQL_CLUSTER_URL).withAuthMode("SECRETS MANAGER").withAuthRegion("us-east-2").withSecretId(SECRETS_ARN).build(); |
There was a problem hiding this comment.
I think this builder stuff is easier to read when each .with is on its own line. Take a look at my PR for reference.
| connection_string = builder.withDSN(dsn).withServer(MYSQL_CLUSTER_URL).withAuthMode("SECRETS MANAGER").withAuthRegion("us-east-1").withSecretId(SECRETS_ARN).build(); | ||
| SQLCHAR conn_out[4096] = "\0"; | ||
| SQLSMALLINT len; | ||
| EXPECT_EQ(SQL_ERROR, SQLDriverConnect(dbc, nullptr, AS_SQLCHAR(connection_string.c_str()), SQL_NTS, conn_out, MAX_NAME_LEN, &len, SQL_DRIVER_NOPROMPT)); |
There was a problem hiding this comment.
When we expect a failed connection, we should also do a SQLError() call and test that we're getting the expected sqlstate (and possibly error message).
There was a problem hiding this comment.
Added SQLError() check
| connection_string = builder.withDSN(dsn).withServer(MYSQL_CLUSTER_URL).withAuthMode("SECRETS MANAGER").withAuthRegion("us-east-2").withSecretId("invalid-id").build(); | ||
| SQLCHAR conn_out[4096] = "\0"; | ||
| SQLSMALLINT len; | ||
| EXPECT_EQ(SQL_ERROR, SQLDriverConnect(dbc, nullptr, AS_SQLCHAR(connection_string.c_str()), SQL_NTS, conn_out, MAX_NAME_LEN, &len, SQL_DRIVER_NOPROMPT)); |
There was a problem hiding this comment.
Added SQLError() check
driver/secrets_manager_proxy.cc
Outdated
| this->secret_json_value = fetch_latest_credentials(); | ||
| fetched = true; | ||
| secrets_cache[this->secret_key] = this->secret_json_value; | ||
| if (fetch_latest_credentials()) { |
There was a problem hiding this comment.
Nit: this can be an else if.
justing-bq
approved these changes
Mar 31, 2023
yanw-bq
pushed a commit
that referenced
this pull request
Apr 28, 2023
justing-bq
pushed a commit
that referenced
this pull request
Apr 28, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
yanw-bq
pushed a commit
that referenced
this pull request
May 1, 2023
yanw-bq
added a commit
that referenced
this pull request
May 2, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
yanw-bq
added a commit
that referenced
this pull request
May 2, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
justing-bq
pushed a commit
that referenced
this pull request
May 4, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
justing-bq
pushed a commit
that referenced
this pull request
May 4, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
yanw-bq
added a commit
that referenced
this pull request
May 17, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
alexr-bq
added a commit
that referenced
this pull request
May 18, 2023
* Add AWS Authentication parameters to DSN UI (#115) * Refactor MYSQL_PROXY (#118) * EFM PROXY * complete EFM PROXY * fix unit tests * fix include * generate node keys after setting next proxy for EFM_PROXY * fix unit tests * stop monitoring init() and options() * replace MYSQL_MONITOR_PROXY with MYSQL_PROXY * fix unit tests * stop monitoring ping() * stop monitoring ssl_set and option4 * stop monitoring autocommit * comment out aws code * stop monitoring client_find_plugin * newline at eof * free monitor connection before getting a new one * fix unit tests * test * disable failover and monitoring for monitor thread * fix memory leak in unit test * needs connection handler for monitor * add null check in monitor service start monitoring * fix env handle still used after deleting * comment out mysql_library_init * move connection handler to its own header file * Further refactor MYSQL_PROXY (#119) * create DUMMY_PROXY to handle mysql library calls, refactor MYSQL_PROXY to only forward proxy call * remove commented include * include thread header in dummy proxy * rename mysql_proxy to connection_proxy * rename dummy proxy to mysql proxy * remove secrets manager proxy as it should be a different PR * include in alphabetical order * AWS SDK helper class (#123) * Implement IAM Authentication (#120) * Add Authentication Parameters to ConnectionStringBuilder (#124) * Secrets Manager Proxy (#121) * new secrets_manager files * working secrets manager client * add cache for secrets manager proxy * first unit test for secrets manager proxy * added more unit tests * fix unique lock missing template arguments * fix multiple calls to aws api in secrets manager unit tests * fix multiple aws init/shutdown api calls in integration tests * fix calling init/shutdownApi in windows * verbose log for windows community tests * wrap initapi and shutdownapi inside secrets manager proxy contructor/destructor * move init/shutdown aws api to alloc/free env handle * move init/shutdown aws api to alloc/free env handle * test * test * fix static variables * fix atomic int init * fix unit tests * renaming * cleanup * introduce aws sdk helpoer * address comments * nit * fix rebase * fix rebase * set error * fix build * better error message * Secrets manager integration test (#127) * create secrets in java util * simple sm test * simplify setup * fix integration * comment out toxiporxy * comment out toxiporxy * sm test not inherit from base anymore * fix build * fix set error message, fix dsn not picked up * add {} in connection string * fix curly braces * fix log * fix build * more logging * more logging * fix test * better error handling * uncomment * address comments * revert * IAM Authentication Integration Tests (#126) * Refactor Integration Tests (#128) * Fix installer (#129) * fix minor version exceeding 255, remove lib folder for installer * remove setup lib from installer * add aws sdk component in wix * build aws sdk on release action * workflow dispatch * bundle aws sdk when doing cpack on mac and linux * fix mac installer aws dependencies * Adjust DSN config UI for AWS Authentication (#130) * Documentation for AWS Authentication (#131) * rename windows installer (#132) * Update license (#133) * update license * ignore file change in license * update license in build script * Parse Region from Secret if User does not provide Region (#134) * Fix SQLCancel not going through proxy (#135) * fix SQLCancel not going through proxy * cleanup ds during dbc clone * spacing * Fix typo in doc * Implement ENABLE_STRICT_READER_FAILOVER user parameter (#136) * Override change_user() in IAM/Secrets Manager proxy (#137) * override change_user() in IAM/Secrets Manager proxy * better naming * Fix failover timeout not obeyed (#138) * replace async future with packed_task and thread to avoid blocking future destructor * fix unit tests * add template arguments for std::packaged_task * missing template arguments * use thread pool to run failover and wait for all when driver unloads, fix logger * increase thread pool size * fix reader failover where failed result overwrite successful result * not stop thread pool on free env handle * redirect maven central and stop thread pool in teardown * maven central * add thread id to logs * move future valid check before wait for * move future valid check before wait_for() for writer failover * mark logger as extern * test diable logs * test failover_integration_test first * test * debug gh action * make env var available for debug session * debug in docker * refactor integration tests to use shared pointer for rds client * make rds client back to non static * fix build * reset rds client in teardown * Revert "refactor integration tests to use shared pointer for rds client" This reverts commit 2ec1ae5. * disable remote debug * fix build * debug * specifically call thread pool stop in myodbc_end * move failover thread pool inside env * fix unit test * fix build * cleanup * China endpoint support (#140) * china endpoint support * nit * Verify Writer Cluster Connections (#139) * Fix shadowing member variables causing null pointer exception (#148) * fix some shadowing member variables casuing null pointer exception * extend perf tests aws credentials duration from 3 to 4 hours * workaround mysqlclient * fix zlib not found * fix mac build * bump mysql version * fix mac build * bump github action runner macos versiooon * update download link * fix build * install openssl1.1 for macos13 * debug * debug * fall back to macos 11 * change macos version to 12 * fix build * fix build * Fix monitor timeout (#149) * fix monitor timeout * fix login timeout and unit tests * pass monitor flag * rename a component that may have name collision issue on mac (#151) * Remove is_multi_writer flag and check for last updated writer * Fix tests * Remove uneeded line --------- Co-authored-by: justing-bq <62349012+justing-bq@users.noreply.github.com> Co-authored-by: Yan Wang <68562925+yanw-bq@users.noreply.github.com> Co-authored-by: justing-bq <justing@bitquilltech.com> Co-authored-by: Alex Rehnby-Martin <alexr@AlexRsMacbookPro.local>
yanw-bq
added a commit
that referenced
this pull request
May 18, 2023
* create secrets in java util
* simple sm test
* simplify setup
* fix integration
* comment out toxiporxy
* comment out toxiporxy
* sm test not inherit from base anymore
* fix build
* fix set error message, fix dsn not picked up
* add {} in connection string
* fix curly braces
* fix log
* fix build
* more logging
* more logging
* fix test
* better error handling
* uncomment
* address comments
* revert
yanw-bq
added a commit
that referenced
this pull request
May 18, 2023
* Add AWS Authentication parameters to DSN UI (#115) * Refactor MYSQL_PROXY (#118) * EFM PROXY * complete EFM PROXY * fix unit tests * fix include * generate node keys after setting next proxy for EFM_PROXY * fix unit tests * stop monitoring init() and options() * replace MYSQL_MONITOR_PROXY with MYSQL_PROXY * fix unit tests * stop monitoring ping() * stop monitoring ssl_set and option4 * stop monitoring autocommit * comment out aws code * stop monitoring client_find_plugin * newline at eof * free monitor connection before getting a new one * fix unit tests * test * disable failover and monitoring for monitor thread * fix memory leak in unit test * needs connection handler for monitor * add null check in monitor service start monitoring * fix env handle still used after deleting * comment out mysql_library_init * move connection handler to its own header file * Further refactor MYSQL_PROXY (#119) * create DUMMY_PROXY to handle mysql library calls, refactor MYSQL_PROXY to only forward proxy call * remove commented include * include thread header in dummy proxy * rename mysql_proxy to connection_proxy * rename dummy proxy to mysql proxy * remove secrets manager proxy as it should be a different PR * include in alphabetical order * AWS SDK helper class (#123) * Implement IAM Authentication (#120) * Add Authentication Parameters to ConnectionStringBuilder (#124) * Secrets Manager Proxy (#121) * new secrets_manager files * working secrets manager client * add cache for secrets manager proxy * first unit test for secrets manager proxy * added more unit tests * fix unique lock missing template arguments * fix multiple calls to aws api in secrets manager unit tests * fix multiple aws init/shutdown api calls in integration tests * fix calling init/shutdownApi in windows * verbose log for windows community tests * wrap initapi and shutdownapi inside secrets manager proxy contructor/destructor * move init/shutdown aws api to alloc/free env handle * move init/shutdown aws api to alloc/free env handle * test * test * fix static variables * fix atomic int init * fix unit tests * renaming * cleanup * introduce aws sdk helpoer * address comments * nit * fix rebase * fix rebase * set error * fix build * better error message * Secrets manager integration test (#127) * create secrets in java util * simple sm test * simplify setup * fix integration * comment out toxiporxy * comment out toxiporxy * sm test not inherit from base anymore * fix build * fix set error message, fix dsn not picked up * add {} in connection string * fix curly braces * fix log * fix build * more logging * more logging * fix test * better error handling * uncomment * address comments * revert * IAM Authentication Integration Tests (#126) * Refactor Integration Tests (#128) * Fix installer (#129) * fix minor version exceeding 255, remove lib folder for installer * remove setup lib from installer * add aws sdk component in wix * build aws sdk on release action * workflow dispatch * bundle aws sdk when doing cpack on mac and linux * fix mac installer aws dependencies * Adjust DSN config UI for AWS Authentication (#130) * Documentation for AWS Authentication (#131) * rename windows installer (#132) * Update license (#133) * update license * ignore file change in license * update license in build script * Parse Region from Secret if User does not provide Region (#134) * Fix SQLCancel not going through proxy (#135) * fix SQLCancel not going through proxy * cleanup ds during dbc clone * spacing * Fix typo in doc * Implement ENABLE_STRICT_READER_FAILOVER user parameter (#136) * Override change_user() in IAM/Secrets Manager proxy (#137) * override change_user() in IAM/Secrets Manager proxy * better naming * Fix failover timeout not obeyed (#138) * replace async future with packed_task and thread to avoid blocking future destructor * fix unit tests * add template arguments for std::packaged_task * missing template arguments * use thread pool to run failover and wait for all when driver unloads, fix logger * increase thread pool size * fix reader failover where failed result overwrite successful result * not stop thread pool on free env handle * redirect maven central and stop thread pool in teardown * maven central * add thread id to logs * move future valid check before wait for * move future valid check before wait_for() for writer failover * mark logger as extern * test diable logs * test failover_integration_test first * test * debug gh action * make env var available for debug session * debug in docker * refactor integration tests to use shared pointer for rds client * make rds client back to non static * fix build * reset rds client in teardown * Revert "refactor integration tests to use shared pointer for rds client" This reverts commit 2ec1ae5. * disable remote debug * fix build * debug * specifically call thread pool stop in myodbc_end * move failover thread pool inside env * fix unit test * fix build * cleanup * China endpoint support (#140) * china endpoint support * nit * Verify Writer Cluster Connections (#139) * Fix shadowing member variables causing null pointer exception (#148) * fix some shadowing member variables casuing null pointer exception * extend perf tests aws credentials duration from 3 to 4 hours * workaround mysqlclient * fix zlib not found * fix mac build * bump mysql version * fix mac build * bump github action runner macos versiooon * update download link * fix build * install openssl1.1 for macos13 * debug * debug * fall back to macos 11 * change macos version to 12 * fix build * fix build * Fix monitor timeout (#149) * fix monitor timeout * fix login timeout and unit tests * pass monitor flag * rename a component that may have name collision issue on mac (#151) * Remove is_multi_writer flag and check for last updated writer * Fix tests * Remove uneeded line --------- Co-authored-by: justing-bq <62349012+justing-bq@users.noreply.github.com> Co-authored-by: Yan Wang <68562925+yanw-bq@users.noreply.github.com> Co-authored-by: justing-bq <justing@bitquilltech.com> Co-authored-by: Alex Rehnby-Martin <alexr@AlexRsMacbookPro.local>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Review Status
Summary
Secrets Manager Integration Test
Description
SECRETS_MANAGER_PROXYAdditional Reviewers
@justing-bq @sergiyvamz