Skip to content

fix: prompt user for Terraform state bucket name to avoid S3 naming issue #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cstirry wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: prompt user for Terraform state bucket name to avoid S3 naming issue #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c6bcc3
fix: prompt user for Terraform state bucket name to avoid S3 naming c…
cstirry May 8, 2026
ac1fbc8
Update CLI configure extended tests
cstirry May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 61 additions & 16 deletions cli/commands/configure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,36 @@

PLUGINS = ["CKAN", "Socrata", "ArcGIS"]

# Bucket name must match the `backend "s3"` block in terraform/aws/main.tf.
# Default suggested bucket name shown in the wizard prompt.
# Users should change this to a unique name for their deployment.
TERRAFORM_STATE_BUCKET = "opencontext-terraform-state"


def _check_state_bucket(bucket_name: str, region: str) -> str:
"""Check whether the S3 bucket is available for use as Terraform state.

Returns one of:
"ok" — bucket exists and belongs to this account
"missing" — bucket does not exist (will be created later)
"taken" — bucket exists but belongs to another account (403)

Any other ClientError is re-raised unchanged.
"""
s3 = boto3.client("s3", region_name=region)
try:
s3.head_bucket(Bucket=bucket_name)
return "ok"
except botocore.exceptions.ClientError as e:
error_code = e.response["Error"]["Code"]
if error_code in ("403", "AccessDenied"):
return "taken"
if error_code in ("404", "NoSuchBucket"):
return "missing"
raise


def _ensure_state_bucket(bucket_name: str, region: str) -> None:
"""Check that the Terraform S3 state bucket exists; create it if not.
"""Create the Terraform S3 state bucket if it does not already exist.

