Feat/deploy script

[jeastham1993]

What does this PR do?

Motivation

Testing Guidelines

Additional Notes

Types of Changes

• Bug fix
• New feature
• Breaking change
• Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

• This PR's description is comprehensive
• This PR contains breaking changes that are documented in the description
• This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
• This PR impacts documentation, and it has been updated (or a ticket has been logged)
• This PR's changes are covered by the automated tests
• This PR collects user input/sensitive content into Datadog

[claude]

Claude finished @jeastham1993's task —— View job

---

Code Review Analysis

• Read and analyze build.sh changes
• Read and analyze cdk-deploy.sh changes
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Evaluate test coverage and reliability

---

Review Feedback

Code Quality and Best Practices

build.sh:

• ✅ Good use of pushd/popd for directory navigation
• ✅ Organized logging with dedicated build-logs directory
• ⚠️ Missing error handling: The script continues even if builds fail (no set -e or explicit error checking)
• ⚠️ No validation: Doesn't check if required tools (mvn, npm, dotnet, make) are installed

cdk-deploy.sh:

• ✅ Logical separation of shared infrastructure (synchronous) and services (parallel)
• ✅ Clear progress messages and organized logging
• ⚠️ No error handling: Missing set -e and no verification that deployments succeed
• ⚠️ No wait mechanism: Script exits without waiting for background deployments to complete

Potential Bugs and Issues

1. Silent failures: Both scripts use &>logfile redirection which could mask critical errors
2. Race conditions: In cdk-deploy.sh, services might have dependencies that require sequential deployment
3. Log overwriting: In build.sh:18 and 24, npm i and ./package.sh both redirect to the same log file without appending (>>)
4. Inconsistent CDK paths: Some services run CDK from subdirectories (/cdk) while others run from service root

Performance Considerations

Positives:

• ✅ cdk-deploy.sh uses parallel deployments for services, which is efficient
• ✅ Good separation of build vs deployment concerns

Areas for improvement:

• Consider parallelizing builds in build.sh where services don't depend on each other
• The sequential shared infrastructure deployment is appropriate (dependencies likely exist)

Security Concerns

1. 🔴 Critical: --require-approval never in CDK deployments bypasses security reviews for infrastructure changes
2. ⚠️ Medium: No input validation or sanitization of environment variables
3. ⚠️ Medium: Scripts run with full permissions of the executing user
4. ⚠️ Low: Log files might contain sensitive information from build/deploy processes

Test Coverage and Reliability

Recommendations:

1. Add error handling:

   set -e  # Exit on first error
   set -u  # Exit on undefined variables

2. Validate prerequisites:

   command -v npm >/dev/null 2>&1 || { echo "npm required but not installed"; exit 1; }

3. Wait for background processes in cdk-deploy.sh:

   wait  # Wait for all background jobs to complete
   echo "All deployments completed"

4. Check deployment success:

   if ! cdk deploy --require-approval never; then
     echo "Deployment failed"
     exit 1
   fi

Summary

The scripts provide a solid foundation for building and deploying the serverless application, but lack proper error handling and validation. The most critical issues are:

1. Silent failures - builds/deployments can fail without the script stopping
2. Security concern - automatic approval of infrastructure changes
3. No completion verification - background deployments may fail silently

Recommendation: Add proper error handling (set -e), prerequisite validation, and success verification before considering these scripts production-ready.

[Copilot]

Pull Request Overview

This PR adds a new make deps step to the build and deployment processes for the activity-service component. The changes ensure that dependencies are properly installed before building and deploying the activity service.

• Adds make deps command to the activity-service build pipeline
• Adds make deps command to the activity-service deployment pipeline

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
build.sh	Adds dependency installation step before building activity service
cdk-deploy.sh	Adds dependency installation step before deploying activity service

[Copilot]

The command chain is becoming complex and difficult to debug when failures occur. Consider breaking this into separate commands or adding error handling between steps to identify which specific step fails.

Suggested change

	make dev && make deps && cdk deploy --require-approval=never &>../../deployment-logs/activity-service.log &
	# Run make dev and check for errors
	make dev &>>../../deployment-logs/activity-service.log
	if [ $? -ne 0 ]; then
	echo "Error: 'make dev' failed for activity-service. Check the logs for details." &>>../../deployment-logs/activity-service.log
	popd
	exit 1
	fi
	# Run make deps and check for errors
	make deps &>>../../deployment-logs/activity-service.log
	if [ $? -ne 0 ]; then
	echo "Error: 'make deps' failed for activity-service. Check the logs for details." &>>../../deployment-logs/activity-service.log
	popd
	exit 1
	fi
	# Run cdk deploy and check for errors
	cdk deploy --require-approval=never &>>../../deployment-logs/activity-service.log
	if [ $? -ne 0 ]; then
	echo "Error: 'cdk deploy' failed for activity-service. Check the logs for details." &>>../../deployment-logs/activity-service.log
	popd
	exit 1
	fi

[Copilot]

The command chain is becoming complex and difficult to debug when failures occur. Consider breaking this into separate commands or adding error handling between steps to identify which specific step fails.

Suggested change

	make dev && make deps && make build &>../../build-logs/activity-service.log
	echo "Running 'make dev' for activity service..."
	make dev &>>../../build-logs/activity-service.log
	if [ $? -ne 0 ]; then
	echo "Error: 'make dev' failed for activity service. Check the log at ../../build-logs/activity-service.log."
	exit 1
	fi
	echo "Running 'make deps' for activity service..."
	make deps &>>../../build-logs/activity-service.log
	if [ $? -ne 0 ]; then
	echo "Error: 'make deps' failed for activity service. Check the log at ../../build-logs/activity-service.log."
	exit 1
	fi
	echo "Running 'make build' for activity service..."
	make build &>>../../build-logs/activity-service.log
	if [ $? -ne 0 ]; then
	echo "Error: 'make build' failed for activity service. Check the log at ../../build-logs/activity-service.log."
	exit 1
	fi