Skip to content

validate: catch config GitHub rejects, before it is merged - #77

Closed
dev-milos wants to merge 1 commit into
mainfrom
fix/team-slug-collision-and-topic-format
Closed

validate: catch config GitHub rejects, before it is merged#77
dev-milos wants to merge 1 commit into
mainfrom
fix/team-slug-collision-and-topic-format

Conversation

@dev-milos

Copy link
Copy Markdown
Collaborator

Found while exercising the organisation features at a larger shape than the release was tested at.

Two kinds of value pass validate today and are rejected only by GitHub, during apply. Apply runs after merge, so by the time the failure appears the config is already on main and every later apply fails until someone pushes a fix. The plan a reviewer sees is clean, so review has no signal to act on.

Team names now fold to the slug, not to lower case

Foo Bar and Foo-Bar both slugify to foo-bar. GitHub does not reject the second — it appends a numeric suffix, and which of the two receives it depends on creation order. The configuration is keyed by name but imported and addressed by slug:

to = github_team.team[each.key]       # name
id = try(each.value.slug, each.key)   # slug

so an unpredictable suffix makes the import ID unpredictable. The existing rule already gave exactly this rationale — "names differing only in case produce the same GitHub team" — it just applied it to case alone rather than to the rest of the transformation. Case-only collisions keep their original message; separator collisions get one naming the derived slug.

Slug derivation was established against the API, not assumed: separators (space, ., /, &) collapse to a single hyphen, underscores survive, Latin accents transliterate, surrounding whitespace is trimmed. Where a name folds to nothing — a fully non-Latin script — the check falls back to the previous behaviour rather than guessing, so it cannot reject a valid pair.

A name that is only whitespace satisfied minLength=1 and reached GitHub, which answers 422 Name can't be blank. Now rejected.

Topics were unconstrained

The schema placed no limit on them. Confirmed against the API:

Sent GitHub
21 topics 422 A repository cannot have more than 20 topics.
51 characters 422 must start with a lowercase letter or number …
UpperCase, with.dot, topic with spaces same 422
9lives accepted — a leading digit is valid

Encoded as maxItems: 20 and ^[a-z0-9][a-z0-9-]{0,49}$ per item.

Verification

  • go test ./... passes, existing cases unchanged
  • a repo config carrying the three invalid topics above now fails validate with the offending index named, instead of planning clean
  • validate and validate-org both run clean against the real gr-oss-developers configuration, including its platform_core and release_engineering teams — no false rejection

repositories.go is one line: the file is not gofmt-clean on main, so reformatting it would have buried the change in ~80 lines of realignment. The schema hook lives in its own file instead.

Two values pass validation today and are only rejected by GitHub during
apply. Apply runs after merge, so the bad config is already on main and
every later apply fails until someone pushes a fix. The plan a reviewer
sees is clean, so there is no signal at review time.

Team names now fold to the slug GitHub derives, not to lower case. Two
names that differ only by a separator -- "Foo Bar" and "Foo-Bar" -- both
slugify to foo-bar. GitHub does not reject the second, it appends a
numeric suffix, and which team gets it depends on creation order. The
configuration is keyed by name but imported and addressed by slug, so
that suffix makes the import ID unpredictable. The existing rule already
gave this rationale for case, it just did not apply it to the rest of
the transformation. Case-only collisions keep their original message.

A name that is only whitespace satisfied minLength=1 and reached GitHub,
which answers 422 Name can't be blank.

Topics were unconstrained. GitHub caps them at 20 per repository and
requires each to start with a lowercase letter or digit, contain only
lowercase letters, digits and hyphens, and be at most 50 characters --
all three confirmed against the API.
@dev-milos
dev-milos marked this pull request as draft August 27, 2026 14:19
@dev-milos dev-milos closed this Aug 27, 2026
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.

1 participant