Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Messenger] Add sessionToken option to SQS transport#45064

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@filkaris
Copy link
Contributor

@filkarisfilkaris commentedJan 18, 2022
edited
Loading

QA
Branch?6.1
Bug fix?no
New feature?yes
Deprecations?no
Tickets
LicenseMIT
Doc PRsymfony/symfony-docs#16411

When trying to authenticate to use an SQS transport, AWS Credentials have 3 "keys"

  • aws_access_key_id
  • aws_secret_access_key
  • aws_session_token

The last one,aws_session_token is only required for temporary credentials. In those cases though, it must be passed as well otherwise AWS returns a 403 Access Denied.

The async-aws library supports this in its configurationhttps://async-aws.com/configuration.html#sessiontoken

This MR essentially makes sure the user can pass this parameter inside yaml options

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@filkarisfilkaris marked this pull request as ready for reviewJanuary 18, 2022 19:32
@carsonbotcarsonbot added this to the6.1 milestoneJan 18, 2022
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

with minor comment

filkaris reacted with thumbs up emoji
Copy link
Member

@jderussejderusse left a comment

Choose a reason for hiding this comment

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

session_token is ephemeral. Having an option for this sounds weird to me.
We already accepted a similar PR for SES (#42982 (review))

@chalasr
Copy link
Member

Having an option for this sounds weird to me.

@jderusse given you approved, did you mean "good"?

@filkaris
Copy link
ContributorAuthor

session_token is ephemeral. Having an option for this sounds weird to me. We already accepted a similar PR for SES (#42982 (review))

I saw what you wrote in the other PR, and it's true that the sdk can use environmental variables instead, and have no configuration in options

However I've had cases with multiple credentials where I wanted to control to which account I wanted to connect

Environmental variables have fixed names and can only keep one set of credentials, but if you use symfony config you can specify custom variable names

eg.

            primary_transport:                dsn: "%env(PRIMARY_SQS_URL)%"                options:                    region: eu-west-1                    access_key: "%env(PRIMARY_AWS_ACCESS_KEY_ID)%"                    secret_key: "%env(PRIMARY_AWS_SECRET_ACCESS_KEY)%"                    session_token: "%env(PRIMARY_AWS_SESSION_TOKEN)%"            transport_in_another_account:                dsn: "%env(SECONDARY_SQS_URL)%"                options:                    region: eu-west-1                    access_key: "%env(SECONDARY_AWS_ACCESS_KEY_ID)%"                    secret_key: "%env(SECONDARY_AWS_SECRET_ACCESS_KEY)%"                    session_token: "%env(SECONDARY_AWS_SESSION_TOKEN)%"

@jderusse
Copy link
Member

jderusse commentedJan 19, 2022
edited
Loading

@filkaris that's a very good use-case. Thank you for the snippet.

👍 for my side

@fabpotfabpotforce-pushed themessenger-add-session-token-option-to-sqs-transport branch froma3ae1b2 tod3bce0eCompareJanuary 19, 2022 16:08
@fabpot
Copy link
Member

Thank you@filkaris.

@fabpotfabpot merged commit1027fad intosymfony:6.1Jan 19, 2022
@filkarisfilkaris deleted the messenger-add-session-token-option-to-sqs-transport branchJanuary 19, 2022 16:12
@fabpotfabpot mentioned this pull requestApr 15, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jderussejderussejderusse approved these changes

@chalasrchalasrchalasr approved these changes

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

5 participants

@filkaris@carsonbot@chalasr@jderusse@fabpot

[8]ページ先頭

©2009-2025 Movatter.jp