didericis/bot-bottle
1
0
Fork 0
Code Issues 35 Pull Requests 8 Actions 2 Packages Releases Wiki Activity

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
Conversation 6 Commits 2 Files Changed 3
+6 -3
2 changed files with 6 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 59be808ab1 - Show all commits
bot_bottle/git_gate_provision.py
+2 -1
View File
@@ -10,6 +10,7 @@ from __future__ import annotations
import os
import dataclasses
import socket
from pathlib import Path
from typing import TYPE_CHECKING
@@ -46,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:{slug}:{entry.Name}"
title = f"bot-bottle:{socket.gethostname()}:{slug}:{entry.Name}"
didericis commented 2026-07-16 22:49:48 -04:00
Outdated
Review
Copy Link
Copy Source

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.
didericis-claude commented 2026-07-16 22:52:34 -04:00
Outdated
Review
Copy Link
Copy Source

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?
didericis commented 2026-07-16 22:56:28 -04:00
Outdated
Review
Copy Link
Copy Source

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.
didericis-claude commented 2026-07-16 23:01:02 -04:00
Outdated
Review
Copy Link
Copy Source

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?
didericis commented 2026-07-16 23:02:11 -04:00
Outdated
Review
Copy Link
Copy Source

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

@didericis-claude cool, sounds good. Go for it.
didericis-claude commented 2026-07-16 23:05:47 -04:00
Outdated
Review
Copy Link
Copy Source

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)
tests/unit/test_git_gate_render_provision.py
+4 -2
View File
@@ -6,6 +6,7 @@ Covers the pure `git_gate_render_gitconfig` renderer and the dynamic
from __future__ import annotations
import socket
import tempfile
import types
import unittest
@@ -126,8 +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 slug + name
self.assertEqual([("o/r", "bot-bottle:myslug:repo")], fake.created)
# owner_repo had .git stripped; title carries hostname + slug + name
hostname = socket.gethostname()
self.assertEqual([("o/r", f"bot-bottle:{hostname}:myslug:repo")], fake.created)
def test_missing_token_raises(self) -> None:
with tempfile.TemporaryDirectory() as d, \