Skip to content

Commit 83f0e75

Browse files
committed
[FIX] fix hardcoded dockerfile generator
1 parent b48b4db commit 83f0e75

6 files changed

Lines changed: 173 additions & 12 deletions

File tree

src/fastapi_fastkit/backend/project_builder/config_generator.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -447,12 +447,20 @@ def _generate_fastapi_users_config(self) -> str:
447447

448448
return "\n".join(content)
449449

450-
def generate_docker_files(self) -> None:
451-
"""Generate Dockerfile and docker-compose.yml."""
450+
def generate_docker_files(self, app_module: str = "src.main:app") -> None:
451+
"""Generate Dockerfile and docker-compose.yml.
452+
453+
The ``app_module`` is the ``module:attr`` string baked into the
454+
Dockerfile's ``CMD`` (and into docker-compose's `command` if the
455+
compose generator ever needs it). Architecture presets that put
456+
the FastAPI app at a non-default location (e.g. ``domain-starter``
457+
ships ``src/app/main.py``) must pass the matching dotted path so
458+
the generated container actually starts.
459+
"""
452460
deployment = self.config.get("deployment", [])
453461

454462
if "Docker" in deployment:
455-
dockerfile_content = self._generate_dockerfile()
463+
dockerfile_content = self._generate_dockerfile(app_module=app_module)
456464
dockerfile_path = self.project_dir / "Dockerfile"
457465
with open(dockerfile_path, "w") as f:
458466
f.write(dockerfile_content)
@@ -463,8 +471,8 @@ def generate_docker_files(self) -> None:
463471
with open(compose_path, "w") as f:
464472
f.write(compose_content)
465473

