Skip to content

check for empty name part in role arn#26315

Merged
GavinFrazar merged 1 commit intomasterfrom
gavinfrazar/update-aws-role-arn-check
May 16, 2023
Merged

check for empty name part in role arn#26315
GavinFrazar merged 1 commit intomasterfrom
gavinfrazar/update-aws-role-arn-check

Conversation

@GavinFrazar
Copy link
Copy Markdown
Contributor

This PR fixes a minor issue where the role arn checking doesn't check if the role name is "" (empty).

We were checking if the role arn resource had at least 2 parts separated by "/", but we didn't check if the last part is empty.
strings.Split("role/", "/")) == []string{"role", ""}

This was a minor inconvenience I ran into recently. Instead of catching my typo early, the db service was trying to assume the invalid role and returning a less helpful error message from the failed assume role call.

@GavinFrazar GavinFrazar added aws Used for AWS Related Issues. database-access Database access related issues and PRs backport/branch/v12 labels May 16, 2023
@GavinFrazar GavinFrazar requested review from greedy52 and removed request for ibeckermayer and rosstimothy May 16, 2023 00:28
@GavinFrazar GavinFrazar requested a review from smallinsky May 16, 2023 00:28
@GavinFrazar
Copy link
Copy Markdown
Contributor Author

huh, I clicked on a suggested reviewer at the same time as the bot added reviewers, and it removed them 🤷‍♂️ oh well

@GavinFrazar GavinFrazar added this pull request to the merge queue May 16, 2023
Merged via the queue into master with commit 0edc5c8 May 16, 2023
@GavinFrazar GavinFrazar deleted the gavinfrazar/update-aws-role-arn-check branch May 16, 2023 17:28
@public-teleport-github-review-bot
Copy link
Copy Markdown

@GavinFrazar See the table below for backport results.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws Used for AWS Related Issues. database-access Database access related issues and PRs size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants