Skip to content

feat(aws_s3 source): separate s3 and sqs auth #23079

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sonnens
Copy link
Contributor

@sonnens sonnens commented May 20, 2025

Summary

add an s3_auth config option, using different credentials for polling SQS and fetching files from S3

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

How did you test this PR?

a different role authorized to poll SQS and S3

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the "no-changelog" label to this PR.

Notes

  • Please read our Vector contributor resources.
  • Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
  • The CI checks run only after we manually approve them.
    • We recommend adding a pre-push hook, please see this template.
    • Alternatively, we recommend running the following locally before pushing to the remote branch:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)
      • ./scripts/check_changelog_fragments.sh
  • After a review is requested, please avoid force pushes to help us review incrementally.
    • Feel free to push as many commits as you want. They will be squashed into one before merging.
    • For example, you can run git merge origin master and git push.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run cargo vdev build licenses to regenerate the license inventory and commit the changes (if any). More details here.

References

@sonnens sonnens requested a review from a team as a code owner May 20, 2025 13:11
@github-actions github-actions bot added the domain: sources Anything related to the Vector's sources label May 20, 2025
@milas
Copy link

milas commented Jun 4, 2025

Had this same use case come up and found this PR, so cherry-picked it - thanks 👍🏻

fwiw I do think that having auth remain as the S3 auth and making sqs.auth the "override" would be more consistent, as the other SQS config is already in under the sqs key: https://vector.dev/docs/reference/configuration/sources/aws_s3/#sqs

@sonnens
Copy link
Contributor Author

sonnens commented Jun 5, 2025

Had this same use case come up and found this PR, so cherry-picked it - thanks 👍🏻

fwiw I do think that having auth remain as the S3 auth and making sqs.auth the "override" would be more consistent, as the other SQS config is already in under the sqs key: https://vector.dev/docs/reference/configuration/sources/aws_s3/#sqs

on precedence:

sqs: auth: ... and no auth clause on the outer config I think is pretty intuitive, "assume this role for the SQS step, use default instance / env creds for the S3 step"

The opposite of that, "assume a role for the S3 step, not the SQS step" would, I guess need an auth: default option and, if not present, use the outer auth config?

more concretely,

Both the SQS & S3 calls assume the role

aws_s3:
  type: aws_s3
  auth:
    assume_role: <whatever>
  sqs:
   queue_url: ...

Only the SQS call assumes the role , the S3 call uses the default system credentials:

aws_s3:
  type: aws_s3
  sqs:
   queue_url: ...
   auth:
    assume_role: <whatever>
  ...

Only the S3 call assumes the role , the SQS call uses the default system credentials:

aws_s3:
  type: aws_s3
  auth:
    assume_role: <whatever>
  sqs:
   queue_url: ...
   auth: default
  ...

there's one use case that I can't think of a sensible way to articulate and that's AssumeRole chaining , "first assume role X, then as role X, assume role Y to do the other step" so probably just ... don't do that ? or launch vector as the role you want it to be in the first place

@thomasqueirozb thomasqueirozb added the meta: awaiting author Pull requests that are awaiting their author. label Jun 26, 2025
@github-actions github-actions bot added domain: sinks Anything related to the Vector's sinks and removed meta: awaiting author Pull requests that are awaiting their author. labels Jun 30, 2025
@sonnens
Copy link
Contributor Author

sonnens commented Jun 30, 2025

@thomasqueirozb & @pront

took the suggestions, now SQS's auth section set to a string literal falls back to the system / AWS SDK strategy and there's no s3_auth top level section. Probably makes more sense

@sonnens sonnens requested review from thomasqueirozb and pront June 30, 2025 21:06
@@ -203,6 +203,8 @@ pub enum AwsAuthentication {
#[configurable(metadata(docs::examples = "us-west-2"))]
region: Option<String>,
},
/// Explicitly default authentication, using the default credentials chain defined by the AWS SDK
Fallback(String),
Copy link
Member

Choose a reason for hiding this comment

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

Is the string used anywhere? Seems like we can just use Default.

Comment on lines +1 to +2
added `sqs.auth` option to specify AWS auth configuration for S3 when different than SQS, and added
a string literal for fallback to default
Copy link
Member

Choose a reason for hiding this comment

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

Rename file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: sinks Anything related to the Vector's sinks domain: sources Anything related to the Vector's sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate AWS auth for SQS & S3 in the aws_s3 source
4 participants