diff --git a/CHANGELOG.md b/CHANGELOG.md index 57507a9..d132d53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,26 @@ and could stop an installed copy from updating. ## [Unreleased] +### Added + +- **`rigging` service containers take an optional `database` name.** + `.rigging.json`'s per-service config gained `database` alongside `version` + and `urlEnv`: `{"postgres": {"version": "16", "urlEnv": "TEST_DATABASE_URL", + "database": "onelife_test"}}`. rigging composes that name into both the + container's `POSTGRES_DB` (or `MYSQL_DATABASE`) and the connection URL it + hands the job, so a repo whose test harness guards on the database name — + refusing to run unless it ends in `_test`, a common way to keep a stray + `TEST_DATABASE_URL` from truncating dev or prod data — can now drive rigging + end to end. The key is **optional and default-preserving**: omitted, each + service uses the name it hardcoded before (`postgres` / `mysql`), so every + existing rendered workflow is byte-identical. It is rejected for `redis`, + which has no database concept, rather than silently ignored. Like + `testCommand` and `services` themselves, it is a manual escape hatch — + `rigging:init` does not propose it. This is the final increment closing the + organization-monorepo adoption of rigging (issue #24 follow-up): a repo on + Postgres with a name-guarded test database was the last case rigging could + not render without a hand-edit. + ### Changed - **keel's "What keel does not do" section now names the heredoc evasion diff --git a/README.md b/README.md index d9a1d91..db7959b 100644 --- a/README.md +++ b/README.md @@ -344,8 +344,9 @@ before 0.3.0 was pre-release; see the [CHANGELOG](CHANGELOG.md). custom `testCommand` when a repo's real test command isn't the default, and can run a `postgres`, `mysql`, or `redis` service alongside the tests (rigging owns the image tag, port, credentials, and health check, and hands the job a -connection URL). `hull` adds a license-free `trufflehog` scanner alongside -`gitleaks`. Two `init` +connection URL — whose `database` name the repo can set, so a harness that +guards its test DB by name works end to end). `hull` adds a license-free +`trufflehog` scanner alongside `gitleaks`. Two `init` skills still **refuse to scaffold** rather than render an artifact that cannot work: `hull:init` in an organization-owned repo with no scanner license, and `rigging:init` when a JavaScript toolchain is genuinely undeterminable (an diff --git a/plugins/rigging/rigging/config.py b/plugins/rigging/rigging/config.py index 9ee78db..32697d1 100644 --- a/plugins/rigging/rigging/config.py +++ b/plugins/rigging/rigging/config.py @@ -33,6 +33,13 @@ #: Actions expression is refused before render. SERVICE_VERSION_RE = VERSION_RE +#: A service database name the repo may choose. Letters, digits, underscore and +#: hyphen only -- safe both as a URL path segment and inside a double-quoted +#: YAML scalar, and narrow enough to refuse whitespace, quotes, and `${{` (none +#: of those characters are in the class), so a chosen name can never need YAML +#: quoting or smuggle an Actions expression into the rendered URL/env. +SERVICE_DATABASE_RE = re.compile(r"^[A-Za-z0-9_-]+$") + #: The literal that opens a GitHub Actions expression. A testCommand element #: containing it is rejected at load: GitHub substitutes `${{ ... }}` at the #: YAML layer, before any shell sees the line, so shlex.quote is no defence -- @@ -63,11 +70,17 @@ class ConfigError(Exception): @dataclass(frozen=True) class ResolvedService: """One service a stack's job runs: which registry service, at what image - version, and the env var its connection URL is exposed in.""" + version, the env var its connection URL is exposed in, and optionally the + database name to create and connect to (None takes the service's default).""" service_id: str version: str url_env: str + #: The database name, or None to take the service's registry default. Only + #: meaningful for a service with a database concept (postgres, mysql); + #: rejected at load for one without (redis). None is the default so an + #: omitted `database` reproduces the pre-existing rendered bytes exactly. + database: Optional[str] = None @dataclass(frozen=True) @@ -212,7 +225,7 @@ def _valid_test_command(value, stack_id): return tuple(argv) -_SERVICE_KEYS = frozenset({"version", "urlEnv"}) +_SERVICE_KEYS = frozenset({"version", "urlEnv", "database"}) def _valid_services(value, stack_id): @@ -222,8 +235,12 @@ def _valid_services(value, stack_id): health-check is worse than none); `version` and `urlEnv` are both required (a service with no urlEnv exposes nothing, and there is no version to default to); and `urlEnv` must be a plain env var identifier, since it is - rendered into YAML. The image is pinned by tag, and the connection URL is - composed by rigging from registry constants, so neither is user input here. + rendered into YAML. `database` is optional -- omitted, the service's + registry default is used, which reproduces the pre-existing bytes; set, it + names the database rigging composes into both the container env and the + connection URL, and is rejected for a service with no database concept. The + image is pinned by tag, and the connection URL is composed by rigging from + registry constants, so neither is user input here. """ if value is None: return () @@ -240,6 +257,7 @@ def _valid_services(value, stack_id): f"service {service_id!r}. Allowed: " f"{', '.join(services_registry.SERVICE_IDS)}." ) + spec = services_registry.SERVICE_REGISTRY[service_id] if not isinstance(entry, dict): raise ConfigError( f"{CONFIG_NAME}: 'stacks.{stack_id}.services.{service_id}' must " @@ -266,8 +284,27 @@ def _valid_services(value, stack_id): f"is required and must be an env var name matching " f"{URL_ENV_RE.pattern} (got {url_env!r})." ) + database = entry.get("database") + if database is not None: + # Reject `database` for a service with no database concept (redis) + # rather than silently ignore it -- the same reasoning as + # _valid_package_manager rejecting packageManager off node: a + # discarded setting leaves the user believing they configured + # something. Name the field so the message is actionable. + if spec.database_env is None: + raise ConfigError( + f"{CONFIG_NAME}: 'stacks.{stack_id}.services.{service_id}." + f"database' is set, but service {service_id!r} has no " + f"database to name; remove it." + ) + if not isinstance(database, str) or not SERVICE_DATABASE_RE.fullmatch(database): + raise ConfigError( + f"{CONFIG_NAME}: 'stacks.{stack_id}.services.{service_id}." + f"database' must be a string matching " + f"{SERVICE_DATABASE_RE.pattern} (got {database!r})." + ) resolved.append(ResolvedService(service_id=service_id, version=version, - url_env=url_env)) + url_env=url_env, database=database)) return tuple(resolved) diff --git a/plugins/rigging/rigging/plan.py b/plugins/rigging/rigging/plan.py index 2469c64..4becf6f 100644 --- a/plugins/rigging/rigging/plan.py +++ b/plugins/rigging/rigging/plan.py @@ -92,19 +92,31 @@ def _resolve_test_argv(stack_id: str, manager_id: str, def _resolve_services(service_list): - """Turn config.ResolvedService entries into (rendered services, job env).""" + """Turn config.ResolvedService entries into (rendered services, job env). + + The database name is the repo's `database` when set, else the service's + registry default (equal to what it hardcoded before the key existed, so an + omitted `database` reproduces the pre-existing bytes). It is composed into + both the container env (as the service's database_env) and the connection + URL. A service with no database concept (redis) has database_env None and a + URL template that ignores the argument. + """ rendered = [] env = [] for rs in service_list: spec = services_registry.SERVICE_REGISTRY[rs.service_id] + database = rs.database or spec.default_database + service_env = spec.base_env + if spec.database_env is not None: + service_env = service_env + ((spec.database_env, database),) rendered.append(RenderedService( name=spec.id, image=spec.image_ref(rs.version), - env=spec.env, + env=service_env, port=spec.port, options=spec.health_options, )) - env.append((rs.url_env, spec.url)) + env.append((rs.url_env, spec.url(database))) return tuple(rendered), tuple(env) diff --git a/plugins/rigging/rigging/services.py b/plugins/rigging/rigging/services.py index 473ca27..572ee34 100644 --- a/plugins/rigging/rigging/services.py +++ b/plugins/rigging/rigging/services.py @@ -14,6 +14,7 @@ from __future__ import annotations from dataclasses import dataclass +from typing import Optional @dataclass(frozen=True) @@ -25,23 +26,38 @@ class ServiceSpec: image: str #: The container's port, mapped host:container 1:1 and used in the URL. port: int - #: Container environment, rendered under the service's `env:`. Ordered so - #: the rendered YAML is deterministic. - env: tuple[tuple[str, str], ...] + #: Container environment that is NOT the database name (e.g. the password), + #: rendered under the service's `env:` before the database pair. Ordered so + #: the rendered YAML is deterministic. The database name is composed + #: separately at resolve time (see database_env) so a repo can choose it. + base_env: tuple[tuple[str, str], ...] #: The full Docker `--health-*` options string. A registry constant so a #: user string never reaches `options:`; every service MUST carry one, or #: the job races container startup and flakes. health_options: str - #: The connection-URL template, composed with .format(port=...). Holds the - #: registry's own credentials and default database name -- never user input. + #: The connection-URL template, composed with .format(port=..., database=...). + #: Holds the registry's own credentials -- never user input. A service with + #: no database concept (redis) simply omits the `{database}` placeholder, so + #: the argument is ignored for it. url_template: str + #: The container env var that names the database (POSTGRES_DB, MYSQL_DATABASE), + #: or None for a service with no database concept (redis). When set, the + #: chosen database is rendered as this env var, appended after base_env. + database_env: Optional[str] = None + #: The database name used when the repo does not set `database`. Equal to the + #: value this service hardcoded before the key existed, so an omitted + #: `database` reproduces the pre-existing bytes exactly. None when there is + #: no database concept (redis). + default_database: Optional[str] = None def image_ref(self, version: str) -> str: return f"{self.image}:{version}" - @property - def url(self) -> str: - return self.url_template.format(port=self.port) + def url(self, database: Optional[str]) -> str: + """The connection URL for `database`. A service without a `{database}` + placeholder (redis) ignores the argument -- str.format drops unused + keyword arguments.""" + return self.url_template.format(port=self.port, database=database) _HEALTH = "--health-interval 10s --health-timeout 5s --health-retries 5" @@ -51,30 +67,36 @@ def url(self) -> str: id="postgres", image="postgres", port=5432, - env=(("POSTGRES_PASSWORD", "postgres"), ("POSTGRES_DB", "postgres")), + base_env=(("POSTGRES_PASSWORD", "postgres"),), health_options=f"--health-cmd pg_isready {_HEALTH}", - url_template="postgresql://postgres:postgres@localhost:{port}/postgres", + url_template="postgresql://postgres:postgres@localhost:{port}/{database}", + database_env="POSTGRES_DB", + default_database="postgres", ), "mysql": ServiceSpec( id="mysql", image="mysql", port=3306, - env=(("MYSQL_ROOT_PASSWORD", "mysql"), ("MYSQL_DATABASE", "mysql")), + base_env=(("MYSQL_ROOT_PASSWORD", "mysql"),), # `mysqladmin ping` is a LIVENESS probe, not a readiness one: it returns # exit 0 as soon as the server answers the socket (even on "Access # denied"), which can be a beat before MySQL will serve queries. That is # the standard GitHub Actions idiom and --health-retries covers the gap; # it is not a query-level check. Do not mistake it for one. health_options=f'--health-cmd "mysqladmin ping" {_HEALTH}', - url_template="mysql://root:mysql@localhost:{port}/mysql", + url_template="mysql://root:mysql@localhost:{port}/{database}", + database_env="MYSQL_DATABASE", + default_database="mysql", ), "redis": ServiceSpec( id="redis", image="redis", port=6379, - env=(), + base_env=(), health_options=f'--health-cmd "redis-cli ping" {_HEALTH}', url_template="redis://localhost:{port}", + database_env=None, + default_database=None, ), } diff --git a/plugins/rigging/skills/init/SKILL.md b/plugins/rigging/skills/init/SKILL.md index 6bb54c0..10e7322 100644 --- a/plugins/rigging/skills/init/SKILL.md +++ b/plugins/rigging/skills/init/SKILL.md @@ -159,13 +159,18 @@ this document. - **`services`** (optional, per stack): service containers the job runs alongside its tests, as `{"": {"version": "", "urlEnv": - ""}}`. Supported services: `postgres`, `mysql`, `redis`. rigging - owns the image, port, credentials, and — crucially — the health check, so - the job waits for the container to be ready instead of racing it. The repo - picks only the image `version` (a major tag, e.g. `"16"`) and the `urlEnv` - the connection URL is exposed in (a plain env var name); rigging composes - the URL from its own credentials and sets it at the job level, so every - step sees it. `init` does not write this — declare it by hand when a suite + "", "database": ""}}`. Supported services: `postgres`, + `mysql`, `redis`. rigging owns the image, port, credentials, and — + crucially — the health check, so the job waits for the container to be ready + instead of racing it. The repo picks the image `version` (a major tag, e.g. + `"16"`), the `urlEnv` the connection URL is exposed in (a plain env var + name), and optionally the `database` name (letters/digits/underscore/hyphen; + default `postgres` / `mysql`) — rigging composes the URL from its own + credentials and the chosen database and sets it at the job level, so every + step sees it. Naming the database lets a repo whose test harness guards on + the DB name (e.g. refusing to run unless it ends in `_test`) drive rigging + end to end; `database` is rejected for `redis`, which has no database + concept. `init` does not write any of this — declare it by hand when a suite needs a live database. ## 3. Propose the config diff --git a/plugins/rigging/tests/golden/node-postgres-database.yml b/plugins/rigging/tests/golden/node-postgres-database.yml new file mode 100644 index 0000000..1210d1b --- /dev/null +++ b/plugins/rigging/tests/golden/node-postgres-database.yml @@ -0,0 +1,31 @@ +name: "ci" +on: + push: + branches: ["main"] + pull_request: +permissions: + contents: read +jobs: + node: + runs-on: "ubuntu-latest" + strategy: + matrix: + node: ["20"] + services: + postgres: + image: "postgres:16" + env: + POSTGRES_PASSWORD: "postgres" + POSTGRES_DB: "onelife_test" + ports: + - "5432:5432" + options: "--health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5" + env: + TEST_DATABASE_URL: "postgresql://postgres:postgres@localhost:5432/onelife_test" + steps: + - uses: "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1" # v7 + - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 + with: + node-version: "${{ matrix.node }}" + - run: "npm ci" + - run: "npm test" diff --git a/plugins/rigging/tests/test_config.py b/plugins/rigging/tests/test_config.py index e1c8d43..c0cbc28 100644 --- a/plugins/rigging/tests/test_config.py +++ b/plugins/rigging/tests/test_config.py @@ -425,3 +425,41 @@ def test_services_not_a_dict_rejected(tmp_path): with pytest.raises(ConfigError) as e: load_config(write(tmp_path, {"stacks": {"node": {"services": ["postgres"]}}})) assert "services" in str(e.value) + + +def test_service_database_threaded_onto_resolved_service(tmp_path): + cfg = load_config(write(tmp_path, { + "stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "TEST_DATABASE_URL", + "database": "onelife_test"}}}} + })) + assert cfg.stacks["node"].services == ( + ResolvedService(service_id="postgres", version="16", + url_env="TEST_DATABASE_URL", database="onelife_test"), + ) + + +def test_service_database_omitted_is_none(tmp_path): + cfg = load_config(write(tmp_path, {"stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "DB_URL"}}}}})) + assert cfg.stacks["node"].services[0].database is None + + +def test_database_on_a_service_without_one_rejected(tmp_path): + # redis has no database concept; naming a database for it is a config error, + # not a silently-ignored setting. + with pytest.raises(ConfigError) as e: + load_config(write(tmp_path, {"stacks": {"node": {"services": { + "redis": {"version": "7", "urlEnv": "REDIS_URL", + "database": "onelife_test"}}}}})) + msg = str(e.value) + assert "database" in msg and "redis" in msg # names the field and the service + + +@pytest.mark.parametrize("bad_database", ["one life", "db/name", "on${{x}}", "", "a.b"]) +def test_bad_service_database_rejected(tmp_path, bad_database): + with pytest.raises(ConfigError) as e: + load_config(write(tmp_path, {"stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "DB_URL", + "database": bad_database}}}}})) + assert "database" in str(e.value) diff --git a/plugins/rigging/tests/test_injection.py b/plugins/rigging/tests/test_injection.py index 3256e41..66b4651 100644 --- a/plugins/rigging/tests/test_injection.py +++ b/plugins/rigging/tests/test_injection.py @@ -234,3 +234,14 @@ def test_hostile_url_env_is_refused_before_render(tmp_path): "postgres": {"version": "16", "urlEnv": "${{ secrets.X }}"}}}}}) with pytest.raises(ConfigError): load_config(tmp_path) + + +def test_hostile_service_database_is_refused_before_render(tmp_path): + # `database` is composed into both a URL path segment and a container env + # value; a name carrying an Actions expression must be refused at load so it + # can never reach the rendered workflow. + write_config(tmp_path, {"name": "ci", "stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "DB_URL", + "database": "x${{ secrets.X }}"}}}}}) + with pytest.raises(ConfigError): + load_config(tmp_path) diff --git a/plugins/rigging/tests/test_render.py b/plugins/rigging/tests/test_render.py index 0083612..707fe9d 100644 --- a/plugins/rigging/tests/test_render.py +++ b/plugins/rigging/tests/test_render.py @@ -335,12 +335,41 @@ def test_existing_goldens_unchanged_after_refactor(tmp_path): ({"stacks": {"node": {"services": { "mysql": {"version": "8", "urlEnv": "DATABASE_URL"}}}}}, "node-mysql.yml"), + # A chosen database flows into BOTH the POSTGRES_DB env line and the URL + # path segment. This is the org-monorepo case: a repo whose test harness + # guards on the DB name ending in _test. + ({"stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "TEST_DATABASE_URL", + "database": "onelife_test"}}}}}, + "node-postgres-database.yml"), ]) def test_serviced_node_job_matches_golden(tmp_path, data, golden): cfg = load_config(write(tmp_path, data)) assert render(build_plan(cfg)) == read_golden(golden) +def test_database_changes_only_the_db_env_line_and_the_url_segment(tmp_path): + # Setting `database` must move exactly two lines relative to the default + # postgres render -- the POSTGRES_DB env value and the URL path segment -- + # and nothing else. A wider diff would mean the key leaked into some other + # part of the rendered workflow. + base = load_config(write(tmp_path, {"stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "TEST_DATABASE_URL"}}}}})) + named = load_config(write(tmp_path, {"stacks": {"node": {"services": { + "postgres": {"version": "16", "urlEnv": "TEST_DATABASE_URL", + "database": "onelife_test"}}}}})) + base_lines = render(build_plan(base)).splitlines() + named_lines = render(build_plan(named)).splitlines() + differing = [(b, n) for b, n in zip(base_lines, named_lines) if b != n] + assert len(base_lines) == len(named_lines) # no lines added or removed + assert differing == [ + (' POSTGRES_DB: "postgres"', + ' POSTGRES_DB: "onelife_test"'), + (' TEST_DATABASE_URL: "postgresql://postgres:postgres@localhost:5432/postgres"', + ' TEST_DATABASE_URL: "postgresql://postgres:postgres@localhost:5432/onelife_test"'), + ] + + def test_a_job_with_no_services_renders_no_services_or_env_block(tmp_path): cfg = load_config(write(tmp_path, {"stacks": {"node": {}}})) out = render(build_plan(cfg)) diff --git a/plugins/rigging/tests/test_services.py b/plugins/rigging/tests/test_services.py index 4cba1f4..9b62f59 100644 --- a/plugins/rigging/tests/test_services.py +++ b/plugins/rigging/tests/test_services.py @@ -13,12 +13,39 @@ def test_image_ref_appends_the_version_tag(): assert SERVICE_REGISTRY["postgres"].image_ref("16") == "postgres:16" -def test_url_is_composed_from_registry_constants_not_the_version(): +def test_url_is_composed_from_registry_constants_and_the_default_database(): pg = SERVICE_REGISTRY["postgres"] - # version selects the image tag only; it is not in the URL. - assert pg.url == "postgresql://postgres:postgres@localhost:5432/postgres" - assert SERVICE_REGISTRY["redis"].url == "redis://localhost:6379" - assert SERVICE_REGISTRY["mysql"].url == "mysql://root:mysql@localhost:3306/mysql" + # version selects the image tag only; it is not in the URL. The database + # defaults to the service's default_database, reproducing the historic URL. + assert pg.url(pg.default_database) == \ + "postgresql://postgres:postgres@localhost:5432/postgres" + mysql = SERVICE_REGISTRY["mysql"] + assert mysql.url(mysql.default_database) == \ + "mysql://root:mysql@localhost:3306/mysql" + + +def test_url_interpolates_a_chosen_database(): + assert SERVICE_REGISTRY["postgres"].url("onelife_test") == \ + "postgresql://postgres:postgres@localhost:5432/onelife_test" + + +def test_redis_url_ignores_the_database_argument(): + # redis has no database concept; its template has no {database} placeholder, + # so the argument is dropped rather than raising. + assert SERVICE_REGISTRY["redis"].url(None) == "redis://localhost:6379" + assert SERVICE_REGISTRY["redis"].url("anything") == "redis://localhost:6379" + + +def test_database_env_and_default_database_are_correct_per_service(): + assert SERVICE_REGISTRY["postgres"].database_env == "POSTGRES_DB" + assert SERVICE_REGISTRY["postgres"].default_database == "postgres" + assert SERVICE_REGISTRY["mysql"].database_env == "MYSQL_DATABASE" + assert SERVICE_REGISTRY["mysql"].default_database == "mysql" + + +def test_redis_has_no_database_concept(): + assert SERVICE_REGISTRY["redis"].database_env is None + assert SERVICE_REGISTRY["redis"].default_database is None def test_every_service_carries_a_health_check():