Skip to content

Build inventory dynamically from the GitHub API (#55) - #74

Merged
silug merged 2 commits into
simp:mainfrom
silug:issue-55-dynamic-inventory
Jul 31, 2026
Merged

Build inventory dynamically from the GitHub API (#55)#74
silug merged 2 commits into
simp:mainfrom
silug:issue-55-dynamic-inventory

Conversation

@silug

@silug silug commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Closes #55

A repolist file can now define puppetsync::repos_source instead of a hand-maintained puppetsync::repos_config — the sync plan builds the repo list from the GitHub org listing at run time. data/sync/repolists/github-org.yaml ships ready to use (repolist=github-org).

Filtering — built around the real org's shape

Surveyed live: the simp org has 258 repos, 86 archived, and a large fork population (the augeasproviders family, puppetlabs-*, etc.). The task (list_github_repos, stdlib net/http, no new gems):

  • Excludes archived and empty repos (exclude_archived defaults true)
  • Excludes forks unless they match include_forks — the escape hatch for forks the org actively maintains. The shipped list carries the two known exceptions: rubygem-simp-rspec-puppet-facts and pupmod-voxpupuli-selinux
  • Honors a GitHub-side opt-out: repos with the puppetsync-ignore topic are excluded by default, so a repo can be excluded (or a future fork exception could be marked with an include_topics convention) without touching puppetsync
  • Name globs make no pupmod-simp-* assumption: the shipped include list is pupmod-*, puppet-*, rubygem-*, which catches puppet-gpasswd; filter_permitted_repos' project_type check after the clone remains the safety net for name-pattern false positives
  • Branches come from the API's default_branch — which the survey proved essential: pupmod-voxpupuli-selinux's default branch is simp-master, something a hand-maintained list has to know and a dynamic one gets for free

Reproducibility: the snapshot

The plan writes the generated list to data/sync/repolists/generated-<config>.yaml — a normal repolist file — and tells you so:

== dynamic inventory: 79 repos from GitHub org simp (snapshot: .../generated-<config>.yaml — use repolist=generated-<config> for approve/merge)

The approve/merge plans need no changes at all: run them with repolist=generated-<config> and they operate on the exact inventory the sync ran against. Snapshots are left untracked for the operator to commit when a session is handed off (matching the existing convention of committed dated repolists). Static repos_config entries merge on top of the generated list (they win for duplicate URLs), so per-repo overrides remain possible.

Verification

  • 9 fixture-driven specs covering the filter matrix (fork exceptions, archived, topics, globs, default-branch mapping, stable sort ordering)
  • Live run against the real simp org: 79 of 258 repos selected; spot-checks all pass — maintained forks kept (with simp-master for selinux), puppetlabs-apache and all augeasproviders* forks excluded, archived pupmod-simp-ntpd excluded, puppet-gpasswd included
  • Live plan e2e (temp config, not committed): repos_source narrowed to one repo → real API listing → real clone → feature branch checked out → snapshot written and verified to load through the normal repolist Hiera path via bolt lookup --plan-hierarchy
  • puppet parser validate --tasks, bolt plan show, the stage-list dry run (which skips generation, as intended), full rspec (166 examples, 0 failures), all green

With #49's idempotency merged, this is the pairing the issue called for: an org-wide repolist=github-org run is now safe — repos needing nothing pass through as unchanged. The remaining step toward #56 (scheduled runs) is CI-friendly pushing (HTTPS + token), the long-standing stretch-goal TODO in init.pp.

🤖 Generated with Claude Code

A repolist file can now define puppetsync::repos_source instead of a
hand-maintained puppetsync::repos_config; the sync plan builds the repo
list from the GitHub org listing at run time.

New list_github_repos task (stdlib net/http, no new gems):
- Excludes archived and empty repos; excludes forks unless they match
  include_forks — the escape hatch for forks the org actively maintains
  (rubygem-simp-rspec-puppet-facts, pupmod-voxpupuli-selinux)
- Excludes repos carrying the 'puppetsync-ignore' GitHub topic, so a
  repo can be opted out on the GitHub side without touching puppetsync
- include/exclude name globs (no pupmod-simp-* assumption: the shipped
  github-org.yaml also matches puppet-* like puppet-gpasswd, and
  project_type filtering after the clone remains the safety net)
- Each branch comes from the API's default_branch (e.g.
  pupmod-voxpupuli-selinux syncs simp-master automatically)

The plan snapshots the generated list to
data/sync/repolists/generated-<config>.yaml (a normal repolist file) so
approve/merge plans run against the exact same inventory via
repolist=generated-<config>; static repos_config entries merge on top.

Verified with 9 fixture-driven specs plus live runs against the real
simp org: 79 of 258 repos selected with all known exceptions handled
(maintained forks kept incl. the simp-master branch, plain forks and
the augeasproviders family excluded, archived pupmod-simp-ntpd
excluded, puppet-gpasswd included), and a live plan e2e that listed,
cloned, checked out, and produced a snapshot that loads as a normal
repolist. Closes simp#55.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@nick-markowski

Copy link
Copy Markdown
Member

Clean implementation of #55, and — worth calling out after the last two task PRs — this one is token-safe: no warn stdin, which matters most here since stdin carries github_authtoken. The token stays a sensitive param, goes over HTTPS with cert verification, and is never logged. Pagination, the doc-matching filter order, name-sorted (stable/idempotent) snapshots, the !list_pipeline_stages gate, and the static-wins + merge all check out. Approving, with one thing worth a decision:

include_forks is necessary but not sufficient. A fork listed in include_forks clears the fork gate but then still has to pass the final include check:

next false if repo['fork'] && exclude_forks && !glob_match?(name, include_forks)
glob_match?(name, include_globs) || topics.any? { |t| include_topics.include?(t) }

So a maintained fork must match include_forks and an include glob. It works for the shipped config only because both exceptions happen to match pupmod-*/rubygem-*, but:

  • The README / github-org.yaml phrasing ("forks are excluded unless listed in include_forks") implies the list alone suffices — a fork added there with an off-pattern name would be silently dropped, which is exactly the escape-hatch case.
  • The spec masks it: "keeps forks matching include_forks" runs with the default include: ['*'], so the coupling never surfaces.

Since listing a fork explicitly reads as "I want this fork," I'd have an include_forks match bypass the include-glob gate (treat it as an explicit allow-list — matches the docs). If you'd rather keep the AND semantics, document them and add a test mirroring the shipped config (specific globs + include_forks). Either way, pin it with a test.

Minor: the inventory fetch is a hard dependency with no retry (run_task here has no _catch_errors), so a transient GitHub blip aborts the whole run — fine interactively, but a small retry/backoff on fetch_org_repos would make scheduled #56 runs more robust. And fetch_org_repos pagination is only exercised by the live run, not a unit test.

Everything else — plan wiring, repos_config default, snapshot path/format, filter matrix coverage — looks right.

- A fork matching include_forks now bypasses the include-glob/topic
  gate entirely: listing a fork means "I want this fork", whatever its
  name. Previously a fork exception with an off-pattern name would be
  silently dropped by the final include check — exactly the escape-
  hatch case. New spec mirrors the shipped-config shape (narrow
  include globs + an off-pattern fork name); non-forks still need to
  pass the include filters
- Retry transient GitHub API failures (network errors, 5xx) with
  backoff, 3 attempts per page, failing fast on 4xx — so a blip
  doesn't abort a whole (possibly scheduled, per simp#56) run

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@silug

silug commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Agreed on the semantics call, and it's in 49414e9:

  1. include_forks is now a true allow-list: a fork matching it bypasses the include-glob/topic gate entirely — listing a fork means "I want this fork", whatever its name, which is exactly what the README/github-org.yaml wording already promised. Non-forks still pass through the include filters. The new spec mirrors the shipped-config shape you flagged as untested: narrow include: ['pupmod-*'] plus an off-pattern fork name, asserting the fork survives and a non-fork outside the globs doesn't. Live re-check against the real org: still 79 repos, exceptions intact.
  2. Retry with backoff added to the inventory fetch: 3 attempts per page on network errors and 5xx (fail-fast on 4xx), so a GitHub blip doesn't abort a run — with Scheduled org-wide puppetsync runs (continuous baseline enforcement) #56's scheduled runs in mind, as you said. Pagination itself remains live-tested only; the subprocess-spec style can't stub Net::HTTP in-process, so I've left that to the real-org runs rather than restructure the task for injectable HTTP.

10 examples, 0 failures.

🤖 Generated with Claude Code

@silug
silug merged commit d75b27b into simp:main Jul 31, 2026
2 checks passed
@silug
silug deleted the issue-55-dynamic-inventory branch July 31, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build inventory dynamically from the GitHub API instead of hand-maintained repolists

2 participants