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

fix: do not call metadata server if security creds and region are retrievable through environment vars #1493

Merged
merged 9 commits into from
Nov 29, 2022
Merged

fix: do not call metadata server if security creds and region are retrievable through environment vars #1493

merged 9 commits into from
Nov 29, 2022

Conversation

lsirac
Copy link
Contributor

@lsirac lsirac commented Nov 23, 2022

Fixes an issue introduced by the AWS IMDSv2 changes where the metadata server is being called even if everything is retrievable through the defined environment variables.

@lsirac lsirac requested review from a team as code owners November 23, 2022 18:52
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Nov 23, 2022
aeitzman
aeitzman approved these changes Nov 23, 2022
View reviewed changes
src/auth/awsclient.ts Outdated Show resolved Hide resolved
src/auth/awsclient.ts Outdated Show resolved Hide resolved
danielbankhead
danielbankhead suggested changes Nov 23, 2022
View reviewed changes
Copy link
Contributor

@danielbankhead danielbankhead left a comment

Choose a reason for hiding this comment

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

Direction and implementation looks good, a requesting a few refactors for description and method reusability.

src/auth/awsclient.ts Show resolved Hide resolved
src/auth/awsclient.ts Outdated Show resolved Hide resolved
src/auth/awsclient.ts Outdated Show resolved Hide resolved
src/auth/awsclient.ts Show resolved Hide resolved
@aeitzman aeitzman added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 23, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 23, 2022
danielbankhead
danielbankhead suggested changes Nov 23, 2022
View reviewed changes
Copy link
Contributor

@danielbankhead danielbankhead left a comment

Choose a reason for hiding this comment

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

Super-clean 🧼 2 things

src/auth/awsclient.ts Outdated Show resolved Hide resolved
src/auth/awsclient.ts Outdated Show resolved Hide resolved
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Nov 23, 2022
danielbankhead
danielbankhead approved these changes Nov 24, 2022
View reviewed changes
Copy link
Contributor

@danielbankhead danielbankhead left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm

@danielbankhead danielbankhead self-assigned this Nov 24, 2022
@danielbankhead danielbankhead added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 24, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 24, 2022
@lsirac lsirac added the owlbot:ignore instruct owl-bot to ignore a PR label Nov 24, 2022
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • .github/workflows/ci.yaml - .github/workflows/ci.yaml (GitHub Actions) should be updated in synthtool

@danielbankhead danielbankhead added owlbot:run Add this label to trigger the Owlbot post processor. and removed owlbot:ignore instruct owl-bot to ignore a PR labels Nov 29, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2022
@danielbankhead danielbankhead merged commit d4de941 into googleapis:main Nov 29, 2022
@release-please release-please bot mentioned this pull request Nov 29, 2022
Merged
@nc-minh nc-minh mentioned this pull request Aug 15, 2024
Open
This was referenced Sep 10, 2024
Open
Open
This was referenced Sep 21, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aeitzman aeitzman aeitzman approved these changes

@danielbankhead danielbankhead danielbankhead approved these changes

Assignees

@danielbankhead danielbankhead

Labels
size: l Pull request size is large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lsirac @danielbankhead @aeitzman