Versioning and server-side encryption are enabled on newly created buckets.
No DynamoDB table is created — the Terraform backend does not use state
Expand Down Expand Up @@ -273,6 +297,27 @@ def configure(
if region is None:
raise typer.Exit(0)

# Prompt for state bucket name and validate immediately.
# Re-prompt if the bucket is owned by another AWS account (403).
# The --state-bucket CLI flag skips this prompt when a non-default value
# is passed, allowing automation and re-runs against existing state.
if state_bucket == TERRAFORM_STATE_BUCKET:
while True:
state_bucket = questionary.text(
"Terraform state bucket name:",
default=TERRAFORM_STATE_BUCKET,
).ask()
if state_bucket is None:
raise typer.Exit(0)
status = _check_state_bucket(state_bucket, region)
if status == "taken":
console.print(
f"[red]Bucket [bold]{state_bucket}[/bold] is owned by another "
f"AWS account. Please choose a different name.[/red]"
)
continue
break

city_slug = city_name.lower().replace(" ", "-")
suggested_lambda = f"{city_slug}-opencontext-mcp-{env}"

Expand Down Expand Up @@ -350,20 +395,20 @@ def configure(

_ensure_state_bucket(state_bucket, region)

# Override the backend config at init time so Terraform uses the correct
# bucket and region instead of the defaults hard-coded in main.tf.
if not (terraform_dir / ".terraform").exists():
init_cmd = [
"terraform",
"init",
f"-backend-config=bucket={state_bucket}",
f"-backend-config=region={region}",
]
run_cmd(
init_cmd,
cwd=terraform_dir,
spinner_msg="Initializing Terraform",
)
# Always reinitialize with -reconfigure to ensure the backend config is
# up to date, even if .terraform already exists from a previous run.
init_cmd = [
"terraform",
"init",
"-reconfigure",
f"-backend-config=bucket={state_bucket}",
f"-backend-config=region={region}",
]
run_cmd(
init_cmd,
cwd=terraform_dir,
spinner_msg="Initializing Terraform",
)

result = subprocess.run(
["terraform", "workspace", "list"],
Expand Down
18 changes: 13 additions & 5 deletions tests/test_cli_configure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ def _side(*a, **kw):
"City",
"120",
"us-east-1",
"opencontext-terraform-state", # state bucket name prompt
"city-mcp-staging",
"512",
"120",
Expand Down Expand Up @@ -602,6 +603,10 @@ def _run_configure_wizard(tmp_path, extra_kwargs: dict | None = None):
mock_s3 = MagicMock()
mock_s3.head_bucket.return_value = {}

# Only include bucket name response if not passing --state-bucket explicitly
skip_bucket_prompt = extra_kwargs and "state_bucket" in extra_kwargs
bucket_responses = [] if skip_bucket_prompt else ["opencontext-terraform-state"]

with (
patch("cli.commands.configure.get_project_root", return_value=tmp_path),
patch("cli.commands.configure.get_terraform_dir", return_value=tf_dir),
Expand All @@ -622,6 +627,7 @@ def _run_configure_wizard(tmp_path, extra_kwargs: dict | None = None):
"City",
"120",
"us-east-1",
*bucket_responses,
"city-mcp-staging",
"512",
"120",
Expand All @@ -636,7 +642,7 @@ def _run_configure_wizard(tmp_path, extra_kwargs: dict | None = None):
configure(**(extra_kwargs or {}))

return mock_run_cmd.call_args_list, mock_s3


class TestStateBucketFlag:
"""Tests for the --state-bucket CLI option on configure()."""
Expand All @@ -647,12 +653,14 @@ def test_default_bucket_passed_to_ensure_state_bucket(self, tmp_path):

_, mock_s3 = _run_configure_wizard(tmp_path)

# boto3.client is called inside _ensure_state_bucket; head_bucket receives
# the bucket name via the Bucket kwarg.
mock_s3.head_bucket.assert_called_once_with(Bucket=TERRAFORM_STATE_BUCKET)
# head_bucket is called twice: once in _check_state_bucket (validation prompt)
# and once in _ensure_state_bucket (creation check). Verify at least one call
# used the default bucket name.
mock_s3.head_bucket.assert_any_call(Bucket=TERRAFORM_STATE_BUCKET)

def test_custom_bucket_passed_to_ensure_state_bucket(self, tmp_path):
"""When --state-bucket is provided, _ensure_state_bucket receives the custom name."""
"""When --state-bucket is provided, the prompt is skipped and _ensure_state_bucket
receives the custom name directly."""
custom = "my-custom-tf-state"
_, mock_s3 = _run_configure_wizard(tmp_path, {"state_bucket": custom})

Expand Down
38 changes: 37 additions & 1 deletion tests/test_cli_configure_extended.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ class TestConfigureWizardCKAN:
@patch("cli.commands.configure.run_cmd")
@patch("cli.commands.configure.get_project_root")
@patch("cli.commands.configure.get_terraform_dir")
@patch("cli.commands.configure.boto3.client")
@patch("cli.commands.configure.questionary")
def test_ckan_wizard_writes_config_and_tfvars(
self,
mock_q,
mock_boto3,
mock_tf_dir,
mock_root,
mock_run_cmd,
Expand All @@ -52,6 +54,10 @@ def test_ckan_wizard_writes_config_and_tfvars(
mock_root.return_value = tmp_path
mock_tf_dir.return_value = tf_dir

mock_s3 = MagicMock()
mock_s3.head_bucket.return_value = {}
mock_boto3.return_value = mock_s3

responses = [
"Use example config as template", # starting_point
"City of Boston", # org_name
Expand All @@ -63,6 +69,7 @@ def test_ckan_wizard_writes_config_and_tfvars(
"Boston", # city_name (plugin)
"120", # timeout
"us-east-1", # region
"opencontext-terraform-state", # state bucket name prompt
"boston-opencontext-mcp-staging", # lambda_name
"512", # lambda_memory
"120", # lambda_timeout
Expand All @@ -82,9 +89,10 @@ def test_ckan_wizard_writes_config_and_tfvars(
responses[10],
responses[11],
responses[12],
responses[13],
]
)
mock_q.confirm.side_effect = _mock_q([responses[13]])
mock_q.confirm.side_effect = _mock_q([responses[14]])

# terraform workspace list returns empty
mock_subproc.return_value = subprocess.CompletedProcess(
Expand All @@ -105,10 +113,12 @@ def test_ckan_wizard_writes_config_and_tfvars(
@patch("cli.commands.configure.run_cmd")
@patch("cli.commands.configure.get_project_root")
@patch("cli.commands.configure.get_terraform_dir")
@patch("cli.commands.configure.boto3.client")
@patch("cli.commands.configure.questionary")
def test_socrata_wizard_without_app_token(
self,
mock_q,
mock_boto3,
mock_tf_dir,
mock_root,
mock_run_cmd,
Expand All @@ -122,6 +132,10 @@ def test_socrata_wizard_without_app_token(
mock_root.return_value = tmp_path
mock_tf_dir.return_value = tf_dir

mock_s3 = MagicMock()
mock_s3.head_bucket.return_value = {}
mock_boto3.return_value = mock_s3

mock_q.select.side_effect = _mock_q(
["Start from scratch", "staging", "Socrata"]
)
Expand All @@ -133,6 +147,7 @@ def test_socrata_wizard_without_app_token(
"", # app_token (empty → omitted)
"120", # timeout
"us-east-1", # region
"opencontext-terraform-state", # state bucket name prompt
"chicago-mcp-staging", # lambda_name
"512", # lambda_memory
"120", # lambda_timeout
Expand All @@ -155,10 +170,12 @@ def test_socrata_wizard_without_app_token(
@patch("cli.commands.configure.run_cmd")
@patch("cli.commands.configure.get_project_root")
@patch("cli.commands.configure.get_terraform_dir")
@patch("cli.commands.configure.boto3.client")
@patch("cli.commands.configure.questionary")
def test_arcgis_wizard_with_custom_domain(
self,
mock_q,
mock_boto3,
mock_tf_dir,
mock_root,
mock_run_cmd,
Expand All @@ -172,6 +189,10 @@ def test_arcgis_wizard_with_custom_domain(
mock_root.return_value = tmp_path
mock_tf_dir.return_value = tf_dir

mock_s3 = MagicMock()
mock_s3.head_bucket.return_value = {}
mock_boto3.return_value = mock_s3

mock_q.select.side_effect = _mock_q(["Start from scratch", "prod", "ArcGIS"])
mock_q.text.side_effect = _mock_q(
[
Expand All @@ -181,6 +202,7 @@ def test_arcgis_wizard_with_custom_domain(
"Seattle", # city_name (plugin)
"120", # timeout
"us-west-2", # region
"opencontext-terraform-state", # state bucket name prompt
"seattle-mcp-prod", # lambda_name
"512", # lambda_memory
"120", # lambda_timeout
Expand Down Expand Up @@ -209,10 +231,12 @@ def test_arcgis_wizard_with_custom_domain(
@patch("cli.commands.configure.run_cmd")
@patch("cli.commands.configure.get_project_root")
@patch("cli.commands.configure.get_terraform_dir")
@patch("cli.commands.configure.boto3.client")
@patch("cli.commands.configure.questionary")
def test_wizard_selects_existing_workspace(
self,
mock_q,
mock_boto3,
mock_tf_dir,
mock_root,
mock_run_cmd,
Expand All @@ -227,6 +251,10 @@ def test_wizard_selects_existing_workspace(
mock_root.return_value = tmp_path
mock_tf_dir.return_value = tf_dir

mock_s3 = MagicMock()
mock_s3.head_bucket.return_value = {}
mock_boto3.return_value = mock_s3

mock_q.select.side_effect = _mock_q(["Start from scratch", "staging", "CKAN"])
mock_q.text.side_effect = _mock_q(
[
Expand All @@ -237,6 +265,7 @@ def test_wizard_selects_existing_workspace(
"Boston",
"120",
"us-east-1",
"opencontext-terraform-state", # state bucket name prompt
"boston-mcp-staging",
"512",
"120",
Expand All @@ -260,10 +289,12 @@ def test_wizard_selects_existing_workspace(
@patch("cli.commands.configure.run_cmd")
@patch("cli.commands.configure.get_project_root")
@patch("cli.commands.configure.get_terraform_dir")
@patch("cli.commands.configure.boto3.client")
@patch("cli.commands.configure.questionary")
def test_wizard_runs_terraform_init_if_needed(
self,
mock_q,
mock_boto3,
mock_tf_dir,
mock_root,
mock_run_cmd,
Expand All @@ -279,6 +310,10 @@ def test_wizard_runs_terraform_init_if_needed(
mock_root.return_value = tmp_path
mock_tf_dir.return_value = tf_dir

mock_s3 = MagicMock()
mock_s3.head_bucket.return_value = {}
mock_boto3.return_value = mock_s3

mock_q.select.side_effect = _mock_q(["Start from scratch", "staging", "CKAN"])
mock_q.text.side_effect = _mock_q(
[
Expand All @@ -289,6 +324,7 @@ def test_wizard_runs_terraform_init_if_needed(
"City",
"120",
"us-east-1",
"opencontext-terraform-state", # state bucket name prompt
"city-mcp-staging",
"512",
"120",
Expand Down
Loading