bug: Fix incorrect input validation (OpenSearch following Pydantic up…

…date) + Added missing CMK use (Alarm topic) (#600)
aws-samples · Oct 31, 2024 · 6c64064 · 6c64064
1 parent bd520f6
commit 6c64064
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 3 deletions.
diff --git a/lib/aws-genai-llm-chatbot-stack.ts b/lib/aws-genai-llm-chatbot-stack.ts
@@ -218,6 +218,7 @@ export class AwsGenAILLMChatbotStack extends cdk.Stack {
 
     const monitoringStack = new cdk.NestedStack(this, "MonitoringStack");
     const monitoringConstruct = new Monitoring(monitoringStack, "Monitoring", {
+      shared,
       prefix: props.config.prefix,
       advancedMonitoring: props.config.advancedMonitoring === true,
       appsycnApi: chatBotApi.graphqlApi,

diff --git a/lib/chatbot-api/functions/api-handler/routes/workspaces.py b/lib/chatbot-api/functions/api-handler/routes/workspaces.py
@@ -48,9 +48,13 @@ class CreateWorkspaceOpenSearchRequest(BaseModel):
     kind: str = SAFE_SHORT_STR_VALIDATION
     name: str = Field(min_length=1, max_length=100, pattern=name_regex)
     embeddingsModelProvider: str = SAFE_SHORT_STR_VALIDATION
-    embeddingsModelName: str = SAFE_SHORT_STR_VALIDATION
-    crossEncoderModelProvider: Optional[str] = SAFE_SHORT_STR_VALIDATION
-    crossEncoderModelName: Optional[str] = SAFE_SHORT_STR_VALIDATION
+    embeddingsModelName: str = Field(
+        min_length=0, max_length=500, pattern=r"^[A-Za-z0-9-_. /]*$", default=None
+    )
+    crossEncoderModelProvider: Optional[str] = SAFE_SHORT_STR_VALIDATION_OPTIONAL
+    crossEncoderModelName: Optional[str] = Field(
+        min_length=0, max_length=500, pattern=r"^[A-Za-z0-9-_. /]*$", default=None
+    )
     languages: List[Annotated[str, SAFE_SHORT_STR_VALIDATION]]
     hybridSearch: bool
     chunkingStrategy: str = SAFE_SHORT_STR_VALIDATION

diff --git a/lib/monitoring/index.ts b/lib/monitoring/index.ts
@@ -24,9 +24,12 @@ import { FilterPattern, ILogGroup, MetricFilter } from "aws-cdk-lib/aws-logs";
 import { IDistribution } from "aws-cdk-lib/aws-cloudfront";
 import { ITopic, Topic } from "aws-cdk-lib/aws-sns";
 import { NagSuppressions } from "cdk-nag";
+import { Shared } from "../shared";
+import { PolicyStatement, ServicePrincipal } from "aws-cdk-lib/aws-iam";
 
 export interface MonitoringProps {
   prefix: string;
+  readonly shared: Shared;
   advancedMonitoring: boolean;
   appsycnApi: IGraphqlApi;
   appsyncResolversLogGroups: ILogGroup[];
@@ -261,8 +264,21 @@ export class Monitoring extends Construct {
       const onAlarmTopic = new Topic(this, "CompositeAlarmTopic", {
         displayName: props.prefix + "CompositeAlarmTopic",
         enforceSSL: true,
+        masterKey: props.shared.kmsKey,
       });
 
+      if (props.shared.kmsKey) {
+        // Following the guide
+        // https://docs.aws.amazon.com/sns/latest/dg/sns-key-management.html#sns-what-permissions-for-sse
+        props.shared.kmsKey.addToResourcePolicy(
+          new PolicyStatement({
+            actions: ["kms:GenerateDataKey*", "kms:Decrypt"],
+            principals: [new ServicePrincipal("cloudwatch.amazonaws.com")],
+            resources: ["*"],
+          })
+        );
+      }
+
       /**
        * CDK NAG suppression
        */

diff --git a/tests/monitoring/__snapshots__/monitoring-contruct.test.ts.snap b/tests/monitoring/__snapshots__/monitoring-contruct.test.ts.snap
@@ -96,6 +96,52 @@ exports[`snapshot test 1`] = `
       "Type": "AWS::SQS::Queue",
       "UpdateReplacePolicy": "Delete",
     },
+    "ExampleA925490C": {
+      "DeletionPolicy": "Retain",
+      "Properties": {
+        "KeyPolicy": {
+          "Statement": [
+            {
+              "Action": "kms:*",
+              "Effect": "Allow",
+              "Principal": {
+                "AWS": {
+                  "Fn::Join": [
+                    "",
+                    [
+                      "arn:",
+                      {
+                        "Ref": "AWS::Partition",
+                      },
+                      ":iam::",
+                      {
+                        "Ref": "AWS::AccountId",
+                      },
+                      ":root",
+                    ],
+                  ],
+                },
+              },
+              "Resource": "*",
+            },
+            {
+              "Action": [
+                "kms:GenerateDataKey*",
+                "kms:Decrypt",
+              ],
+              "Effect": "Allow",
+              "Principal": {
+                "Service": "cloudwatch.amazonaws.com",
+              },
+              "Resource": "*",
+            },
+          ],
+          "Version": "2012-10-17",
+        },
+      },
+      "Type": "AWS::KMS::Key",
+      "UpdateReplacePolicy": "Retain",
+    },
     "Index": {
       "Properties": {
         "Edition": "edition",
@@ -121,6 +167,12 @@ exports[`snapshot test 1`] = `
       },
       "Properties": {
         "DisplayName": "CompositeAlarmTopic",
+        "KmsMasterKeyId": {
+          "Fn::GetAtt": [
+            "ExampleA925490C",
+            "Arn",
+          ],
+        },
       },
       "Type": "AWS::SNS::Topic",
     },

diff --git a/tests/monitoring/monitoring-contruct.test.ts b/tests/monitoring/monitoring-contruct.test.ts
@@ -16,6 +16,8 @@ import {
 import { Function } from "aws-cdk-lib/aws-lambda";
 import { StateMachine } from "aws-cdk-lib/aws-stepfunctions";
 import { LogGroup } from "aws-cdk-lib/aws-logs";
+import { Shared } from "../../lib/shared";
+import { Key } from "aws-cdk-lib/aws-kms";
 
 jest.spyOn(console, "log").mockImplementation(() => {});
 
@@ -36,6 +38,7 @@ new Queue(stack, "Queue", {
 
 new Monitoring(stack, "Monitoring", {
   prefix: "",
+  shared: { kmsKey: new Key(stack, "Example") } as Shared,
   advancedMonitoring: true,
   appsycnApi: GraphqlApi.fromGraphqlApiAttributes(stack, "GraphQL", {
     graphqlApiId: "graphqlApiId",