Skip to content

feat: added terraform script to deploy MCP server on Agentcore runtime#1086

Open
anujbpanchal wants to merge 5 commits intomainfrom
feature/mcp-agentcore-runtime
Open

feat: added terraform script to deploy MCP server on Agentcore runtime#1086
anujbpanchal wants to merge 5 commits intomainfrom
feature/mcp-agentcore-runtime

Conversation

@anujbpanchal
Copy link
Copy Markdown
Collaborator

Proposed changes

  • Includes the terraform script required to deploy the MCP server on AWS Bedrock Agentcore Runtime.
  • Added some files that need to be ignored in .gitignore

Checklist

@anujbpanchal anujbpanchal requested a review from a team as a code owner April 21, 2026 12:42
@anujbpanchal anujbpanchal requested review from jeroenvervaeke and removed request for a team April 21, 2026 12:42
@anujbpanchal anujbpanchal changed the title Added terraform script to deploy MCP server on Agentcore runtime Feat: Added terraform script to deploy MCP server on Agentcore runtime Apr 21, 2026
@anujbpanchal anujbpanchal changed the title Feat: Added terraform script to deploy MCP server on Agentcore runtime feat: Added terraform script to deploy MCP server on Agentcore runtime Apr 21, 2026
@anujbpanchal anujbpanchal changed the title feat: Added terraform script to deploy MCP server on Agentcore runtime feat: added terraform script to deploy MCP server on Agentcore runtime Apr 21, 2026
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Apr 21, 2026

Coverage Report for CI Build 24723317621

Warning

No base build found for commit 6c2a51f on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 81.798%

Details

  • Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 3525
Covered Lines: 3071
Line Coverage: 87.12%
Relevant Branches: 2205
Covered Branches: 1616
Branch Coverage: 73.29%
Branches in Coverage %: Yes
Coverage Strength: 169.51 hits per line

💛 - Coveralls

Comment on lines +61 to +71
output "get_token_command" {
description = "Command to generate an OAuth token for the test user"
value = <<-EOT
python3 scripts/get_token.py \
--region ${local.region} \
--user-pool-id ${aws_cognito_user_pool.mcp.id} \
--client-id ${aws_cognito_user_pool_client.mcp.id} \
--username ${var.cognito_test_username} \
--password <PASSWORD>
EOT
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Cognito pool is configured with username_attributes = ["email"], so users must authenticate with their email. The Cognito user resource reflects this:

resource "aws_cognito_user" "test_user" {
  username = var.cognito_test_user_email  # email is the username

But the generated command uses a different variable:

--username ${var.cognito_test_username}  # defaults to "mcp-test-user"

Note: the cognito_test_username output above this block correctly maps to var.cognito_test_user_email, but Terraform outputs don't assign to input variables, so the command will embed "mcp-test-user" and fail.

Replace var.cognito_test_username with var.cognito_test_user_email on the --username line.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this should be checked in/committed

}

# MongoDB / Atlas credentials passed to the MCP server at runtime
variable "mdb_connection_string" {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The variable has no default, making it required at terraform apply. Anyone using only Atlas tools (no direct MongoDB connection) will be blocked.

Add default = "" to match the other optional credential variables.

@jeroenvervaeke
Copy link
Copy Markdown
Member

Thanks for putting this together. The README is thorough and the IAM policies are well-scoped. Before we merge, I'd like to discuss the intended audience for this module.

As it stands, a few things make it feel more like a personal "here's how I got it working" setup than something others can drop in:

  • The test user is baked into the infrastructure. The Cognito user, get_token.py, and the password variable are all wired into the Terraform module itself rather than being a separate dev convenience.
  • Docker build happens inside terraform apply, which couples a CI/CD operation to infrastructure provisioning. Most teams already have a separate image pipeline and this conflicts with it.
  • Cognito is assumed as the OIDC provider, but AgentCore supports any OIDC-compatible IdP. A more general module would accept oidc_discovery_url and allowed_client_ids as inputs rather than owning the Cognito setup.
  • There is no workspace support, log group paths are hardcoded, and there is no namespacing to allow multiple instances in the same account.

Would you be open to one of these directions?

  1. Reusable module: strip out the test user, get_token.py, and the Docker build step; accept the OIDC provider as an input. An examples/ subdirectory could show the opinionated setup for anyone who wants the full walkthrough.
  2. Example only: keep it as-is but move it under examples/ and make clear in the README that this is a reference implementation, not a production-ready module.

Happy to help shape whichever direction makes more sense.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants