Skip to content
Open
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
25 changes: 18 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,8 @@ github:
# contexts are the names of checks that must pass.
contexts:
- gh-infra/jenkins
- another/build-that-must-pass
- context: CodeQL
app: github-advanced-security
required_pull_request_reviews:
dismiss_stale_reviews: true
require_last_push_approval: false
Expand Down Expand Up @@ -421,9 +422,8 @@ required_status_checks:
strict: <boolean>
contexts:
- <string>
checks:
- context: <string>
app_id: <integer>
app: <integer> or <string>
```

If not explicitly specified, these values will be used by default:
Expand All @@ -440,14 +440,17 @@ required_pull_request_reviews:
required_status_checks:
strict: false
contexts: ~
checks: ~
```

**Notes**
1. Enabling any of the above checks overrides what you may have set previously, so you'll need to add all the existing checks to your `.asf.yaml` file to reproduce any that Infra set manually for you.
2. If you need to remove a required check in order to push a change to `.asf.yaml`, create an Infra Jira ticket with a request to have the check manually removed.

Using the 'contexts' list will automatically set an app ID of `-1` (any source) for checks. If you wish to specify a specific source app ID, you can make use of the expanded `checks` list instead:
The `contexts` list allows two kinds of entries:
- If you wish to specify a specific source app, you need to provide a `context` property for the name of the check and an `app` property for the app.
You can use either the application's id or its slug.
The correctness of the slug can be checked accessing the URL `https://github.com/apps/<app_slug>`, e.g. https://github.com/apps/github-actions.
- Otherwise, you can just specify the name of the check.

~~~yaml
github:
Expand All @@ -457,10 +460,18 @@ github:
# strict means "Require branches to be up to date before merging".
strict: true
checks:
# A Github Actions workflow name that must pass
- context: build / build (ubuntu-latest)
app: github-actions
# A security check that must pass
- context: CodeQL
app: github-advanced-security
# GitHub App specified by id
- context: gh-infra/jenkins
app_id: 1234
app: 1234
# Equivalent to any GitHub App
- context: another/build-that-must-pass
app_id: -1
- a-third/build-that-must-pass
...
~~~

