-
Notifications
You must be signed in to change notification settings - Fork 66
feat: Implement multiple AWS authentication methods #184
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
base: main
Are you sure you want to change the base?
Conversation
I've implemented three authentication methods for AWS integration: 1. Credentials: Using Access Key ID and Secret Access Key (existing method). 2. CloudFormation IAM Role: A new "one-click" experience using a provided CloudFormation template (`apps/dbagent/public/xata-agent-iam-role.yaml`) that creates an IAM role. You provide the Role ARN to me. 3. EC2 Instance IAM Role: Automatic authentication if I'm running on an EC2 instance with an appropriate IAM role attached. Key changes include: - Updated `AwsIntegration` type in `integrations.ts` to support discriminated unions for different auth methods and their specific data (e.g., `cloudformationStackArn`). - Modified AWS SDK client initialization in `rds.ts` to handle role assumption (for CloudFormation) and EC2 instance metadata credentials. - Added EC2 instance/role detection logic (`isEc2InstanceWithRole` in `rds.ts` and `checkEc2InstanceRoleStatus` action). - Updated the AWS integration UI (`aws-integration.tsx`) to allow you to select the authentication method and provide necessary inputs. - Created the `xata-agent-iam-role.yaml` CloudFormation template. - Updated backend actions in `actions.ts` to work with the new `AwsIntegration` type. - Added comprehensive unit tests for the new logic. - Prepared documentation content (`Xata-Agent-AWS-integration-guide.md`) for the GitHub wiki, detailing the new methods. Known Issue: - The "Launch Stack" button in the AWS integration UI currently points to a placeholder URL (`https://example.com/cloudformation-template.yaml`) instead of the correct `/xata-agent-iam-role.yaml`. This is due to a persistent issue I encountered that prevented me from updating the `aws-integration.tsx` file's relevant section. You should refer to the documentation for the correct link to the CloudFormation template (raw GitHub URL: https://raw.githubusercontent.com/xataio/agent/main/apps/dbagent/public/xata-agent-iam-role.yaml) until this UI bug can be addressed. Output:
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR extends AWS integration to support three authentication methods—static credentials, CloudFormation-based role assumption, and EC2 instance profiles—by updating types, client initialization logic, UI actions, and documentation.
- Introduce a discriminated union
AwsIntegration
type covering credentials, CloudFormation, and EC2 instance methods - Refactor
initializeRDSClient
andinitializeCloudWatchClient
to handle role assumption and EC2 metadata - Add EC2 instance/role detection and update backend actions and tests to use the new integration type
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
package.json | Add AWS STS SDK dependency |
apps/dbagent/src/lib/db/integrations.ts | Define AwsIntegration union types |
apps/dbagent/src/lib/aws/rds.ts | Refactor client init to support multiple methods |
apps/dbagent/src/lib/aws/rds.test.ts | Add unit tests for new auth logic and EC2 check |
apps/dbagent/src/components/aws-integration/actions.ts | Update action signatures to accept AwsIntegration |
apps/dbagent/src/components/aws-integration/actions.test.ts | Update tests for new integration signatures |
apps/dbagent/public/xata-agent-iam-role.yaml | Add CloudFormation template |
Xata-Agent-AWS-integration-guide.md | Document the three authentication methods |
Comments suppressed due to low confidence (4)
apps/dbagent/src/lib/db/integrations.ts:22
- [nitpick] The property
cloudformationStackArn
is used as the IAM Role ARN when calling STS. Consider renaming it toroleArn
oriamRoleArn
for clarity.
cloudformationStackArn: string;
apps/dbagent/src/components/aws-integration/actions.ts:4
- There are duplicate imports of
getRDSClusterInfo
,getRDSInstanceInfo
, andinitializeRDSClient
. Remove the repeated entries to clean up the import block.
import { getRDSClusterInfo, getRDSInstanceInfo, initializeRDSClient, getRDSClusterInfo, getRDSInstanceInfo, initializeRDSClient, isEc2InstanceWithRole,
apps/dbagent/src/lib/aws/rds.ts:90
- Missing import for
CloudWatchClient
andGetMetricStatisticsCommand
. Addimport { CloudWatchClient, GetMetricStatisticsCommand } from '@aws-sdk/client-cloudwatch';
at the top to ensure the CloudWatch client is available.
export async function initializeCloudWatchClient(integration: AwsIntegration): Promise<CloudWatchClient> {
package.json:64
- Since the code now uses
CloudWatchClient
, consider adding"@aws-sdk/client-cloudwatch"
todependencies
to avoid runtime missing module errors.
"@aws-sdk/client-sts": "^3.812.0"
Signed-off-by: Alexis Rico <[email protected]>
Signed-off-by: Alexis Rico <[email protected]>
Signed-off-by: Alexis Rico <[email protected]>
Signed-off-by: Alexis Rico <[email protected]>
Signed-off-by: Alexis Rico <[email protected]>
Fix #124