Add support for role chain assumption - #132
Conversation
|
@moserke thanks for your contribution! |
03cf621 to
bfdd035
Compare
|
Sure thing, sorry about that. Signed now! |
Signed-off-by: Moser, Kevin <kevin.moser@horizon3.ai>
Signed-off-by: Moser, Kevin <kevin.moser@horizon3.ai>
|
aha. If only I read the error message closer.... DCO is passing now |
There was a problem hiding this comment.
Pull request overview
Adds role chaining support for AWS MSK IAM authentication so provider-kafka can use ambient AWS credentials (e.g., IRSA/PodIdentity) and optionally assume a configured role before authenticating to MSK, addressing cross-account MSK access (Fixes #131).
Changes:
- Extended Kafka SASL config to accept an optional
roleArn. - Updated AWS MSK IAM auth to assume the configured role via STS before retrieving credentials.
- Promoted required AWS SDK v2 modules to direct dependencies in
go.mod.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/clients/kafka/config.go | Adds sasl.roleArn to the JSON config schema. |
| internal/clients/kafka/client.go | Wraps AWS MSK IAM auth to optionally perform STS AssumeRole (role chaining). |
| go.mod | Adds direct dependencies needed for STS assume-role credentials caching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if roleArn != "" { | ||
| stsClient := sts.NewFromConfig(s) | ||
| provider := stscreds.NewAssumeRoleProvider(stsClient, roleArn, func(o *stscreds.AssumeRoleOptions) { | ||
| o.RoleSessionName = "crossplane-provider-kafka" | ||
| }) | ||
| s.Credentials = aws.NewCredentialsCache(provider) | ||
| } |
There was a problem hiding this comment.
The new role-chaining branch (roleArn -> STS AssumeRole) is not covered by tests. Since this package already has unit tests, please add coverage for the new behavior (e.g., refactor authenticateAwsIam to allow injecting/mocking the STS client/credential provider and assert that credentials retrieval uses AssumeRole when roleArn is set).
| type SASL struct { | ||
| Mechanism string `json:"mechanism"` | ||
| RoleArn string `json:"roleArn"` | ||
| Username string `json:"username"` | ||
| Password string `json:"password"` //nolint:gosec |
There was a problem hiding this comment.
RoleArn extends the expected JSON schema for the provider secret, but the repository documentation/examples that point to internal/clients/kafka/config.go will become outdated unless they mention the new sasl.roleArn field and how it is used with aws-msk-iam. Please update the user-facing docs/examples accordingly so users can discover and correctly configure role chaining.
| github.com/aws/aws-sdk-go-v2 v1.41.4 | ||
| github.com/aws/aws-sdk-go-v2/config v1.32.12 | ||
| github.com/aws/aws-sdk-go-v2/credentials v1.19.12 | ||
| github.com/aws/aws-sdk-go-v2/service/sts v1.41.9 |
There was a problem hiding this comment.
| github.com/aws/aws-sdk-go-v2 v1.41.4 | |
| github.com/aws/aws-sdk-go-v2/config v1.32.12 | |
| github.com/aws/aws-sdk-go-v2/credentials v1.19.12 | |
| github.com/aws/aws-sdk-go-v2/service/sts v1.41.9 | |
| github.com/aws/aws-sdk-go-v2 v1.41.5 | |
| github.com/aws/aws-sdk-go-v2/config v1.32.14 | |
| github.com/aws/aws-sdk-go-v2/credentials v1.19.14 | |
| github.com/aws/aws-sdk-go-v2/service/sts v1.41.10 |
and tidy
|
@moserke could you please resolve the conflicts? After that I feel we're good to merge and release v1.2 |
Signed-off-by: Moser, Kevin <kevin.moser@horizon3.ai>
|
@fernandezcuesta resolved the conflicts and did a go mod tidy. |
|
Thanks a lot @moserke for your contribution, all good and merging! |
Description of your changes
Fixes #131
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
ran the custom image in a local crossplane cluster