feat(knowledge-base): enable structured data store knowledge-base support #970
Conversation
erdemayyildiz
requested review from
krokoko,
MichaelWalker-git,
dineshSajwan,
alexa-perlov and
aws-rafams
| * If redshiftQueryEngineType is not of type `PROVISIONED`, | ||
| * do not include this property as it will throw error. | ||
| */ | ||
| readonly redshiftProvisionedConfiguration?: bedrock.CfnKnowledgeBase.RedshiftProvisionedConfigurationProperty; |
There was a problem hiding this comment.
L2s should avoid the usage of L1 Cfn. Maybe we could for instance create an L2 Configuration Property that takes as input an ISecret for the username_password option?
| /** | ||
| * The query generation configuration. | ||
| */ | ||
| readonly queryGenerationConfiguration?: bedrock.CfnKnowledgeBase.QueryGenerationConfigurationProperty | undefined; |
There was a problem hiding this comment.
Sam thing here, maybe we can create a new interface that takes for instance a CDK.Duration for the executionTimeout?
| * Validate that the Sql Knowledge configuration set correctly. | ||
| * It prevents the wrong use of configurations of query engine and storage types together. | ||
| */ | ||
| function validateConfigs(props: SqlKnowledgeBaseProps) { |
There was a problem hiding this comment.
Would be a good idea to validate executionTimeoutSeconds as well as
| * do not include this property as it will throw error. | ||
| */ | ||
| readonly redshiftQueryEngineAwsDataCatalogStorageConfiguration?: bedrock.CfnKnowledgeBase | ||
| .RedshiftQueryEngineAwsDataCatalogStorageConfigurationProperty; |
There was a problem hiding this comment.
Same as before, maybe we can create an L2 such as QueryEngineStorage.fromAwsDataCatalog(tableNames) and QueryEngineStorage.fromRedshift(databaseName) using static methods. Some inspiration can be taken from here.
| actions: [ | ||
| 'redshift-data:*', | ||
| 'sqlworkbench:*', | ||
| 'secretsmanager:GetSecretValue', |
There was a problem hiding this comment.
Would be good to limit as much as possible the permissions granted to the executionRole. For instance, by using the console the policy generated is this one (when using a serverless Redshift engine).
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "RedshiftDataAPIStatementPermissions",
"Effect": "Allow",
"Action": [
"redshift-data:GetStatementResult",
"redshift-data:DescribeStatement",
"redshift-data:CancelStatement"
],
"Resource": [
"*"
],
"Condition": {
"StringEquals": {
"redshift-data:statement-owner-iam-userid": "${aws:userid}"
}
}
},
{
"Sid": "RedshiftDataAPIExecutePermissions",
"Effect": "Allow",
"Action": [
"redshift-data:ExecuteStatement"
],
"Resource": [
"arn:aws:redshift-serverless:eu-central-1:ACCT_NUMBER:workgroup/bb1e6fbc-2311-4ae1-8b7a-2ada8fa2433f"
]
},
{
"Sid": "RedshiftServerlessGetCredentials",
"Effect": "Allow",
"Action": "redshift-serverless:GetCredentials",
"Resource": [
"arn:aws:redshift-serverless:eu-central-1:ACCT_NUMBER:workgroup/bb1e6fbc-2311-4ae1-8b7a-2ada8fa2433f"
]
},
{
"Sid": "SqlWorkbenchAccess",
"Effect": "Allow",
"Action": [
"sqlworkbench:GetSqlRecommendations",
"sqlworkbench:PutSqlGenerationContext",
"sqlworkbench:GetSqlGenerationContext",
"sqlworkbench:DeleteSqlGenerationContext"
],
"Resource": "*"
},
{
"Sid": "KbAccess",
"Effect": "Allow",
"Action": [
"bedrock:GenerateQuery"
],
"Resource": "*"
}
]
}
Fixes #843
Developed a new construct that creates structured Amazon Bedrock Knowledgebase by extending KnowledgeBaseBase, and using Redshift as data sources.
Note:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.