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.

Sign up for GitHub

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

Jump to bottom

add RDS MariaDB e2e tests #40066

Merged
merged 7 commits into from
Apr 8, 2024
Merged

add RDS MariaDB e2e tests #40066

merged 7 commits into from
Apr 8, 2024

Conversation

GavinFrazar
Copy link
Contributor

@GavinFrazar GavinFrazar commented Mar 30, 2024 •
edited
Loading

This PR adds RDS MariaDB discovery, connection, and auto user provisioning tests.

stacked on #40065 and will be automatically rebased onto master when that merges.

We only added auto user provisioning in v14 for mariadb, but I'm backporting to v13 for discovery and connection tests. we only added e2e test in v14. Soon to be out of support anyway. I'll only backport to v14+.

@GavinFrazar GavinFrazar added database-access Database access related issues and PRs backport/branch/v13 backport/branch/v14 no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 labels Mar 30, 2024
@github-actions github-actions bot requested review from mdwn and probakowski March 30, 2024 02:34
greedy52
greedy52 reviewed Apr 1, 2024
View reviewed changes
Copy link
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Same comment as the other PR that we need to verify db_roles are assigned to the user.

e2e/aws/rds_test.go Outdated Show resolved Hide resolved
Base automatically changed from gavinfrazar/update-rds-e2e-tests to master April 2, 2024 18:14
@GavinFrazar
add AWS RDS MariaDB e2e tests
4aafda0
@GavinFrazar
fix spurious teardown error logs
dbe00fc 
* if a role or user doesn't exist, that is not an error nor is it worth
  logging during auto user teardown.
@GavinFrazar GavinFrazar force-pushed the gavinfrazar/e2e-test-rds-mariadb branch from fd02d99 to dbe00fc Compare April 4, 2024 01:22
@GavinFrazar
Copy link
Contributor Author

GavinFrazar commented Apr 4, 2024 •
edited
Loading

@greedy52 let me know if there's any issues with doing dbe00fc

I also updated the mariadb tests to check all role assignments work

@GavinFrazar
Copy link
Contributor Author

I can't figure out why the gha runners are being blocked by the MariaDB instance.

performance_schema is disabled by default, so it wouldn't even be possible to look at that.

I figured out that a host only gets blocked for connection failure, not auth failure. This can be simulated by just running nc -q 0 -v <host> <port> - the protocol handshake is server initiated and since we start talking to the db and then leave, it counts as a connection failure.

Default max_connect_errors is 100. I'm going to bump that value up a lot and then just wait for the db to get recreated over the weekend.

@GavinFrazar GavinFrazar mentioned this pull request Apr 5, 2024
Merged
@greedy52
Copy link
Contributor

greedy52 commented Apr 5, 2024

I can't figure out why the gha runners are being blocked by the MariaDB instance.

performance_schema is disabled by default, so it wouldn't even be possible to look at that.

I figured out that a host only gets blocked for connection failure, not auth failure. This can be simulated by just running nc -q 0 -v <host> <port> - the protocol handshake is server initiated and since we start talking to the db and then leave, it counts as a connection failure.

Default max_connect_errors is 100. I'm going to bump that value up a lot and then just wait for the db to get recreated over the weekend.

Sounds good. Hopefully we will see some data once performance_schema is enabled.

camscale
camscale approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@camscale camscale left a comment

Choose a reason for hiding this comment

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

bot

@GavinFrazar GavinFrazar added this pull request to the merge queue Apr 8, 2024
Merged via the queue into master with commit 54d2e90 Apr 8, 2024
38 checks passed
@GavinFrazar GavinFrazar deleted the gavinfrazar/e2e-test-rds-mariadb branch April 8, 2024 22:03
@public-teleport-github-review-bot
Copy link

@GavinFrazar See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed

GavinFrazar added a commit that referenced this pull request Apr 11, 2024
@GavinFrazar
add RDS MariaDB e2e tests (#40066)
9ecc7cf 
* add AWS RDS MariaDB e2e tests

* fix spurious teardown error logs

* if a role or user doesn't exist, that is not an error nor is it worth
  logging during auto user teardown.

* check for admin option

* add some extra secret fetching checkings

* only drop redshift user if the user exists
GavinFrazar added a commit that referenced this pull request Apr 11, 2024
@GavinFrazar
add RDS MariaDB e2e tests (#40066)
ac8f4f2 
* add AWS RDS MariaDB e2e tests

* fix spurious teardown error logs

* if a role or user doesn't exist, that is not an error nor is it worth
  logging during auto user teardown.

* check for admin option

* add some extra secret fetching checkings

* only drop redshift user if the user exists
This was referenced Apr 11, 2024
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Apr 18, 2024
@GavinFrazar
add RDS MariaDB e2e tests (#40066) (#40511)
0fcbcf9 
* add AWS RDS MariaDB e2e tests

* fix spurious teardown error logs

* if a role or user doesn't exist, that is not an error nor is it worth
  logging during auto user teardown.

* check for admin option

* add some extra secret fetching checkings

* only drop redshift user if the user exists
github-merge-queue bot pushed a commit that referenced this pull request Apr 18, 2024
@GavinFrazar
add RDS MariaDB e2e tests (#40066) (#40510)
61d74df 
* add AWS RDS MariaDB e2e tests

* fix spurious teardown error logs

* if a role or user doesn't exist, that is not an error nor is it worth
  logging during auto user teardown.

* check for admin option

* add some extra secret fetching checkings

* only drop redshift user if the user exists
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camscale camscale camscale approved these changes

@greedy52 greedy52 greedy52 approved these changes

@probakowski probakowski probakowski approved these changes

@klizhentas klizhentas Awaiting requested review from klizhentas klizhentas is a code owner

@russjones russjones Awaiting requested review from russjones russjones is a code owner

@r0mant r0mant Awaiting requested review from r0mant r0mant is a code owner

@zmb3 zmb3 Awaiting requested review from zmb3 zmb3 is a code owner

@fheinecke fheinecke Awaiting requested review from fheinecke fheinecke is a code owner

@tcsc tcsc Awaiting requested review from tcsc tcsc is a code owner

Assignees
No one assigned
Labels
backport/branch/v14 backport/branch/v15 database-access Database access related issues and PRs no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GavinFrazar @greedy52 @probakowski @camscale