466-
def _generate_dockerfile(self) -> str:
467-
"""Generate Dockerfile content."""
474+
def _generate_dockerfile(self, app_module: str = "src.main:app") -> str:
475+
"""Generate Dockerfile content with a layout-aware uvicorn target."""
468476
content = []
469477
content.append(
470478
"# --------------------------------------------------------------------------"
@@ -487,7 +495,7 @@ def _generate_dockerfile(self) -> str:
487495
content.append("")
488496
content.append("# Run application")
489497
content.append(
490-
'CMD ["uvicorn", "src.main:app", "--host", "0.0.0.0", "--port", "8000"]'
498+
f'CMD ["uvicorn", "{app_module}", "--host", "0.0.0.0", "--port", "8000"]'
491499
)
492500
content.append("")
493501

src/fastapi_fastkit/backend/project_builder/preset_layout.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,21 @@ def main_py_target(self, project_dir: str) -> Path:
130130
"""Absolute path where the dynamic main.py overlay should land."""
131131
return Path(project_dir) / self.profile.main_py_relpath
132132

133+
@property
134+
def app_module(self) -> str:
135+
"""Return the ``module:attr`` string uvicorn / Docker should target.
136+
137+
Derived from ``main_py_relpath`` so docker generation, runserver,
138+
and any future container-orchestration code all agree on the
139+
entrypoint a given preset produces.
140+
"""
141+
# Strip the trailing ``.py`` and convert path separators to dots.
142+
relpath = self.profile.main_py_relpath
143+
if relpath.endswith(".py"):
144+
relpath = relpath[: -len(".py")]
145+
module_part = relpath.replace("/", ".").replace("\\", ".")
146+
return f"{module_part}:app"
147+
133148
def db_config_target(self, project_dir: str) -> Path:
134149
"""Absolute path for the generated database config module."""
135150
return Path(project_dir) / self.profile.db_config_relpath

src/fastapi_fastkit/cli.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import shutil
99
import subprocess
1010
import sys
11-
from pathlib import Path
1211
from typing import Union, cast
1312

1413
import click
@@ -486,12 +485,14 @@ def init(
486485
# placeholder app (minimal, single-module). For richer presets
487486
# (classic-layered, domain-starter) we keep the template's
488487
# router-aware main.py intact.
488+
#
489+
# The strategist's ``main_py_target`` is always ``src/main.py``
490+
# for both regenerate-main presets, and both fastapi-empty and
491+
# fastapi-single-module ship that file, so we can write
492+
# straight to the strategist's path without a flat-``main.py``
493+
# fallback branch.
489494
if strategist.should_regenerate_main:
490495
main_py_path = strategist.main_py_target(project_dir)
491-
if not main_py_path.exists():
492-
fallback = Path(project_dir) / "main.py"
493-
if fallback.exists():
494-
main_py_path = fallback
495496
main_py_path.parent.mkdir(parents=True, exist_ok=True)
496497
main_py_path.write_text(generator.generate_main_py())
497498
else:
@@ -531,7 +532,12 @@ def init(
531532
# Generate Docker files if deployment selected
532533
deployment = config.get("deployment", [])
533534
if deployment and deployment != ["None"]:
534-
generator.generate_docker_files()
535+
# Thread the preset-aware app module so the generated
536+
# Dockerfile's ``CMD ["uvicorn", "<module>:app", ...]``
537+
# matches the layout the user actually generated. Default
538+
# ``src.main:app`` only works for minimal / single-module /
539+
# classic-layered; domain-starter needs ``src.app.main:app``.
540+
generator.generate_docker_files(app_module=strategist.app_module)
535541
print_success("Generated Docker deployment files")
536542

537543
# Surface preset-specific warnings (e.g. "you picked a preset

tests/test_backends/test_preset_layout.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,25 @@ def test_targets_match_documented_matrix(
123123
assert strategist.auth_config_target(project) == Path(project) / expected_auth
124124

125125

126+
class TestAppModule:
127+
"""The ``app_module`` property must match each preset's main.py path."""
128+
129+
@pytest.mark.parametrize(
130+
"preset_id, expected_app_module",
131+
[
132+
("minimal", "src.main:app"),
133+
("single-module", "src.main:app"),
134+
("classic-layered", "src.main:app"),
135+
("domain-starter", "src.app.main:app"),
136+
],
137+
)
138+
def test_app_module_matches_main_py_path(
139+
self, preset_id: str, expected_app_module: str
140+
) -> None:
141+
strategist = PresetLayoutStrategist(preset_id)
142+
assert strategist.app_module == expected_app_module
143+
144+
126145
class TestCompatibilityWarnings:
127146
"""Warnings only fire on presets that don't regenerate main.py."""
128147

tests/test_backends/test_project_builder_config_generator.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,35 @@ def test_generated_dockerfile_uses_exec_form_cmd(self) -> None:
368368
assert parsed[0] == "uvicorn"
369369
assert "src.main:app" in parsed
370370

371+
def test_generated_dockerfile_honors_custom_app_module(self) -> None:
372+
"""The Dockerfile CMD must target the caller-supplied app module.
373+
374+
Regression for the Codex P1 finding on PR #55: domain-starter
375+
ships ``src/app/main.py``, so the default ``src.main:app`` would
376+
produce a container that fails at startup. Callers (like the
377+
interactive init flow, via ``PresetLayoutStrategist.app_module``)
378+
thread the layout-correct module through ``generate_docker_files``.
379+
"""
380+
# given
381+
with tempfile.TemporaryDirectory() as tmp:
382+
config = {"deployment": ["Docker"]}
383+
generator = DynamicConfigGenerator(config, tmp)
384+
385+
# when
386+
generator.generate_docker_files(app_module="src.app.main:app")
387+
388+
# then
389+
dockerfile = Path(tmp) / "Dockerfile"
390+
content = dockerfile.read_text()
391+
cmd_lines = [
392+
line for line in content.splitlines() if line.startswith("CMD ")
393+
]
394+
assert len(cmd_lines) == 1
395+
parsed = json.loads(cmd_lines[0][len("CMD ") :])
396+
assert "src.app.main:app" in parsed
397+
# The bogus default must not leak in alongside the override.
398+
assert "src.main:app" not in parsed
399+
371400

372401
class TestHelperMethods:
373402
"""Test cases for helper methods."""

tests/test_cli_operations/test_cli_interactive_integration.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,90 @@ def test_init_interactive_cleans_up_on_failure(
349349
),
350350
]
351351

352+
@patch("fastapi_fastkit.cli.subprocess.run")
353+
@patch("fastapi_fastkit.backend.package_managers.uv_manager.UvManager.is_available")
354+
def test_domain_starter_dockerfile_targets_src_app_main(
355+
self,
356+
mock_uv_available: MagicMock,
357+
mock_subprocess: MagicMock,
358+
temp_dir: str,
359+
) -> None:
360+
"""Regression for Codex P1 on PR #55.
361+
362+
domain-starter ships ``src/app/main.py``; the generated Dockerfile's
363+
``CMD`` must target ``src.app.main:app``, not the legacy default
364+
``src.main:app`` — otherwise the container fails to boot.
365+
"""
366+
# given
367+
os.chdir(temp_dir)
368+
project_name = "docker-domain-starter"
369+
mock_uv_available.return_value = True
370+
371+
def _mock_subprocess(*args: Any, **kwargs: Any) -> MagicMock:
372+
if args and "venv" in str(args[0]):
373+
venv_path = Path(temp_dir) / project_name / ".venv"
374+
venv_path.mkdir(parents=True, exist_ok=True)
375+
bin_dir = "Scripts" if os.name == "nt" else "bin"
376+
(venv_path / bin_dir).mkdir(exist_ok=True)
377+
mock_result = MagicMock()
378+
mock_result.returncode = 0
379+
return mock_result
380+
381+
mock_subprocess.side_effect = _mock_subprocess
382+
383+
# when — pick the domain-starter preset and Docker-only deployment.
384+
result = self.runner.invoke(
385+
fastkit_cli,
386+
["init", "--interactive"],
387+
input="\n".join(
388+
[
389+
project_name,
390+
"Docker Tester",
391+
"docker@test.com",
392+
"Domain-starter Docker regression",
393+
"4", # Architecture preset: domain-starter
394+
"6", # Database: None (last option)
395+
"5", # Authentication: None (last option)
396+
"3", # Background Tasks: None (last option)
397+
"2", # Caching: None (last option)
398+
"4", # Monitoring: None (last option)
399+
"1", # Testing: Basic
400+
"", # Utilities: skip
401+
"1", # Deployment: Docker
402+
"2", # Package manager: uv
403+
"", # Custom packages: skip
404+
"Y", # Proceed
405+
"Y", # Create project folder
406+
]
407+
),
408+
)
409+
410+
# then
411+
project_path = Path(temp_dir) / project_name
412+
assert project_path.exists(), (
413+
f"project not created. exit_code={result.exit_code}\n"
414+
f"output:\n{result.output}"
415+
)
416+
417+
dockerfile = project_path / "Dockerfile"
418+
assert dockerfile.exists(), (
419+
f"Dockerfile missing despite Docker deployment selection.\n"
420+
f"output:\n{result.output}"
421+
)
422+
423+
content = dockerfile.read_text()
424+
assert "src.app.main:app" in content, (
425+
"Domain-starter Dockerfile must target src.app.main:app; "
426+
"the CMD currently reads:\n"
427+
+ "\n".join(
428+
line for line in content.splitlines() if line.startswith("CMD ")
429+
)
430+
)
431+
assert '"src.main:app"' not in content, (
432+
"Bogus legacy default src.main:app leaked into the generated "
433+
"Dockerfile despite the preset override."
434+
)
435+
352436
@pytest.mark.parametrize(
353437
"preset_choice,suffix,base_template_name,main_relpath,db_relpath,regenerates_main",
354438
_PRESET_LAYOUT_CASES,

0 commit comments

Comments
 (0)