Skip to content

use rds proxy port instead of proxy target port#35347

Merged
GavinFrazar merged 2 commits intomasterfrom
gavinfrazar/fix-rds-proxy-discovery-port
Dec 5, 2023
Merged

use rds proxy port instead of proxy target port#35347
GavinFrazar merged 2 commits intomasterfrom
gavinfrazar/fix-rds-proxy-discovery-port

Conversation

@GavinFrazar
Copy link
Copy Markdown
Contributor

@GavinFrazar GavinFrazar commented Dec 5, 2023 •
edited
Loading

fixes #35268

I tested that discovery works correctly with this change for MySQL, Postgres, and MSSQL Server, and that connections work as well regardless of target database port.

changelog: IAM permissions for rds:DescribeDBProxyTargets are no longer required for RDS Proxy discovery.

@GavinFrazar GavinFrazar added aws Used for AWS Related Issues. database-access Database access related issues and PRs db/postgres PostgreSQL related database access issues db/mysql MySQL related database access issues db/sqlserver Microsoft SQL Server related database access issues backport/branch/v12 discovery labels Dec 5, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2023

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

smallinsky
smallinsky approved these changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky left a comment

Choose a reason for hiding this comment

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

Linter seems to complain about unused getRDSProxyTargetPort function Otherwise LGTM.

greedy52
greedy52 approved these changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
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.

If removing getRDSProxyTargetPort, we can also remove the corresponding IAM action from AWS configurator

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2023

The PR changelog entry failed validation: The changelog entry must start with a letter.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2023

The PR changelog entry failed validation: The changelog entry must start with a letter.

@GavinFrazar GavinFrazar enabled auto-merge December 5, 2023 20:06
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2023

🤖 Vercel preview here: https://docs-pu7etpl3q-goteleport.vercel.app/docs/ver/preview

@GavinFrazar GavinFrazar added this pull request to the merge queue Dec 5, 2023
Merged via the queue into master with commit 0562ce0 Dec 5, 2023
@GavinFrazar GavinFrazar deleted the gavinfrazar/fix-rds-proxy-discovery-port branch December 5, 2023 20:40
@public-teleport-github-review-bot
Copy link
Copy Markdown

public-teleport-github-review-bot Bot commented Dec 5, 2023

@GavinFrazar See the table below for backport results.

Branch Result
branch/v12 Failed
branch/v13 Create PR
branch/v14 Create PR

This was referenced Dec 5, 2023
Merged
Merged
GavinFrazar added a commit that referenced this pull request Dec 5, 2023
@GavinFrazar
[v12] use rds proxy port instead of proxy target port
68a64e5 
Backport #35347 to branch/v12.
@GavinFrazar GavinFrazar mentioned this pull request Dec 5, 2023
Merged
github-merge-queue Bot pushed a commit that referenced this pull request Dec 6, 2023
@GavinFrazar
[v12] use rds proxy port instead of proxy target port (#35391)
7a6330d 
* [v12] use rds proxy port instead of proxy target port

Backport #35347 to branch/v12.

* remove unused test mock func
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greedy52 greedy52 greedy52 approved these changes

@smallinsky smallinsky smallinsky approved these changes

+1 more reviewer

@AntonAM AntonAM AntonAM approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

aws Used for AWS Related Issues. database-access Database access related issues and PRs db/mysql MySQL related database access issues db/postgres PostgreSQL related database access issues db/sqlserver Microsoft SQL Server related database access issues discovery size/sm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RDS Proxy auto-discovery uses proxy target port

4 participants

@GavinFrazar @AntonAM @greedy52 @smallinsky