Expand Down
13 changes: 6 additions & 7 deletions asfyaml/feature/github/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ class ASFGitHubFeature(ASFYamlFeature, name="github"):
strictyaml.Optional("ghp_branch"): strictyaml.Str(),
strictyaml.Optional("ghp_path", default="/docs"): strictyaml.Str(),
# Branch protection rules - TODO: add actual schema
strictyaml.Optional("protected_branches"): asfyaml.validators.EmptyValue()
| strictyaml.MapPattern(
strictyaml.Optional("protected_branches"): strictyaml.MapPattern(
strictyaml.Str(),
strictyaml.Map(
{
Expand All @@ -118,12 +117,12 @@ class ASFGitHubFeature(ASFYamlFeature, name="github"):
strictyaml.Optional("required_status_checks"): strictyaml.Map(
{
strictyaml.Optional("strict", default=False): strictyaml.Bool(),
strictyaml.Optional("contexts"): strictyaml.Seq(strictyaml.Str()),
strictyaml.Optional("checks"): strictyaml.Seq(
strictyaml.Map(
strictyaml.Optional("contexts"): strictyaml.Seq(
strictyaml.Str()
| strictyaml.Map(
{
strictyaml.Optional("context"): strictyaml.Str(),
strictyaml.Optional("app_id", default=-1): strictyaml.Int(),
"context": strictyaml.Str(),
strictyaml.Optional("app"): strictyaml.Int() | strictyaml.Str(),
}
)
),
Expand Down
68 changes: 47 additions & 21 deletions asfyaml/feature/github/branch_protection.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,42 @@ def get_head_refs(self: ASFGitHubFeature) -> list[Mapping[str, Any]]:
return []


def __context_get_name(context: str | dict) -> str:
return context if isinstance(context, str) else context["context"]


def __slug_get_app_id(self: ASFGitHubFeature, slug: str) -> int | None:
# Test run
if "noop" in self.instance.environments_enabled:
return 1234
try:
return self.gh.get_app(slug).id
except pygithub.GithubException:
print(f"[github] Unable to find GitHub app for slug {slug}.")
return None


# Get the application id for a protected branch check
def __context_get_app_id(self: ASFGitHubFeature, context: str | dict) -> int:
if isinstance(context, str):
return -1
app = context.get("app")
if isinstance(app, str):
app = __slug_get_app_id(self, app)
return -1 if app is None else int(app)


@directive
def branch_protection(self: ASFGitHubFeature):
# Branch protections
if "protected_branches" not in self.yaml:
return
# Testing mode
dry_run: bool = self.noop("protected_branches")

# Collect all branches and whether they have active branch protection rules
try:
refs = get_head_refs(self)
refs = get_head_refs(self) if not dry_run else []
except Exception as ex:
print(f"Error: failed to retrieve current refs: {ex!s}")
refs = []
Expand All @@ -115,15 +142,6 @@ def branch_protection(self: ASFGitHubFeature):
protected_branches.remove(branch)

branch_changes = []
try:
ghbranch = self.ghrepo.get_branch(branch=branch)
except pygithub.GithubException as e:
if e.status == 404: # No such branch, skip to next rule
protection_changes[branch] = [f"Branch {branch} does not exist, protection could not be configured"]
continue
else:
# propagate other errors, GitHub API might have an outage
raise e

# We explicitly disable force pushes when branch protections are enabled
allow_force_push = False
Expand Down Expand Up @@ -164,10 +182,7 @@ def branch_protection(self: ASFGitHubFeature):
require_strict = required_status_checks.get("strict", NotSet)

contexts = required_status_checks.get("contexts", [])
checks = required_status_checks.get("checks", [])
checks_as_dict = {**{ctx: -1 for ctx in contexts}, **{c["context"]: int(c["app_id"]) for c in checks}}

required_checks = list(checks_as_dict.items())
required_checks = list((__context_get_name(c), __context_get_app_id(self, c)) for c in contexts)

# if no checks are defined, we remove the status checks completely
if len(required_checks) == 0:
Expand All @@ -179,7 +194,9 @@ def branch_protection(self: ASFGitHubFeature):

# Log changes that will be applied
try:
live_branch_protection_settings = ghbranch.get_protection()
live_branch_protection_settings = (
self.ghrepo.get_branch(branch=branch).get_protection() if not dry_run else None
)
except pygithub.GithubException:
live_branch_protection_settings = None

Expand Down Expand Up @@ -254,7 +271,16 @@ def branch_protection(self: ASFGitHubFeature):
branch_changes.append(f" - {ctx} (app_id: {appid})")

# Apply all the changes
if not self.noop("protected_branches"):
if not dry_run:
try:
ghbranch = self.ghrepo.get_branch(branch=branch)
except pygithub.GithubException as e:
if e.status == 404: # No such branch, skip to next rule
protection_changes[branch] = [f"Branch {branch} does not exist, protection could not be configured"]
continue
else:
# propagate other errors, GitHub API might have an outage
raise e
branch_protection_settings = ghbranch.edit_protection(
allow_force_pushes=allow_force_push,
required_linear_history=required_linear,
Expand Down Expand Up @@ -292,16 +318,16 @@ def branch_protection(self: ASFGitHubFeature):

# remove branch protection from all remaining protected branches
for branch_name in protected_branches:
branch = self.ghrepo.get_branch(branch_name)
protection_changes[branch] = [f"Remove branch protection from branch '{branch_name}'"]
protection_changes[branch_name] = [f"Remove branch protection from branch '{branch_name}'"]

if not self.noop("github::protected_branches"):
if not dry_run:
branch = self.ghrepo.get_branch(branch_name)
branch.remove_protection()

if protection_changes:
summary = ""
for branch, changes in protection_changes.items():
summary += f"Updates to the {branch} branch:\n"
for branch_name, changes in protection_changes.items():
summary += f"Updates to the {branch_name} branch:\n"
for change in changes:
summary += f" - {change}\n"
print(summary)
63 changes: 63 additions & 0 deletions tests/github_branch_protection.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

"""Unit tests for .asf.yaml GitHub branch protection"""
from helpers import YamlTest
import asfyaml.asfyaml
import asfyaml.dataobjects

# Set .asf.yaml to debug mode
asfyaml.asfyaml.DEBUG = True


valid_github_checks = YamlTest(
None,
None,
"""
github:
protected_branches:
main:
required_status_checks:
contexts:
- "check1"
# Only slug
- context: "check2"
app: "github-actions"
# Only id
- context: "check3"
app: 15368
# Only context
- context: "check4"
""",
)


def test_basic_yaml(test_repo: asfyaml.dataobjects.Repository):
print("[github] Testing branch protection")

tests_to_run = (
valid_github_checks,
)

for test in tests_to_run:
with test.ctx():
a = asfyaml.asfyaml.ASFYamlInstance(
repo=test_repo, committer="humbedooh", config_data=test.yaml, branch=asfyaml.dataobjects.DEFAULT_BRANCH
)
a.environments_enabled.add("noop")
a.no_cache = True
a.run_parts()