Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
3 changes: 3 additions & 0 deletions .github/workflows/apply_peribolos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ jobs:
--fix-repos
--fix-team-repos
--min-admins 2
--required-admins jflowers
--required-admins jpower432
--required-admins marcusburghardt
--require-self=false
)

Expand Down
49 changes: 37 additions & 12 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

var configPath = flag.String("config", "../peribolos.yaml", "Path to peribolos config")
var ownersDir = flag.String("owners-dir", "../", "Directory to CODEOWNERS")
var ownersDir = flag.String("owners-dir", "../.github", "Directory to CODEOWNERS")

var cfg org.FullConfig

Expand Down Expand Up @@ -62,34 +62,37 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}

func loadOwners(dir string) ([]string, error) {
var owners []string

func loadOwners(dir string) (users []string, teams []string, err error) {
dir = path.Clean(dir)
file, err := os.Open(path.Join(dir, "CODEOWNERS"))
if err != nil {
return nil, err
return nil, nil, err
}

ruleset, err := codeowners.ParseFile(file)
if err != nil {
return nil, err
return nil, nil, err
}

rule, err := ruleset.Match(*configPath)
if err != nil {
return nil, err
return nil, nil, err
}

if rule == nil {
return nil, fmt.Errorf("no matching rule found for %s", *configPath)
return nil, nil, fmt.Errorf("no matching rule found for %s", *configPath)
}

for _, owner := range rule.Owners {
owners = append(owners, owner.String())
ownerStr := owner.String()
if strings.Contains(ownerStr, "/") {
teams = append(teams, ownerStr)
} else {
users = append(users, ownerStr)
}
}

return owners, nil
return users, teams, nil
}

func testDuplicates(list sets.Set[string]) error {
Expand Down Expand Up @@ -185,7 +188,7 @@ func testTeamMembers(teams map[string]org.Team, admins sets.Set[string], orgMemb
}

func TestOrgs(t *testing.T) {
own, err := loadOwners(*ownersDir)
ownUsers, ownTeams, err := loadOwners(*ownersDir)
if err != nil {
t.Fatalf("failed to load CODEOWNERS: %v", err)
}
Expand All @@ -195,7 +198,8 @@ func TestOrgs(t *testing.T) {
admins := normalize(sets.New(org.Admins...))
allOrgMembers := members.Union(admins)

approvers := normalize(sets.New(own...))
// Validate individual CODEOWNERS users are org admins
approvers := normalize(sets.New(ownUsers...))

if diff := approvers.Difference(admins); len(diff) > 0 {
t.Errorf("users do not match in CODEOWNERS and org admins '%s': %s", *org.Name, strings.Join(diff.UnsortedList(), ", "))
Expand All @@ -209,6 +213,27 @@ func TestOrgs(t *testing.T) {
t.Errorf("duplicate approvers: %v", err)
}

// Validate CODEOWNERS team references exist in peribolos config
teamRefs := normalize(sets.New(ownTeams...))

if err := testDuplicates(teamRefs); err != nil {
t.Errorf("duplicate team references in CODEOWNERS: %v", err)
}

if org.Teams != nil {
for _, ref := range teamRefs.UnsortedList() {
// Team references are in the form "org/team-name";
// extract the team name (part after the last "/").
teamName := ref
if idx := strings.LastIndex(ref, "/"); idx >= 0 {
teamName = ref[idx+1:]
}
if _, exists := org.Teams[teamName]; !exists {
t.Errorf("CODEOWNERS references team '%s' which does not exist in org '%s' teams", ref, *org.Name)
}
}
}

if both := admins.Intersection(members); len(both) > 0 {
t.Errorf("users in both org admin and member roles for org '%s': %s", *org.Name, strings.Join(both.UnsortedList(), ", "))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-05-12
163 changes: 163 additions & 0 deletions openspec/changes/restructure-teams-and-codeowners/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
## Context

The complytime GitHub organization manages 12 repositories with peribolos
(declarative GitHub org management via YAML). The organization recently split
content from `complyctl` by moving the openscap-plugin to `complytime-providers`
(the concept formerly called "plugins" is now called "providers"). Two providers exist today (openscap, ampel), a third
(opa) is expected. Additionally, `complytime-policies` needs dedicated ownership
for Gemara compliance content.

This change assumes the `fix-peribolos-implementation` change has been applied.
That change wires the `testTeamMembers()` validation function into `TestOrgs()`
and fixes admin/member role placement in existing teams. The new teams defined
here follow the corrected role assignment pattern established by that change.

Current state:
- `openscap-plugin-approvers` team still points at `complyctl` with stale naming
- No per-provider teams exist for ampel or opa
- No CODEOWNERS in complytime-providers or complytime-policies
- complyctl CODEOWNERS references `/cmd/openscap-plugin/` which no longer exists
- CODEOWNERS file locations are inconsistent (root vs `.github/`)
- `config_test.go` only validates individual users in CODEOWNERS, not team refs

## Goals / Non-Goals

**Goals:**
- Reflect the provider split in peribolos team structure
- Enable per-provider code review gates via CODEOWNERS
- Establish Gemara content ownership in complytime-policies
- Repurpose complytime-approvers for non-code repo stakeholder access
- Standardize CODEOWNERS location to `.github/CODEOWNERS` across all repos
- Update config_test.go to validate team references in CODEOWNERS
- Document why `privacy: closed` is required for all teams

**Non-Goals:**
- Reducing member duplication across teams via YAML anchors or nested teams
(explored and deferred; explicit lists kept for clarity)
- Changing complytime-dev membership or its broad write-access model
- Modifying repository settings beyond team access and CODEOWNERS
- Automating CODEOWNERS generation from peribolos config

## Decisions

### 1. Team naming convention: `*-provider-approvers`

Rename `openscap-plugin-approvers` to `openscap-provider-approvers` and follow
the same pattern for new teams: `ampel-provider-approvers`,
`opa-provider-approvers`. This reflects the terminology shift from "plugins" to
"providers."

**Alternative considered**: Generic `*-approvers` naming. Rejected because the
`-provider-` infix makes it clear these teams scope to the complytime-providers
repository specifically.

### 2. All complytime-dev members in every provider team

Every provider team includes all complytime-dev members (maintainers as team
maintainers, members as team members). Provider-specific contributors are added
on top (e.g., `fortiz-ai` for opa). This ensures the core dev team can always
review any provider code.

**Alternative considered**: Nested teams where provider teams inherit
complytime-dev membership. Rejected because GitHub CODEOWNERS only resolves
direct team members, not parent-team members. Child teams inherit repo
permissions but not CODEOWNERS review eligibility.

### 3. CODEOWNERS standardized to `.github/CODEOWNERS`

GitHub searches for CODEOWNERS in `.github/`, root, then `docs/`, using the
first found. The `.github/` location is recommended by GitHub documentation as
the most secure option, particularly for protecting the CODEOWNERS file itself.

**Reference**: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

### 4. `privacy: closed` for all teams (requirement, not preference)

All teams MUST use `privacy: closed`. GitHub CODEOWNERS requires teams to be
"visible" to be referenced. In GitHub's team privacy model, `closed` means
visible to all organization members, while `secret` teams cannot be referenced
in CODEOWNERS files.

**References**:
- CODEOWNERS visibility requirement: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
- Team privacy values: https://docs.github.com/en/rest/teams/teams#create-a-team

### 5. complytime-approvers repurposed for non-code repos

Rather than deleting `complytime-approvers` and creating a new team, repurpose
it with updated membership and repo access. This team grants write access to
non-code repositories: `community`, `complytime-demos`, and `website`.

The `.github` repository is explicitly excluded from this team's repo access.
Write access to the org management repo would provide no practical benefit
(contributors can fork to create PRs) while unnecessarily expanding the attack
surface. Only org admins retain write access to `.github` through their admin
role.

Membership: jflowers, jpower432, marcusburghardt (maintainers),
beatrizmcouto, hbraswelrh (members).

### 5a. Peribolos `--required-admins` for admin removal protection

The `apply_peribolos.yml` workflow includes `--required-admins` flags for each
current org admin (jflowers, jpower432, marcusburghardt). This causes peribolos
to fail if any of these admins are removed from the `admins:` list in
peribolos.yaml, providing defense-in-depth against admin removal even if a
malicious change passes code review.

### 6. config_test.go validation strategy

Split CODEOWNERS owner parsing into individual users and team references
(detected by presence of `/` in the owner string). Individual users are
validated as org admins (existing behavior). Team references are validated
against peribolos.yaml team definitions.

**Alternative considered**: Skipping team references entirely in validation.
Rejected because this would allow typos or references to non-existent teams.

### 7. complytime-providers CODEOWNERS uses provider teams only (not dev + provider)

Each provider path references only its provider team, not `complytime-dev`:
```
/cmd/openscap-provider/ @complytime/openscap-provider-approvers
```

Since all complytime-dev members are already in each provider team, adding
`@complytime/complytime-dev` would be redundant. The `*` fallback to
`@complytime/complytime-dev` covers shared code and any paths not matching a
provider-specific rule.

## Risks / Trade-offs

**[Member list duplication]** Provider teams duplicate complytime-dev members
explicitly. Adding/removing a dev requires updating multiple teams.
-> Mitigation: Accepted trade-off. The teams are defined in a single file
(peribolos.yaml) and validated by tests. YAML anchors or nested teams were
explored and deferred for simplicity.

**[Cross-repo coordination]** Changes span 4 repositories. CODEOWNERS changes
in complyctl, complytime-providers, and complytime-policies depend on the
teams existing first (via peribolos apply). If CODEOWNERS references a
non-existent team, GitHub silently ignores the reference — PRs merge without
the intended review gate, which is a silent security degradation.
-> Mitigation: Apply peribolos.yaml changes first (teams must exist before
CODEOWNERS references them). CODEOWNERS updates in other repos follow. After
peribolos apply, trigger the `drift_detection.yml` workflow manually to confirm
convergence between peribolos.yaml and the actual GitHub org state.

**[Team rename partial failure]** Renaming `openscap-plugin-approvers` to
`openscap-provider-approvers` is a destructive, non-atomic operation — peribolos
deletes the old team and creates the new one. If the apply fails midway, the
old team may be deleted before the new team is created, temporarily leaving
affected users without team-based write access.
-> Mitigation: Risk accepted. The impact is limited to the openscap-plugin team
rename only. Users retain org-level read access and complytime-dev write access
during any transient state. The drift detection workflow catches divergence.

**[Provider team divergence from CODEOWNERS]** If the last-matching-pattern
rule in CODEOWNERS selects only a provider team and a future member is removed
from that team but stays in complytime-dev, they lose review access for that
provider.
-> Mitigation: This is the intended behavior. Provider teams are the authority
for provider-specific code review. The `*` fallback ensures complytime-dev
reviews shared/non-provider code.
70 changes: 70 additions & 0 deletions openspec/changes/restructure-teams-and-codeowners/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
## Why

The complytime organization has undergone structural changes: the openscap-plugin
was moved from complyctl to complytime-providers (the concept formerly called
"plugins" is now called "providers"), and
complytime-providers now hosts multiple providers (openscap, ampel) with a third
(opa) expected. Additionally, complytime-policies needs dedicated ownership for
Gemara content. The current peribolos team structure and CODEOWNERS files do not
reflect these changes, leaving stale references, missing ownership rules, and
no per-provider review gates.

## What Changes

- **Rename** `openscap-plugin-approvers` to `openscap-provider-approvers`, expand
membership to all complytime-dev members, and point repo access at
`complytime-providers` instead of `complyctl`.
- **Create** `ampel-provider-approvers` and `opa-provider-approvers` teams, each
with all complytime-dev members as approvers (opa additionally includes
`fortiz-ai`).
- **Create** `complytime-policies-approvers` team for Gemara content ownership in
complytime-policies, with `fortiz-ai` as initial member.
- **Repurpose** `complytime-approvers` for write access to non-code repositories
(.github, community, complytime-demos, website) for project stakeholders.
- **Standardize** all CODEOWNERS files to `.github/CODEOWNERS` across repositories,
following GitHub's recommended location.
- **Create** CODEOWNERS for complytime-providers with per-provider path rules.
- **Create** CODEOWNERS for complytime-policies with combined team ownership.
- **Clean up** complyctl CODEOWNERS by removing stale openscap-plugin references
and simplifying to a single complytime-dev fallback.
- **Move** this repo's CODEOWNERS from root to `.github/CODEOWNERS` and add
`@complytime/complytime-approvers` as a code owner.
- **Update** `config_test.go` to handle team references in CODEOWNERS and adjust
the file path for the new CODEOWNERS location.

## Capabilities

### New Capabilities

- `team-restructuring`: Peribolos team definitions reflecting the new
organizational structure (rename, create, and repurpose teams).
- `codeowners-management`: CODEOWNERS file creation, cleanup, and standardization
across complytime-providers, complytime-policies, complyctl, and .github repos.
- `test-validation`: Updated config_test.go to validate team references in
CODEOWNERS and support the new `.github/CODEOWNERS` location.

### Modified Capabilities

(none -- no existing specs to modify)

### Removed Capabilities

(none -- `openscap-plugin-approvers` is being renamed, not removed)

## Impact

- **peribolos.yaml**: Team definitions restructured (1 rename, 3 creates,
1 repurpose). Repo access mappings change for multiple teams.
- **config_test.go**: Test logic updated to split CODEOWNERS parsing into
individual users and team references, with validation that referenced teams
exist in peribolos.yaml.
- **CODEOWNERS (this repo)**: Moved from root to `.github/`, team reference added.
- **CODEOWNERS (complyctl)**: Stale rules removed, simplified to single fallback.
- **CODEOWNERS (complytime-providers)**: New file with per-provider ownership.
- **CODEOWNERS (complytime-policies)**: New file with combined team ownership.
- **Cross-repo**: Changes span 4 repositories (.github, complyctl,
complytime-providers, complytime-policies). CODEOWNERS changes in other repos
MUST NOT be merged until peribolos has applied the new team definitions.
- **Documentation**: No README or CONTRIBUTING updates required. PR descriptions
for cross-repo CODEOWNERS changes should explain the new review gate behavior
to contributors of the affected repositories.
Loading
Loading