|
| 1 | +# Pull Request: Add Conditional S3 Vectors KMS Policy and Fix Permission Issues |
| 2 | + |
| 3 | +## 🎯 Summary |
| 4 | + |
| 5 | +This PR implements conditional KMS policy support for S3 Vectors integration with Bedrock Knowledge Base, ensuring the "Allow S3 Vectors indexing service to use the key" policy is only applied when users select `S3_VECTORS` for their `KnowledgeBaseVectorStore`, following security best practices and the principle of least privilege. |
| 6 | + |
| 7 | +## 🔧 Changes Made |
| 8 | + |
| 9 | +### 1. Conditional KMS Policy Implementation |
| 10 | +- **File**: `template.yaml` |
| 11 | +- **Change**: Modified the `CustomerManagedEncryptionKey` policy to conditionally include S3 Vectors indexing service permissions |
| 12 | +- **Condition**: Uses existing `IsS3VectorsVectorStore` condition to check if `KnowledgeBaseVectorStore = "S3_VECTORS"` |
| 13 | +- **Security Benefit**: Only grants KMS permissions to S3 Vectors service when actually needed |
| 14 | + |
| 15 | +### 2. S3 Vectors IAM Permission Fixes |
| 16 | +- **File**: `options/bedrockkb/template.yaml` |
| 17 | +- **Issue**: `KnowledgeBaseServiceRole` was missing proper resource access patterns for S3 Vectors operations |
| 18 | +- **Fix**: Updated IAM policy to include both bucket and bucket contents access: |
| 19 | + - `arn:${AWS::Partition}:s3vectors:${AWS::Region}:${AWS::AccountId}:bucket/${BucketName}` |
| 20 | + - `arn:${AWS::Partition}:s3vectors:${AWS::Region}:${AWS::AccountId}:bucket/${BucketName}/*` |
| 21 | + |
| 22 | +### 3. Bucket Name Normalization Fix |
| 23 | +- **Issue**: IAM policy was using raw `${AWS::StackName}-s3-vectors` instead of AWS-normalized bucket name |
| 24 | +- **Root Cause**: AWS S3 Vectors service normalizes bucket names (lowercase, special character handling) |
| 25 | +- **Fix**: Use actual sanitized bucket name from custom resource: `!GetAtt S3VectorBucketAndIndex.BucketName` |
| 26 | + |
| 27 | +## 🚀 Benefits |
| 28 | + |
| 29 | +- **Security**: Implements least privilege access - S3 Vectors service only gets KMS permissions when selected |
| 30 | +- **Reliability**: Eliminates `s3vectors:QueryVectors` and `kms:Decrypt` permission errors |
| 31 | +- **Cost Optimization**: Enables users to leverage S3 Vectors for 40-60% lower storage costs vs OpenSearch Serverless |
| 32 | +- **Backward Compatibility**: No impact on existing deployments using OpenSearch Serverless |
| 33 | + |
| 34 | +## 🔍 Technical Details |
| 35 | + |
| 36 | +### Before vs After |
| 37 | + |
| 38 | +**Before**: S3 Vectors indexing service always had KMS permissions regardless of vector store selection |
| 39 | +```yaml |
| 40 | +- Sid: Allow S3 Vectors indexing service to use the key |
| 41 | + Effect: Allow |
| 42 | + Principal: |
| 43 | + Service: !Sub "indexing.s3vectors.${AWS::URLSuffix}" |
| 44 | + # ... always present |
| 45 | +``` |
| 46 | + |
| 47 | +**After**: Conditional policy based on user selection |
| 48 | +```yaml |
| 49 | +- !If |
| 50 | + - IsS3VectorsVectorStore |
| 51 | + - Sid: Allow S3 Vectors indexing service to use the key |
| 52 | + Effect: Allow |
| 53 | + Principal: |
| 54 | + Service: !Sub "indexing.s3vectors.${AWS::URLSuffix}" |
| 55 | + # ... only when S3_VECTORS selected |
| 56 | + - !Ref AWS::NoValue |
| 57 | +``` |
| 58 | +
|
| 59 | +### Error Resolution |
| 60 | +
|
| 61 | +**Original Error**: |
| 62 | +``` |
| 63 | +User: arn:aws:sts::912625584728:assumed-role/.../... is not authorized to perform: s3vectors:QueryVectors on resource: arn:aws:s3vectors:us-west-2:912625584728:bucket/idp-s3vectors-documentbedrockkb-da107qbjvad6-s3-vectors/index/bedrock-kb-index |
| 64 | +``` |
| 65 | +
|
| 66 | +**Resolution**: Fixed resource ARN patterns and bucket name references to match AWS service expectations. |
| 67 | +
|
| 68 | +## 🧪 Testing |
| 69 | +
|
| 70 | +- [x] Verified conditional KMS policy only applies when `KnowledgeBaseVectorStore = "S3_VECTORS"` |
| 71 | +- [x] Confirmed S3 Vectors Knowledge Base creation succeeds |
| 72 | +- [x] Validated data ingestion jobs complete successfully |
| 73 | +- [x] Tested backward compatibility with OpenSearch Serverless deployments |
| 74 | + |
| 75 | +## 📚 Documentation |
| 76 | + |
| 77 | +- **Updated**: `CHANGELOG.md` with concise feature summary |
| 78 | +- **Reference**: `docs/s3-vectors-knowledge-base.md` for complete S3 Vectors documentation |
| 79 | +- **Architecture**: Maintains unified Knowledge Base interface across both vector store types |
| 80 | + |
| 81 | +## 🔄 Deployment Impact |
| 82 | + |
| 83 | +- **Zero Breaking Changes**: Existing stacks continue to function normally |
| 84 | +- **Parameter Compatibility**: All existing parameters preserved |
| 85 | +- **Default Behavior**: OpenSearch Serverless remains the default (no behavior change) |
| 86 | +- **Migration Path**: Users can opt-in to S3 Vectors for new deployments |
| 87 | + |
| 88 | +## 🎉 Related Work |
| 89 | + |
| 90 | +This PR completes the S3 Vectors integration story by: |
| 91 | +1. ✅ Adding S3 Vectors as vector store option (previous work) |
| 92 | +2. ✅ Implementing custom resources for S3 vector management (previous work) |
| 93 | +3. ✅ **NEW**: Conditional security policies for proper access control |
| 94 | +4. ✅ **NEW**: Resolving all permission and normalization issues |
| 95 | + |
| 96 | +## 📋 Checklist |
| 97 | + |
| 98 | +- [x] Code follows project standards and conventions |
| 99 | +- [x] Security policies implement least privilege access |
| 100 | +- [x] All IAM permissions properly scoped and conditional |
| 101 | +- [x] Backward compatibility maintained |
| 102 | +- [x] Documentation updated (CHANGELOG.md) |
| 103 | +- [x] No breaking changes introduced |
| 104 | +- [x] Error scenarios tested and resolved |
| 105 | + |
| 106 | +--- |
| 107 | + |
| 108 | +This enhancement enables cost-optimized vector storage for Bedrock Knowledge Base while maintaining security best practices through conditional access control policies. |
0 commit comments