Skip to content

Conversation

@OlaOla-37
Copy link
Contributor

@OlaOla-37 OlaOla-37 commented Feb 17, 2025

Changes Made

1. SSM Parameter Resolution

Modified SSM parameter resolution to use proper stack name prefixing using intrinsic functions:

# Before

SupportFilesBucketName:

    Type: AWS::SSM::Parameter::Value<String>

    Default: "SupportFilesBucketName"

# After

SupportFilesBucketName:

    Type: AWS::SSM::Parameter::Value<String>

    Default: resolve:ssm:${StackName}-SupportFilesBucketName

2. Resource ARN Construction

Updated S3 bucket ARN construction:

# Before

!Sub 'arn:aws:s3:::${BulkUploadBucketName}'

# After

!Join

  - ''

  - - 'arn:aws:s3:::'

    - !Sub '{{resolve:ssm:${ParentStackName}-BulkUploadBucketName}}'

3. CloudWatch LogGroup Naming

Enhanced LogGroup naming to properly resolve SSM parameters:

Before

LogGroupName: !Sub '/aws/vendedlogs/${StepFunctionName}'

After

LogGroupName: !Sub '/aws/vendedlogs/{{resolve:ssm:${ParentStackName}-StepFunctionName}}'

4. S3 Path Construction

Improved S3 path construction for better parameter resolution:

# Before

!Sub 's3://${OutputBucketName}/${OutputBucketParsedResults}/'

# After

!Join

  - ''

  - - 's3://'

    - !Sub '{{resolve:ssm:${ParentStackName}-OutputBucketName}}'

    - '/'

    - !Sub '{{resolve:ssm:${ParentStackName}-OutputBucketParsedResults}}'

    - '/'

5. Lambda SSM retreival

# Before
# Parameter Store Field Names used by main workflow

CONF_COMP_LANGS = "ComprehendLanguages"

CONF_REDACTION_LANGS = "ContentRedactionLanguages"

# After

# Get the stack name from environment variable

STACK_NAME = os.environ.get('STACK_NAME')

# Parameter Store Field Names used by main workflow

CONF_COMP_LANGS = f"{STACK_NAME}-ComprehendLanguages"

CONF_REDACTION_LANGS = f"{STACK_NAME}-ContentRedactionLanguages"

6. Glue Database Name Requirement

Modified database parameter to be required:

# Before

DatabaseName:

    Type: String

    Default: 'pca'

    Description: (Required) Glue catalog database name used to contain tables/views for SQL integration.

# After

DatabaseName:

    Type: String

    Description: Glue catalog database name used to contain tables/views for SQL integration.

    # Removed default value to make it required

    # Users must provide a valid database name that meets AWS Glue naming restrictions

Testing Performed

  1. New Stack Deployment

    • Tested fresh deployment in clean account

    • Verified all SSM parameters are created correctly

    • Confirmed resource naming follows expected pattern

  2. Stack Updates

    • Updated existing stack with new changes

    • Verified no disruption to running resources

    • Confirmed backward compatibility

  3. Multiple Stack Deployments

    • Deployed multiple stacks in same account

    • Verified parameter isolation between stacks

    • Confirmed no naming conflicts

Files Changed

  • pca-main-nokendra.template

  • pca-main.template

  • python-utilities.template

  • ssm.template

  • trigger.template

  • boto3.template

  • bulk.template

  • ffmeg.template

  • glue-database.template

  • pca.template

  • pca-server.template

  • pca-ui.template

  • pcaconfiguration.py

Related Documentation

Checklist

  • Tested new stack deployment

  • Tested stack updates

@OlaOla-37 OlaOla-37 closed this Feb 17, 2025
@OlaOla-37 OlaOla-37 deleted the feature/ssm-parameter-improvements branch February 17, 2025 23:14
@OlaOla-37 OlaOla-37 restored the feature/ssm-parameter-improvements branch February 17, 2025 23:17
@OlaOla-37 OlaOla-37 reopened this Feb 17, 2025
Copy link
Contributor

@rstrahan rstrahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a truly excellent Pull Request - I cannot thank you enough!
The fact that PCA did not support multiple stacks in the same region has bugged me, and tripped up many others for so long. This is gold.
Also, I confess - I did not know about the intrinsic function for referencing ssm param values {{resolve:ssm:parameter-name:version}} - so thank you so much for revealing it to me!!
I will pull to a branch and try it out today.. Meantime I left a few (non-critical) review comments - mostly about hidden files that made their way into the PR, and the aesthetics of deleting rather than commenting out lines... all minor stuff.


DatabaseName:
Type: String
Default: 'pca'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than force user to enter a unique database name, can we instead allow an empty default and use a condition in the template to replace an empty DatabaseName with a unique name constructed from the StackName?

@rstrahan rstrahan requested a review from kishd February 18, 2025 13:42
@rstrahan
Copy link
Contributor

I'm built it, updated an existing stack, inspected changes, and verified it worked like a charm by processing a couple of jobs. Merging it now to develop branch so a colleague can also merge and test with his feature branch where he's adding Amazon Nova model support.
Thanks again!! Happy to accept more Pull Requests from @OlaOla-37 !

@rstrahan
Copy link
Contributor

Looks like I merged too soon - my colleague found issues with fresh deployment.. He will post details here..

@dave-moser
Copy link
Contributor

I get errors when trying to deploy these changes as a new stack in an AWS account. The CopySamples stack is not able to fetch the SSM parameters for InputBucketName, InputBucketRawAudio, and SupportFilesBucketName.

image

@rstrahan
Copy link
Contributor

Thanks @dave-moser
@OlaOla-37 Can you please take a look at the CopySamples issue when deploying a new stack, and resubmit the PR? Also see my comment/request in the review. Tx!

@OlaOla-37
Copy link
Contributor Author

Hey @rstrahan . I'll take a look and resubmit the PR with changes. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants