-
Notifications
You must be signed in to change notification settings - Fork 20
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
✨ #131: Support AWS Bedrock #141
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #141 +/- ##
===========================================
- Coverage 71.31% 70.56% -0.76%
===========================================
Files 34 37 +3
Lines 1684 1821 +137
===========================================
+ Hits 1201 1285 +84
- Misses 418 469 +51
- Partials 65 67 +2 ☔ View full report in Codecov by Sentry. |
pkg/providers/bedrock/chat.go
Outdated
} | ||
|
||
cfg, _ := config.LoadDefaultConfig(ctx, | ||
config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider(c.config.AccessKey, c.config.SecretKey, "")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AWS is full of magic ways to authenticate 😌 I just want to make sure this override won't prevent it from using other authentication approaches like one based on ARN annotation added on the k8s service account resource level (https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html). This is likely to be a production way to give Glide access to Titan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I implement this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkrueger12 normally AWS SDK comes with support for all of these. But I would double check this override doen't break these other authentication approaches.
Because the problem with these Access/Secret keys is that it gets expired in 6-12h, so it cannot be really used for setups other than local testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do access keys expire that fast if they are attached to an IAM role? I am showing keys that are still active after 3 days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roma-glushko So I just tested this and the Access & secret keys are still live. We can go a different auth route but this way does work. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good to know. Maybe that's how people operate if they want to use Bedrock while their infra is outside of AWS (e.g. Azure or GCP).
I will try to inspect a pod that has ARN configured to its service account to see what AWS mounts there additionally to make sure you are with that usecase.
Need to create IAM role. https://github.com/aws-samples/bedrock-api-postman