Skip to content

feat(mqconfig): support IAM role auth for SQS MQ provider#996

Open
ambroziepaval wants to merge 2 commits into
hookdeck:mainfrom
ambroziepaval:feat/sqs-iam-role-credentials
Open

feat(mqconfig): support IAM role auth for SQS MQ provider#996
ambroziepaval wants to merge 2 commits into
hookdeck:mainfrom
ambroziepaval:feat/sqs-iam-role-credentials

Conversation

@ambroziepaval

Copy link
Copy Markdown
Contributor

The AWS SQS message-queue provider previously required static IAM user keys (AWS_SQS_ACCESS_KEY_ID / AWS_SQS_SECRET_ACCESS_KEY) and always passed them explicitly to the SDK, which suppressed the default credential chain. This made role-based auth (ECS/EKS task role, EC2 instance profile) impossible.

When no static credentials are configured, ToCredentials now returns (nil, nil) and the client builders omit WithCredentialsProvider, letting the AWS SDK default credential chain resolve credentials. IsConfigured selects SQS on region alone so the keys can be omitted. Existing static-key configs are unchanged. Mirrors the GCP Pub/Sub provider's implicit-credential behavior.

Refs #463

The AWS SQS message-queue provider previously required static IAM user
keys (AWS_SQS_ACCESS_KEY_ID / AWS_SQS_SECRET_ACCESS_KEY) and always passed
them explicitly to the SDK, which suppressed the default credential chain.
This made role-based auth (ECS/EKS task role, EC2 instance profile)
impossible.

When no static credentials are configured, ToCredentials now returns
(nil, nil) and the client builders omit WithCredentialsProvider, letting
the AWS SDK default credential chain resolve credentials. IsConfigured
selects SQS on region alone so the keys can be omitted. Existing static-key
configs are unchanged. Mirrors the GCP Pub/Sub provider's implicit-credential
behavior.

Refs hookdeck#463

Generated with AI

Co-Authored-By: Claude <noreply@anthropic.com>

@alexluong alexluong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Overall LGTM with a small comment.

There's also publishmq which has a different AWS SDK path. I think it's probably relevant and something we can apply similar logic. Not a blocker but can you include that in this PR as well?

Comment on lines +63 to 67
// IsConfigured selects SQS on Region alone; keys are optional so IAM-role auth
// can use the AWS SDK default credential chain.
func (c *AWSSQSConfig) IsConfigured() bool {
return c.AccessKeyID != "" && c.SecretAccessKey != "" && c.Region != ""
return c.Region != ""
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While this is correct, I discovered a config data issue where if only 1 of AWS_SQS_ACCESS_KEY_ID or AWS_SQS_SECRET_ACCESS_KEY is provided, it is accepted and led to a runtime auth error at a later stage even though this is invalid.

I don't think IsConfigured() is the right place but maybe we can apply this validation check at ToQueueConfig()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @alexluong! addressed both points:

  • added the partial-credential guard in ToQueueConfig() as suggested so it fails at startup via validateMQs instead of deferring to a runtime error
  • extended IAM-role support to publishmq. The gap was in the config layer, since it already shared the IAM-ready SDK path. Relaxed the key requirementss, make selection region-only and applied the same partial-credentials check

…lishmq

Address PR review feedback on hookdeck#996:

- ToQueueConfig now rejects a config with exactly one of AWS_SQS_ACCESS_KEY_ID
  / AWS_SQS_SECRET_ACCESS_KEY set. Both must be provided together (static
  credentials) or both omitted (AWS SDK default credential chain). Previously a
  partial config was accepted and failed with a deferred runtime auth error.
  The check surfaces at startup via the existing validateMQs flow.

- Extend the same IAM-role support to the publish MQ SQS provider
  (PublishAWSSQSConfig): keys are now optional, hasPublishAWSSQSConfig selects
  on Region alone, and the partial-credential rule is enforced via a new
  PublishMQConfig.Validate wired into config validation (validatePublishMQ).

Generated with AI

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants