feat(git-gate): include hostname in deploy key title #393

Merged
didericis merged 2 commits from deploy-key-hostname-in-title into main 2026-07-16 23:34:09 -04:00
3 changed files with 14 additions and 4 deletions
Showing only changes of commit bbb8913382 - Show all commits
+10
View File
@@ -31,6 +31,7 @@ from __future__ import annotations
import dataclasses
import json
import secrets
import socket
import string
from dataclasses import dataclass
from pathlib import Path
@@ -87,6 +88,14 @@ def bottle_identity(agent_name: str) -> str:
return f"{slug}-{suffix}"
def globalize_slug(slug: str) -> str:
"""Return a globally-unique slug qualified with the current hostname.
Assumes slug is a value returned from mint_slug. Use wherever a slug
must be unique across hosts (e.g. deploy-key titles)."""
return f"{socket.gethostname()}-{slug}"
@dataclass(frozen=True)
class BottleMetadata:
"""Persistent record of how a bottle was launched, written at
@@ -340,6 +349,7 @@ __all__ = [
"BottleMetadata",
"agent_state_dir",
"bottle_identity",
"globalize_slug",
"bottle_state_dir",
"cleanup_state",
"clear_preserve_marker",
+2 -2
View File
@@ -10,10 +10,10 @@ from __future__ import annotations
import os
import dataclasses
import socket
from pathlib import Path
from typing import TYPE_CHECKING
from .bottle_state import globalize_slug
from .errors import MissingEnvVarError
from .log import info
from .manifest import ManifestBottle, ManifestGitEntry
@@ -47,7 +47,7 @@ def _provision_dynamic_key(
owner_repo = entry.UpstreamPath
if owner_repo.endswith(".git"):
owner_repo = owner_repo[:-4]
title = f"bot-bottle:{socket.gethostname()}:{slug}:{entry.Name}"
title = f"bot-bottle:{globalize_slug(slug)}:{entry.Name}"
Outdated
Review

bigger change, but if there's still a slug util method that's used to generate these slugs, I think we should add hostname to that instead. Ideally we want a consistent slug reused throughout the app.

bigger change, but if there's still a `slug` util method that's used to generate these slugs, I think we should add hostname to that instead. Ideally we want a consistent slug reused throughout the app.
Outdated
Review

Yes — mint_slug in bot_bottle/backend/resolve_common.py is that utility. Its output flows into Docker compose project names, container instance names (bot-bottle-{slug}), network names, and state-dir paths in addition to this deploy-key title.

Adding socket.gethostname() inside mint_slug would make ALL of those identifiers host-qualified — a consistent approach but a broader blast radius than just the key title. Two things to verify before doing that:

  1. Docker compose project names and container names have a 63-char limit — a long hostname on top of a label or agent name could push over it.
  2. Existing bottles resumed after the change would get a different slug (their state dirs live under the old slug), so resume would break for in-flight bottles.

Happy to make the change in mint_slug if that is the intended scope. If you prefer to keep the blast radius narrow, an alternative is a thin host_slug(slug) helper in resolve_common.py that only the deploy-key path calls — still centralises how we qualify slugs with hostname without touching Docker names or state dirs.

Which direction do you want?

Yes — mint_slug in bot_bottle/backend/resolve_common.py is that utility. Its output flows into Docker compose project names, container instance names (bot-bottle-{slug}), network names, and state-dir paths in addition to this deploy-key title. Adding socket.gethostname() inside mint_slug would make ALL of those identifiers host-qualified — a consistent approach but a broader blast radius than just the key title. Two things to verify before doing that: 1. Docker compose project names and container names have a 63-char limit — a long hostname on top of a label or agent name could push over it. 2. Existing bottles resumed after the change would get a different slug (their state dirs live under the old slug), so resume would break for in-flight bottles. Happy to make the change in mint_slug if that is the intended scope. If you prefer to keep the blast radius narrow, an alternative is a thin host_slug(slug) helper in resolve_common.py that only the deploy-key path calls — still centralises how we qualify slugs with hostname without touching Docker names or state dirs. Which direction do you want?
Outdated
Review

Second objection doesn't matter (nothing published yet), but first objection is valid. Think a host_slug method is useful. Explicitly state it assumes a slug returned from mint_slug. Also, the entry.Name is a bit of a smell... that's the random characters needed for uniqueness, correct? If so, maybe should change the method to globalize_slug, and have it prepend the hostname/append the random slug. Respond to this before doing anything.

Second objection doesn't matter (nothing published yet), but first objection is valid. Think a `host_slug` method is useful. Explicitly state it assumes a slug returned from `mint_slug`. Also, the `entry.Name` is a bit of a smell... that's the random characters needed for uniqueness, correct? If so, maybe should change the method to `globalize_slug`, and have it prepend the hostname/append the random slug. Respond to this before doing anything.
Outdated
Review

Clarifying entry.Name: it is the user-defined key from git-gate.repos. in the manifest (e.g. "upstream" or "myrepo"), not random characters. The 5-char random suffix that prevents collisions lives inside the slug itself (from bottle_identity()). entry.Name is needed in the title because a single bottle can gate multiple repos — it disambiguates which deploy key belongs to which entry.

Given that, globalize_slug makes sense as:

def globalize_slug(slug: str) -> str:
    """Return a globally-unique slug qualified with the hostname.
    Assumes slug is a value returned from mint_slug."""
    return f"{socket.gethostname()}-{slug}"

And the title would become:

title = f"bot-bottle:{globalize_slug(slug)}:{entry.Name}"

This keeps entry.Name as a suffix (for per-repo disambiguation within a bottle) and centralises the hostname-qualification logic in globalize_slug. Does that match your intent?

Clarifying entry.Name: it is the user-defined key from git-gate.repos.<Name> in the manifest (e.g. "upstream" or "myrepo"), not random characters. The 5-char random suffix that prevents collisions lives inside the slug itself (from bottle_identity()). entry.Name is needed in the title because a single bottle can gate multiple repos — it disambiguates which deploy key belongs to which entry. Given that, globalize_slug makes sense as: def globalize_slug(slug: str) -> str: """Return a globally-unique slug qualified with the hostname. Assumes slug is a value returned from mint_slug.""" return f"{socket.gethostname()}-{slug}" And the title would become: title = f"bot-bottle:{globalize_slug(slug)}:{entry.Name}" This keeps entry.Name as a suffix (for per-repo disambiguation within a bottle) and centralises the hostname-qualification logic in globalize_slug. Does that match your intent?
Outdated
Review

@didericis-claude cool, sounds good. Go for it.

@didericis-claude cool, sounds good. Go for it.
Outdated
Review

Done — c44a010. globalize_slug lives in bottle_state.py next to bottle_identity (resolve_common.py caused a circular import: git_gate_provision → resolve_common → git_gate → git_gate_provision). The title format is now bot-bottle:{host}-{slug}:{entry.Name}, using - as the separator between host and slug within the globalized portion, with : still separating the three top-level fields.

Done — c44a010. globalize_slug lives in bottle_state.py next to bottle_identity (resolve_common.py caused a circular import: git_gate_provision → resolve_common → git_gate → git_gate_provision). The title format is now bot-bottle:{host}-{slug}:{entry.Name}, using - as the separator between host and slug within the globalized portion, with : still separating the three top-level fields.
info(f"provisioning deploy key for git-gate.repos[{entry.Name!r}]")
key_id, private_key_bytes = provisioner.create(owner_repo, title)
+2 -2
View File
@@ -127,9 +127,9 @@ class TestProvisionDynamicKey(unittest.TestCase):
self.assertEqual(b"PRIVATE-KEY-BYTES", key_file.read_bytes())
id_file = Path(d) / "repo-deploy-key-id"
self.assertEqual("kid123", id_file.read_text())
# owner_repo had .git stripped; title carries hostname + slug + name
# owner_repo had .git stripped; title carries globalize_slug(slug) + name
hostname = socket.gethostname()
self.assertEqual([("o/r", f"bot-bottle:{hostname}:myslug:repo")], fake.created)
self.assertEqual([("o/r", f"bot-bottle:{hostname}-myslug:repo")], fake.created)
def test_missing_token_raises(self) -> None:
with tempfile.TemporaryDirectory() as d, \