diff --git a/README.md b/README.md index cc5fd98..43f0654 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,7 @@ Use the following environment variables to configure the application: | OWNER_CHECKER_IGNORED_OWNERS | `@ghost` | The comma-separated list of owners that should not be validated. Example: `"@owner1,@owner2,@org/team1,example@email.com"`. | | OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS | `true` | Specifies whether CODEOWNERS may have unowned files. For example:

`/infra/oncall-rotator/ @sre-team`
`/infra/oncall-rotator/oncall-config.yml`

The `/infra/oncall-rotator/oncall-config.yml` file is not owned by anyone. | | OWNER_CHECKER_OWNERS_MUST_BE_TEAMS | `false` | Specifies whether only teams are allowed as owners of files. | +| OWNER_CHECKER_ALLOW_CROSS_ORG_TEAMS | `false` | Specifies whether teams from different organizations are allowed. When set to `true`, the validator will: skip the organization check, validate team existence in their actual organization, and skip permission checks for cross-org teams (as permissions can't be verified across orgs). This is useful during transitions between organizations. | | NOT_OWNED_CHECKER_SKIP_PATTERNS | | The comma-separated list of patterns that should be ignored by `not-owned-checker`. For example, you can specify `*` and as a result, the `*` pattern from the **CODEOWNERS** file will be ignored and files owned by this pattern will be reported as unowned unless a later specific pattern will match that path. It's useful because often we have default owners entry at the begging of the CODOEWNERS file, e.g. `* @global-owner1 @global-owner2` | | NOT_OWNED_CHECKER_SUBDIRECTORIES | | The comma-separated list of subdirectories to check in `not-owned-checker`. When specified, only files in the listed subdirectories will be checked if they do not have specified owners in CODEOWNERS. | | NOT_OWNED_CHECKER_TRUST_WORKSPACE | `false` | Specifies whether the repository path should be marked as safe. See: https://github.com/actions/checkout/issues/766. | diff --git a/action.yml b/action.yml index 2dae83d..c1bdd92 100644 --- a/action.yml +++ b/action.yml @@ -69,6 +69,11 @@ inputs: default: "false" required: false + owner_checker_allow_cross_org_teams: + description: "Specifies whether teams from different organizations are allowed. When set to true, the validator will: skip the organization check, validate team existence in their actual organization, and skip permission checks for cross-org teams (as permissions can't be verified across orgs). This is useful during transitions between organizations." + default: "false" + required: false + not_owned_checker_subdirectories: description: "Only check listed subdirectories for CODEOWNERS ownership that don't have owners." required: false diff --git a/docs/gh-action.md b/docs/gh-action.md index a57d8c2..57263ad 100644 --- a/docs/gh-action.md +++ b/docs/gh-action.md @@ -89,6 +89,12 @@ jobs: # Specifies whether only teams are allowed as owners of files. owner_checker_owners_must_be_teams: "false" + # Specifies whether teams from different organizations are allowed. When set to true, + # the validator will: skip the organization check, validate team existence in their + # actual organization, and skip permission checks for cross-org teams (as permissions + # can't be verified across orgs). This is useful during transitions between organizations. + owner_checker_allow_cross_org_teams: "false" + # Only check listed subdirectories for CODEOWNERS ownership that don't have owners. not_owned_checker_subdirectories: "" ``` diff --git a/internal/check/valid_owner.go b/internal/check/valid_owner.go index 76b6e8b..3148736 100644 --- a/internal/check/valid_owner.go +++ b/internal/check/valid_owner.go @@ -38,6 +38,13 @@ type ValidOwnerConfig struct { AllowUnownedPatterns bool `envconfig:"default=true"` // OwnersMustBeTeams specifies whether owners must be teams in the same org as the repository OwnersMustBeTeams bool `envconfig:"default=false"` + // AllowCrossOrgTeams specifies whether teams from different organizations are allowed. + // When set to true, the validator will: + // - Skip the organization membership check + // - Validate team existence in their actual organization + // - Skip permission checks for cross-org teams (can't verify across orgs) + // This is useful during transitions between organizations. + AllowCrossOrgTeams bool `envconfig:"default=false"` } // ValidOwner validates each owner @@ -51,6 +58,7 @@ type ValidOwner struct { ignOwners map[string]struct{} allowUnownedPatterns bool ownersMustBeTeams bool + allowCrossOrgTeams bool } // NewValidOwner returns new instance of the ValidOwner @@ -73,6 +81,7 @@ func NewValidOwner(cfg ValidOwnerConfig, ghClient *github.Client, checkScopes bo ignOwners: ignOwners, allowUnownedPatterns: cfg.AllowUnownedPatterns, ownersMustBeTeams: cfg.OwnersMustBeTeams, + allowCrossOrgTeams: cfg.AllowCrossOrgTeams, }, nil } @@ -171,23 +180,32 @@ func (v *ValidOwner) selectValidateFn(name string) func(context.Context, string) } func (v *ValidOwner) initOrgListTeams(ctx context.Context) *validateError { + teams, err := v.fetchTeamsForOrg(ctx, v.orgName) + if err != nil { + return err + } + v.orgTeams = teams + return nil +} + +func (v *ValidOwner) fetchTeamsForOrg(ctx context.Context, orgName string) ([]*github.Team, *validateError) { var teams []*github.Team req := &github.ListOptions{ PerPage: 100, } for { - resultPage, resp, err := v.ghClient.Teams.ListTeams(ctx, v.orgName, req) + resultPage, resp, err := v.ghClient.Teams.ListTeams(ctx, orgName, req) if err != nil { // TODO(mszostok): implement retry? switch err := err.(type) { case *github.ErrorResponse: if err.Response.StatusCode == http.StatusUnauthorized { - return newValidateError("Teams for organization %q could not be queried. Requires GitHub authorization.", v.orgName) + return nil, newValidateError("Teams for organization %q could not be queried. Requires GitHub authorization.", orgName) } - return newValidateError("HTTP error occurred while calling GitHub: %v", err) + return nil, newValidateError("HTTP error occurred while calling GitHub: %v", err) case *github.RateLimitError: - return newValidateError("GitHub rate limit reached: %v", err.Message) + return nil, newValidateError("GitHub rate limit reached: %v", err.Message) default: - return newValidateError("Unknown error occurred while calling GitHub: %v", err) + return nil, newValidateError("Unknown error occurred while calling GitHub: %v", err) } } teams = append(teams, resultPage...) @@ -197,18 +215,10 @@ func (v *ValidOwner) initOrgListTeams(ctx context.Context) *validateError { req.Page = resp.NextPage } - v.orgTeams = teams - - return nil + return teams, nil } func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateError { - if v.orgTeams == nil { - if err := v.initOrgListTeams(ctx); err != nil { - return err.AsPermanent() - } - } - // called after validation it's safe to work on `parts` slice parts := strings.SplitN(name, "/", 2) org := parts[0] @@ -216,14 +226,40 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr team := parts[1] // GitHub normalizes name before comparison - if !strings.EqualFold(org, v.orgName) { + // Skip org check if cross-org teams are allowed + if !v.allowCrossOrgTeams && !strings.EqualFold(org, v.orgName) { return newValidateError("Team %q does not belong to %q organization.", name, v.orgName) } + // Determine which organization's teams to check + targetOrg := v.orgName + if v.allowCrossOrgTeams { + targetOrg = org // Use the team's actual organization + } + + // If we're checking a different org than what's cached, we need to fetch those teams + var teamsToCheck []*github.Team + if strings.EqualFold(targetOrg, v.orgName) { + // Check against the repository's org (use cached teams) + if v.orgTeams == nil { + if err := v.initOrgListTeams(ctx); err != nil { + return err.AsPermanent() + } + } + teamsToCheck = v.orgTeams + } else { + // Need to fetch teams from the different organization + var err *validateError + teamsToCheck, err = v.fetchTeamsForOrg(ctx, targetOrg) + if err != nil { + return err.AsPermanent() + } + } + teamExists := func() bool { - for _, v := range v.orgTeams { + for _, t := range teamsToCheck { // GitHub normalizes name before comparison - if strings.EqualFold(v.GetSlug(), team) { + if strings.EqualFold(t.GetSlug(), team) { return true } } @@ -231,7 +267,13 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr } if !teamExists() { - return newValidateError("Team %q does not exist in organization %q.", name, org) + return newValidateError("Team %q does not exist in organization %q.", name, targetOrg) + } + + // Skip permissions check for cross-org teams + // We can't verify permissions for teams in other organizations + if v.allowCrossOrgTeams && !strings.EqualFold(org, v.orgName) { + return nil } // repo contains the permissions for the team slug given diff --git a/internal/check/valid_owner_test.go b/internal/check/valid_owner_test.go index b42ee65..1e77b27 100644 --- a/internal/check/valid_owner_test.go +++ b/internal/check/valid_owner_test.go @@ -159,3 +159,55 @@ func TestValidOwnerCheckerOwnersMustBeTeams(t *testing.T) { }) } } + +func TestValidOwnerCheckerAllowCrossOrgTeams(t *testing.T) { + // This test validates the AllowCrossOrgTeams config flag behavior + // Note: This test only validates the configuration parsing and basic team syntax validation + // The actual org validation requires a GitHub client and is tested in integration tests + + t.Run("Config with AllowCrossOrgTeams enabled", func(t *testing.T) { + // given + ownerCheck, err := check.NewValidOwner(check.ValidOwnerConfig{ + Repository: "org/repo", + AllowUnownedPatterns: true, + AllowCrossOrgTeams: true, // Enable cross-org teams + }, nil, true) + require.NoError(t, err) + + // Verify the config was properly set + assert.NotNil(t, ownerCheck) + }) + + t.Run("Config with AllowCrossOrgTeams disabled", func(t *testing.T) { + // given + ownerCheck, err := check.NewValidOwner(check.ValidOwnerConfig{ + Repository: "org/repo", + AllowUnownedPatterns: true, + AllowCrossOrgTeams: false, // Disable cross-org teams (default) + }, nil, true) + require.NoError(t, err) + + // Verify the config was properly set + assert.NotNil(t, ownerCheck) + }) + + t.Run("Team syntax validation", func(t *testing.T) { + // Test that team syntax is recognized correctly regardless of org + tests := []struct { + owner string + isValid bool + }{ + {"@org/team", true}, + {"@different-org/team", true}, + {"@altana-poc/team", true}, + {"@altana-tech/team", true}, + {"@org/", false}, + {"org/team", false}, + } + + for _, tc := range tests { + result := check.IsValidOwner(tc.owner) + assert.Equal(t, tc.isValid, result, "Owner %s validation failed", tc.owner) + } + }) +}