diff --git a/CODEOWNERS b/.github/CODEOWNERS similarity index 100% rename from CODEOWNERS rename to .github/CODEOWNERS diff --git a/.github/workflows/apply_peribolos.yml b/.github/workflows/apply_peribolos.yml index 597442f..11df929 100644 --- a/.github/workflows/apply_peribolos.yml +++ b/.github/workflows/apply_peribolos.yml @@ -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 ) diff --git a/config/config_test.go b/config/config_test.go index a0e4c82..20ea9ac 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -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 @@ -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 { @@ -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) } @@ -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(), ", ")) @@ -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(), ", ")) } diff --git a/openspec/changes/restructure-teams-and-codeowners/.openspec.yaml b/openspec/changes/restructure-teams-and-codeowners/.openspec.yaml new file mode 100644 index 0000000..40cc12f --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-12 diff --git a/openspec/changes/restructure-teams-and-codeowners/design.md b/openspec/changes/restructure-teams-and-codeowners/design.md new file mode 100644 index 0000000..70288ea --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/design.md @@ -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. diff --git a/openspec/changes/restructure-teams-and-codeowners/proposal.md b/openspec/changes/restructure-teams-and-codeowners/proposal.md new file mode 100644 index 0000000..84736fe --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/proposal.md @@ -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. diff --git a/openspec/changes/restructure-teams-and-codeowners/specs/codeowners-management/spec.md b/openspec/changes/restructure-teams-and-codeowners/specs/codeowners-management/spec.md new file mode 100644 index 0000000..6fc08ab --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/specs/codeowners-management/spec.md @@ -0,0 +1,119 @@ +## ADDED Requirements + +### Requirement: CODEOWNERS standardized to .github directory + +All CODEOWNERS files across the complytime organization SHALL be located at +`.github/CODEOWNERS` within each repository. This follows GitHub's recommended +location and search priority order (`.github/`, root, `docs/`). + +Reference: +https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners + +#### Scenario: This repo CODEOWNERS moved from root to .github + +- **GIVEN** the `.github` repository previously had CODEOWNERS at the root +- **WHEN** the migration is complete +- **THEN** `.github/CODEOWNERS` exists with the ownership rules +- **AND** the root `CODEOWNERS` file is deleted + +### Requirement: .github repo CODEOWNERS lists only org admins + +The `.github` repository CODEOWNERS SHALL list only individual org admin +users as code owners. The `@complytime/complytime-approvers` team SHALL NOT +be included in CODEOWNERS, despite having write access to the repository. +The file at `.github/CODEOWNERS` SHALL contain the line: + +``` +* @jflowers @jpower432 @marcusburghardt +``` + +This separation between write access (via team) and approval authority (via +CODEOWNERS) prevents privilege escalation. With `require_code_owner_review: +true` in the repository ruleset, only org admins can approve merges to +peribolos.yaml and other org management files. + +#### Scenario: CODEOWNERS file content validated + +- **GIVEN** the `.github/CODEOWNERS` file exists in this repository +- **WHEN** the file is read +- **THEN** it contains the line `* @jflowers @jpower432 @marcusburghardt` +- **AND** no team references appear in the file + +### Requirement: complyctl CODEOWNERS cleaned up + +The complyctl repository (at `.github/CODEOWNERS`, which is the file's current +location) SHALL be simplified to a single fallback rule assigning +`@complytime/complytime-dev` as the owner for all files. The stale +`/cmd/openscap-plugin/` rule and the `/cmd/complyctl/` specific rule SHALL be +removed. + +#### Scenario: Stale openscap-plugin rule removed + +- **GIVEN** the complyctl `.github/CODEOWNERS` file has been updated +- **WHEN** the file is read +- **THEN** there is no rule referencing `/cmd/openscap-plugin/` or + `@complytime/openscap-plugin-approvers` + +#### Scenario: Single fallback rule + +- **GIVEN** the complyctl `.github/CODEOWNERS` file has been updated +- **WHEN** the file is read +- **THEN** the only rule is `* @complytime/complytime-dev` + +Note: GitHub will request review from `@complytime/complytime-dev` for all PRs. + +### Requirement: complytime-providers CODEOWNERS created with per-provider rules + +The complytime-providers repository SHALL have a CODEOWNERS file at +`.github/CODEOWNERS` with a fallback rule for `@complytime/complytime-dev` +and per-provider path rules for each provider directory under `cmd/`. + +The file SHALL contain: +``` +* @complytime/complytime-dev +/cmd/openscap-provider/ @complytime/openscap-provider-approvers +/cmd/ampel-provider/ @complytime/ampel-provider-approvers +/cmd/opa-provider/ @complytime/opa-provider-approvers +``` + +#### Scenario: CODEOWNERS file content validated + +- **GIVEN** the complytime-providers `.github/CODEOWNERS` file has been created +- **WHEN** the file is read +- **THEN** it contains the fallback rule `* @complytime/complytime-dev` and + per-provider rules for `/cmd/openscap-provider/`, `/cmd/ampel-provider/`, + and `/cmd/opa-provider/` + +Note: GitHub uses last-matching-pattern semantics. A PR modifying only +`/cmd/openscap-provider/` triggers review from `openscap-provider-approvers` +only. A PR modifying both `/cmd/openscap-provider/` and `/internal/` triggers +review from both the provider team and `complytime-dev`. Shared code under +`/internal/` or root-level files match only the `*` fallback. + +### Requirement: complytime-policies CODEOWNERS created + +The complytime-policies repository SHALL have a CODEOWNERS file at +`.github/CODEOWNERS` with a single fallback rule assigning both +`@complytime/complytime-policies-approvers` and `@complytime/complytime-dev` +as code owners for all files. + +``` +* @complytime/complytime-policies-approvers @complytime/complytime-dev +``` + +#### Scenario: CODEOWNERS file content validated + +- **GIVEN** the complytime-policies `.github/CODEOWNERS` file has been created +- **WHEN** the file is read +- **THEN** it contains the line + `* @complytime/complytime-policies-approvers @complytime/complytime-dev` + +Note: GitHub will request review from both teams for all PRs. + +### Scope Note + +Validation of CODEOWNERS files in complyctl, complytime-providers, and +complytime-policies is out of scope for `config_test.go` in this repository. +Each repository's own CI pipeline is responsible for validating its CODEOWNERS +file. The `config_test.go` in this repo only validates the `.github/CODEOWNERS` +file within this repository. diff --git a/openspec/changes/restructure-teams-and-codeowners/specs/team-restructuring/spec.md b/openspec/changes/restructure-teams-and-codeowners/specs/team-restructuring/spec.md new file mode 100644 index 0000000..7180ba5 --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/specs/team-restructuring/spec.md @@ -0,0 +1,151 @@ +## ADDED Requirements + +### Requirement: Rename openscap-plugin-approvers to openscap-provider-approvers + +The peribolos configuration SHALL rename the team `openscap-plugin-approvers` to +`openscap-provider-approvers`. The team SHALL have `privacy: closed`. The team +description SHALL reference "openscap-provider in complytime-providers". +Maintainers SHALL be `jpower432` and `marcusburghardt`. Members SHALL be +`gvauter`, `hbraswelrh`, `sonupreetam`, and `trevor-vaughan`. The team SHALL +have write access to `complytime-providers` and SHALL NOT have access to +`complyctl`. + +#### Scenario: Team renamed and repo access updated + +- **GIVEN** the peribolos.yaml file contains the updated team definition +- **WHEN** peribolos.yaml is parsed +- **THEN** a team `openscap-provider-approvers` exists with `privacy: closed`, + maintainers `["jpower432", "marcusburghardt"]`, members `["gvauter", + "hbraswelrh", "sonupreetam", "trevor-vaughan"]`, and repos including + `complytime-providers: write` +- **AND** no team named `openscap-plugin-approvers` exists + +### Requirement: Create ampel-provider-approvers team + +The peribolos configuration SHALL define a team `ampel-provider-approvers` with +`privacy: closed`. Maintainers SHALL be `jpower432` and `marcusburghardt`. +Members SHALL be `gvauter`, `hbraswelrh`, `sonupreetam`, and `trevor-vaughan`. +The team SHALL have write access to `complytime-providers`. + +#### Scenario: Team created with correct membership + +- **GIVEN** the peribolos.yaml file contains the team definition +- **WHEN** peribolos.yaml is parsed +- **THEN** the team `ampel-provider-approvers` exists with `privacy: closed`, + maintainers `["jpower432", "marcusburghardt"]`, members `["gvauter", + "hbraswelrh", "sonupreetam", "trevor-vaughan"]`, and repos including + `complytime-providers: write` + +### Requirement: Create opa-provider-approvers team + +The peribolos configuration SHALL define a team `opa-provider-approvers` with +`privacy: closed`. Maintainers SHALL be `jpower432` and `marcusburghardt`. +Members SHALL be `fortiz-ai`, `gvauter`, `hbraswelrh`, `sonupreetam`, and +`trevor-vaughan`. The team SHALL have write access to `complytime-providers`. + +#### Scenario: Team created with provider-specific member + +- **GIVEN** the peribolos.yaml file contains the team definition +- **WHEN** peribolos.yaml is parsed +- **THEN** the team `opa-provider-approvers` exists with `privacy: closed`, + members including `fortiz-ai` in addition to all complytime-dev members, + and repos including `complytime-providers: write` + +### Requirement: Create complytime-policies-approvers team + +The peribolos configuration SHALL define a team `complytime-policies-approvers` +with `privacy: closed`. Maintainers SHALL be `jflowers`, `jpower432`, and +`marcusburghardt`. Members SHALL include `fortiz-ai`. The team SHALL have write +access to `complytime-policies`. + +#### Scenario: Team created for Gemara content ownership + +- **GIVEN** the peribolos.yaml file contains the team definition +- **WHEN** peribolos.yaml is parsed +- **THEN** the team `complytime-policies-approvers` exists with `privacy: closed`, + maintainers `["jflowers", "jpower432", "marcusburghardt"]`, member `fortiz-ai`, + and repos including `complytime-policies: write` + +### Requirement: Repurpose complytime-approvers team + +The peribolos configuration SHALL update `complytime-approvers` with +description "Write access to non-code repos for project stakeholders". +Maintainers SHALL be `jflowers`, `jpower432`, and `marcusburghardt`. Members +SHALL be `beatrizmcouto` and `hbraswelrh`. The team SHALL have write access to +`community`, `complytime-demos`, and `website`. The team SHALL NOT have write +access to `complyctl`, `complytime`, or `.github`. + +#### Scenario: Team repurposed with updated membership and repos + +- **GIVEN** the peribolos.yaml file contains the updated team definition +- **WHEN** peribolos.yaml is parsed +- **THEN** the team `complytime-approvers` has maintainers `["jflowers", + "jpower432", "marcusburghardt"]`, members `["beatrizmcouto", "hbraswelrh"]`, + and repos `community`, `complytime-demos`, `website` (all write) +- **AND** `.github` is not in the team's repos list + +#### Scenario: Previous members removed + +- **GIVEN** the peribolos.yaml file contains the updated team definition +- **WHEN** peribolos.yaml is parsed +- **THEN** `gvauter`, `sonupreetam`, and `trevor-vaughan` are not listed as + members of `complytime-approvers` + +## PRESERVED Requirements + +The following validations already exist in `config_test.go` (via +`testTeamMembers`). They are documented here to confirm they MUST be maintained +and will apply to all new and modified teams. No new test code is needed for +these — the existing validation covers them. + +### Requirement: All teams use privacy closed + +All teams in peribolos.yaml SHALL use `privacy: closed`. This is required +because CODEOWNERS team references require teams to be visible to all +organization members. In GitHub's team privacy model, `closed` means visible +to all organization members, while `secret` teams are only visible to team +members and organization owners and 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 + +#### Scenario: Secret team rejected by validation + +- **GIVEN** peribolos.yaml is loaded and config_test.go runs `testTeamMembers` +- **WHEN** a team in peribolos.yaml uses `privacy: secret` +- **THEN** the validation fails with an error indicating the team does not have + the `privacy: closed` field + +### Requirement: Team maintainers must be org admins + +All team maintainers in peribolos.yaml SHALL be organization admins. Non-admin +users SHALL be listed as team members, not maintainers. Organization admins +listed in a team SHALL be listed as maintainers, not members. + +#### Scenario: Non-admin listed as maintainer + +- **GIVEN** peribolos.yaml is loaded and config_test.go runs `testTeamMembers` +- **WHEN** a non-admin user is listed as a team maintainer +- **THEN** the validation fails with an error indicating the user should be in + the members list + +#### Scenario: Admin listed as member + +- **GIVEN** peribolos.yaml is loaded and config_test.go runs `testTeamMembers` +- **WHEN** an org admin is listed as a team member +- **THEN** the validation fails with an error indicating the user should be in + the maintainers list + +### Requirement: Team member and maintainer lists must be sorted + +All maintainer and member lists in peribolos.yaml team definitions SHALL be +sorted alphabetically. + +#### Scenario: Unsorted member list + +- **GIVEN** peribolos.yaml is loaded and config_test.go runs `testTeamMembers` +- **WHEN** a team has an unsorted member list +- **THEN** the validation fails with an error indicating the list is unsorted diff --git a/openspec/changes/restructure-teams-and-codeowners/specs/test-validation/spec.md b/openspec/changes/restructure-teams-and-codeowners/specs/test-validation/spec.md new file mode 100644 index 0000000..7d7940e --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/specs/test-validation/spec.md @@ -0,0 +1,108 @@ +## ADDED Requirements + +### Requirement: CODEOWNERS path updated for .github location + +The config_test.go `--owners-dir` flag default SHALL be updated from `"../"` +to `"../.github"` to reflect the new standardized CODEOWNERS location at +`.github/CODEOWNERS`. The `loadOwners` function SHALL read CODEOWNERS from +the `.github/` directory. + +#### Scenario: Test reads CODEOWNERS from .github directory + +- **GIVEN** config_test.go runs with default flags +- **WHEN** `loadOwners` is invoked +- **THEN** the test reads the CODEOWNERS file from `../.github/CODEOWNERS` + instead of `../CODEOWNERS` + +### Requirement: CODEOWNERS parsing separates users from teams + +The `loadOwners` function SHALL return separate lists for individual users +and team references. The return signature SHALL be +`(users []string, teams []string, err error)`. + +A team reference is identified by the presence of a `/` character in the owner +string (e.g., `@complytime/complytime-approvers`). Individual users do not +contain a `/`. + +Individual users SHALL have the `@` prefix stripped (e.g., `"jflowers"` not +`"@jflowers"`). Team references SHALL retain the org-qualified form without +`@` (e.g., `"complytime/complytime-approvers"`). + +#### Scenario: Mixed individual and team owners parsed + +- **GIVEN** the CODEOWNERS file contains + `* @jflowers @jpower432 @marcusburghardt @complytime/complytime-approvers` +- **WHEN** `loadOwners` parses the file +- **THEN** individual users are `["jflowers", "jpower432", "marcusburghardt"]` + and team references are `["complytime/complytime-approvers"]` + +### Requirement: Individual CODEOWNERS users validated as org admins + +The test SHALL validate that all individual users listed in CODEOWNERS are +org admins. This preserves the existing validation behavior. The minimum of +3 individual approvers requirement SHALL be maintained. + +#### Scenario: Non-admin individual in CODEOWNERS + +- **GIVEN** peribolos.yaml is loaded with its org admin list +- **WHEN** the CODEOWNERS file lists an individual user who is not an org admin +- **THEN** the test fails with an error indicating the user does not match + org admins + +#### Scenario: Fewer than 3 individual approvers + +- **GIVEN** peribolos.yaml is loaded +- **WHEN** the CODEOWNERS file lists fewer than 3 individual users +- **THEN** the test fails with an error indicating insufficient approvers + +### Requirement: Team references validated against peribolos config + +The test SHALL validate that all team references in CODEOWNERS correspond to +teams defined in peribolos.yaml. The team name extracted from the CODEOWNERS +entry (the part after the `/`, e.g., `complytime-approvers` from +`complytime/complytime-approvers`) MUST exist as a key in the org's teams map. + +#### Scenario: Valid team reference + +- **GIVEN** peribolos.yaml is loaded and contains the team `complytime-approvers` +- **WHEN** the CODEOWNERS file references `@complytime/complytime-approvers` +- **THEN** the test passes the team reference validation + +#### Scenario: Invalid team reference + +- **GIVEN** peribolos.yaml is loaded and contains no team `nonexistent-team` +- **WHEN** the CODEOWNERS file references `@complytime/nonexistent-team` +- **THEN** the test fails with an error indicating the team does not exist + in the org configuration + +### Requirement: No duplicate owners in CODEOWNERS + +The test SHALL validate that there are no duplicate entries in the CODEOWNERS +file, checking both individual users and team references independently. + +#### Scenario: Duplicate individual user + +- **GIVEN** the CODEOWNERS file has been parsed +- **WHEN** the same user appears twice in the owners list +- **THEN** the test fails with a duplicate approvers error + +#### Scenario: Duplicate team reference + +- **GIVEN** the CODEOWNERS file has been parsed +- **WHEN** the same team appears twice in the owners list +- **THEN** the test fails with a duplicate teams error + +## PRESERVED Requirements + +The following validations already exist in `config_test.go` and MUST be +maintained. The `loadOwners` changes MUST NOT regress these behaviors: + +- **Privacy check**: `testTeamMembers` validates all teams have `privacy: closed` +- **Admin-as-maintainer check**: `testTeamMembers` validates non-admins are not + listed as maintainers and admins are not listed as regular members +- **Sorted lists check**: `testTeamMembers` validates maintainer and member lists + are alphabetically sorted +- **Org membership check**: `testTeamMembers` validates all team members are org + members +- **Duplicate check**: `testTeamMembers` validates no duplicate maintainers or + members within a team diff --git a/openspec/changes/restructure-teams-and-codeowners/tasks.md b/openspec/changes/restructure-teams-and-codeowners/tasks.md new file mode 100644 index 0000000..60303b7 --- /dev/null +++ b/openspec/changes/restructure-teams-and-codeowners/tasks.md @@ -0,0 +1,45 @@ +## 1. Peribolos Team Definitions (this repo: .github) + +- [x] 1.1 Rename `openscap-plugin-approvers` to `openscap-provider-approvers` in peribolos.yaml: update team name, description, add `privacy: closed`, expand maintainers to include `jpower432`, expand members to all complytime-dev members, change repo access from `complyctl: write` to `complytime-providers: write` +- [x] 1.2 Create `ampel-provider-approvers` team in peribolos.yaml with `privacy: closed`, maintainers `jpower432` and `marcusburghardt`, members `gvauter`, `hbraswelrh`, `sonupreetam`, `trevor-vaughan`, and `complytime-providers: write` +- [x] 1.3 Create `opa-provider-approvers` team in peribolos.yaml with `privacy: closed`, maintainers `jpower432` and `marcusburghardt`, members `fortiz-ai`, `gvauter`, `hbraswelrh`, `sonupreetam`, `trevor-vaughan`, and `complytime-providers: write` +- [x] 1.4 Create `complytime-policies-approvers` team in peribolos.yaml with `privacy: closed`, maintainers `jflowers`, `jpower432`, `marcusburghardt`, member `fortiz-ai`, and `complytime-policies: write` +- [x] 1.5 Repurpose `complytime-approvers` in peribolos.yaml: update description to "Write access to non-code repos for project stakeholders", set maintainers to `jflowers`, `jpower432`, `marcusburghardt`, set members to `beatrizmcouto`, `hbraswelrh`, set repos to `community`, `complytime-demos`, `website` (all write). `.github` excluded — no non-admin write access to the org management repo. + +## 2. CODEOWNERS for This Repo (.github) + +- [x] 2.1 Create `.github/CODEOWNERS` with content: `* @jflowers @jpower432 @marcusburghardt` (team excluded for privilege escalation prevention) +- [x] 2.2 Delete the root `CODEOWNERS` file if it exists + +## 3. CODEOWNERS for complyctl (repo: complyctl) [blocked-by: 1.x applied via peribolos] + +- [x] 3.1 [P] In the `complyctl` repository, update `.github/CODEOWNERS` to a single rule: `* @complytime/complytime-dev` (remove stale `/cmd/openscap-plugin/` rule and `/cmd/complyctl/` specific rule) + +## 4. CODEOWNERS for complytime-providers (repo: complytime-providers) [blocked-by: 1.x applied via peribolos] + +- [x] 4.1 [P] In the `complytime-providers` repository, create `.github/CODEOWNERS` with fallback `* @complytime/complytime-dev` and per-provider rules for `/cmd/openscap-provider/`, `/cmd/ampel-provider/`, `/cmd/opa-provider/` + +## 5. CODEOWNERS for complytime-policies (repo: complytime-policies) [blocked-by: 1.x applied via peribolos] + +- [x] 5.1 [P] In the `complytime-policies` repository, create `.github/CODEOWNERS` with rule: `* @complytime/complytime-policies-approvers @complytime/complytime-dev` + +## 6. Test Validation Updates (this repo: .github) [should complete before or alongside sections 2-5] + +- [x] 6.1 Update `config_test.go` `--owners-dir` flag default from `"../"` to `"../.github"` +- [x] 6.2 Update `loadOwners` function to return separate lists for individual users and team references (split on `/` presence). Return signature: `(users []string, teams []string, err error)` +- [x] 6.3 Add validation in `TestOrgs` that team references from CODEOWNERS exist as teams in peribolos.yaml +- [x] 6.4 Maintain existing validation that individual CODEOWNERS users are org admins with minimum 3 required +- [x] 6.5 Add duplicate check for team references +- [x] 6.6 Run `go test ./...` and verify all tests pass (covers privacy:closed, sorted lists, admin-as-maintainer from existing `testTeamMembers` — no new code needed for those) + +## 7a. Peribolos Admin Protection (this repo: .github) + +- [x] 7a.1 Add `--required-admins` flags for jflowers, jpower432, and marcusburghardt to apply_peribolos.yml + +## 7. Verification [blocked-by: all prior sections] + +- [x] 7.1 Run `yamllint peribolos.yaml` and verify no lint errors +- [x] 7.2 Run `go test ./config/... -v -count=1` and verify all structural validations pass (privacy:closed, sorted lists, admin-as-maintainer, team reference validation) +- [ ] 7.3 After peribolos apply, trigger `drift_detection.yml` workflow manually to confirm convergence + + diff --git a/peribolos.yaml b/peribolos.yaml index 10c370c..8fa80bd 100644 --- a/peribolos.yaml +++ b/peribolos.yaml @@ -9,22 +9,26 @@ orgs: - jflowers - jpower432 - marcusburghardt - - pme-bot members: - au-der - beatrizmcouto - benroose - bplaxco - eeasley2014 + - fkolacek-rh - fortiz-ai - gvauter - hbraswelrh - jamimoor - JenniferPrivette + - jiprocha + - jpadmanrh - mraml - nladha09 - phoward-rh + - ppsomiad - rbaratam-psc + - rmonk-redhat - sonupreetam - trevor-vaughan - vojtapolasek @@ -104,7 +108,21 @@ orgs: org-infra: triage website: triage complytime-approvers: - description: This would be a CODEOWNERS group for cmd complytime + description: Write access to non-code repos for project stakeholders + privacy: closed + maintainers: + - jflowers + - jpower432 + - marcusburghardt + members: + - beatrizmcouto + - hbraswelrh + repos: + community: write + complytime-demos: write + website: write + ampel-provider-approvers: + description: CODEOWNERS group for ampel-provider in complytime-providers privacy: closed maintainers: - jpower432 @@ -115,16 +133,45 @@ orgs: - sonupreetam - trevor-vaughan repos: - complyctl: write - openscap-plugin-approvers: - description: This would be a CODEOWNERS group for cmd openscap-plugin + complytime-providers: write + openscap-provider-approvers: + description: CODEOWNERS group for openscap-provider in complytime-providers privacy: closed maintainers: + - jpower432 - marcusburghardt members: - gvauter + - hbraswelrh + - sonupreetam + - trevor-vaughan repos: - complyctl: write + complytime-providers: write + opa-provider-approvers: + description: CODEOWNERS group for opa-provider in complytime-providers + privacy: closed + maintainers: + - jpower432 + - marcusburghardt + members: + - fortiz-ai + - gvauter + - hbraswelrh + - sonupreetam + - trevor-vaughan + repos: + complytime-providers: write + complytime-policies-approvers: + description: CODEOWNERS group for complytime-policies + privacy: closed + maintainers: + - jflowers + - jpower432 + - marcusburghardt + members: + - fortiz-ai + repos: + complytime-policies: write complytime-dev: description: People working on complytime repo privacy: closed