zuul/zuul
Code Issues Proposed changes

Merge "Add some github configuration deprecations"

Browse Source
This commit is contained in:
Zuul 2024-03-01 18:54:10 +00:00 committed by Gerrit Code Review
commit 3d30928d39
10 changed files with 300 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
doc/source/drivers/github.rst
View File

@ -292,6 +292,13 @@ the following options.
.. attr:: event
:required:
.. warning:: While it is currently possible to list more than
one event in a single trigger, that is deprecated
and support will be removed in a future version.
Instead, specify a single event type per trigger,
and list multiple triggers as necessary to cover
all intended events.
The event from github. Supported events are:
.. value:: pull_request
@ -359,7 +366,17 @@ the following options.
.. value:: requested
A check run is requested.
.. warning:: This is deprecated and will be removed in a
future version. Use
:value:`pipeline.trigger.<github
source>.action.rerequested` instead.
Deprecated alias for :value:`pipeline.trigger.<github
source>.action.rerequested`.
.. value:: rerequested
A check run is rerequested.
.. value:: completed
@ -400,7 +417,7 @@ the following options.
.. attr:: status
This is used for ``pull-request`` and ``status`` actions. It
This is used for ``pull_request`` and ``status`` actions. It
accepts a list of strings each of which matches the user setting
the status, the status context, and the status itself in the
format of ``user:context:status``. For example,

15
releasenotes/notes/github-trigger-deprecations-067ba9feab4c0a76.yaml Normal file
View File

@ -0,0 +1,15 @@
deprecations:
- |
Specifying :attr:`pipeline.trigger.<github source>.event` as a
list is deprecated. Update any instances to specify multiple
triggers each with a single associated event instead.
- |
The use of :value:`pipeline.trigger.<github
source>.action.requested` has been deprecated for some time
already. Its use will now produce a configuration syntax warning,
and in a future version of Zuul, an error.
- |
The use of :attr:`pipeline.trigger.<github source>.require-status`
has been deprecated for some already. Its use will now produce a
configuration syntax warning, and in a future version of Zuul, an
error.

2
tests/fixtures/layouts/github-merge-mode.yaml vendored
View File

@ -5,7 +5,7 @@
github:
- event: pull_request_review
action: submitted
state: approve
state: approved
- event: pull_request
action:
- opened

2
tests/fixtures/layouts/github-message-update.yaml vendored
View File

@ -5,7 +5,7 @@
github:
- event: pull_request_review
action: submitted
state: approve
state: approved
start:
github: {}
success:

43
tests/fixtures/layouts/github-schema.yaml vendored Normal file
View File

@ -0,0 +1,43 @@
- pipeline:
name: check
manager: independent
trigger:
github:
# Valid
- event: pull_request
action: comment
comment: test me
# Valid
- event: pull_request
action:
- opened
- closed
# "extra keys not allowed @ data['check']"
- event: pull_request
action:
- opened
- closed
branch: ^master$
check: foo
# "extra keys not allowed @ data['branch']"
- event: check_run
branch: ^master$
# "as a list is deprecated"
- event:
- pull_request
- check_run
# "'require-status' trigger attribute"
# "extra keys not allowed @ data['require-status']"
- event: check_run
require-status: foo
# "'unlabel' trigger attribute"
# "extra keys not allowed @ data['unlabel']"
- event: check_run
unlabel: foo
# "Use 'rerequested' instead"
- event: check_run
action: requested
success:
github: {}
failure:
github: {}

4
tests/fixtures/layouts/pcre-deprecation-github.yaml vendored
View File

@ -3,9 +3,7 @@
manager: independent
trigger:
github:
- event: pull_request_review
action: submitted
state: approve
- event: push
ref: '(?!invalid)'
start:
github: {}

2
tests/fixtures/layouts/reviews-github.yaml vendored
View File

@ -5,7 +5,7 @@
github:
- event: pull_request_review
action: submitted
state: approve
state: approved
success:
github:
label:

2
tests/unit/test_github_crd.py
View File

@ -195,7 +195,7 @@ class TestGithubCrossRepoDeps(ZuulTestCase):
self.executor_server.hold_jobs_in_build = True
# Enqueue A,B,C
self.fake_github.emitEvent(C.getReviewAddedEvent('approve'))
self.fake_github.emitEvent(C.getReviewAddedEvent('approved'))
self.waitUntilSettled()
self.assertEqual(len(self.builds), 1)

39
tests/unit/test_github_driver.py
View File

@ -375,7 +375,7 @@ class TestGithubDriver(ZuulTestCase):
@simple_layout('layouts/reviews-github.yaml', driver='github')
def test_reviews(self):
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getReviewAddedEvent('approve'))
self.fake_github.emitEvent(A.getReviewAddedEvent('approved'))
self.waitUntilSettled()
self.assertEqual(1, len(self.history))
self.assertEqual('project-reviews', self.history[0].name)
@ -383,7 +383,7 @@ class TestGithubDriver(ZuulTestCase):
# test_review_unmatched_event
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
self.fake_github.emitEvent(B.getReviewAddedEvent('comment'))
self.fake_github.emitEvent(B.getReviewAddedEvent('commented'))
self.waitUntilSettled()
self.assertEqual(1, len(self.history))
@ -2790,3 +2790,38 @@ class TestGithubDefaultBranch(ZuulTestCase):
self.assertEqual('foobar', md.default_branch)
new_layout = layout.uuid
self.assertNotEqual(new_layout, prev_layout)
class TestGithubSchemaWarnings(ZuulTestCase):
config_file = 'zuul-github-driver.conf'
@simple_layout('layouts/github-schema.yaml', driver='github')
def test_broken_config_on_startup_warnings(self):
tenant = self.scheds.first.sched.abide.tenants.get('tenant-one')
self.assertEquals(
len(tenant.layout.loading_errors), 8,
"An error should have been stored")
self.assertIn(
"extra keys not allowed @ data['check']",
str(tenant.layout.loading_errors[0].error))
self.assertIn(
"extra keys not allowed @ data['branch']",
str(tenant.layout.loading_errors[1].error))
self.assertIn(
"as a list is deprecated",
str(tenant.layout.loading_errors[2].error))
self.assertIn(
"'require-status' trigger attribute",
str(tenant.layout.loading_errors[3].error))
self.assertIn(
"extra keys not allowed @ data['require-status']",
str(tenant.layout.loading_errors[4].error))
self.assertIn(
"'unlabel' trigger attribute",
str(tenant.layout.loading_errors[5].error))
self.assertIn(
"extra keys not allowed @ data['unlabel']",
str(tenant.layout.loading_errors[6].error))
self.assertIn(
"Use 'rerequested' instead",
str(tenant.layout.loading_errors[7].error))

204
zuul/driver/github/githubtrigger.py
View File

@ -14,34 +14,78 @@
# under the License.
import logging
import voluptuous as v
import voluptuous as vs
from zuul.trigger import BaseTrigger
from zuul.driver.github.githubmodel import GithubEventFilter
from zuul.driver.github import githubsource
from zuul.driver.util import scalar_or_list, to_list, make_regex, ZUUL_REGEX
from zuul.configloader import DeprecationWarning
class GithubUnlabelDeprecation(DeprecationWarning):
zuul_error_name = 'Github unlabel Deprecation'
zuul_error_message = """The 'unlabel' trigger attribute
is deprecated. Use 'label' instead."""
class GithubEventListDeprecation(DeprecationWarning):
zuul_error_name = 'Github event list Deprecation'
zuul_error_message = """Specifying the 'event' trigger attribute
as a list is deprecated. Use a single item instead."""
class GithubRequireStatusDeprecation(DeprecationWarning):
zuul_error_name = 'Github require-status Deprecation'
zuul_error_message = """The 'require-status' trigger attribute
is deprecated. Use 'require' instead."""
class GithubRequestedActionDeprecation(DeprecationWarning):
zuul_error_name = 'Github requested action Deprecation'
zuul_error_message = """The 'requested' value for the 'action'
trigger attribute is deprecated. Use 'rerequested' instead."""
class GithubTriggerConfigurationWarning(DeprecationWarning):
zuul_error_name = 'Github Trigger Configuration Warning'
class GithubTrigger(BaseTrigger):
name = 'github'
log = logging.getLogger("zuul.trigger.GithubTrigger")
def __init__(self, driver, connection, config=None):
# This is a compatibility layer to map the action 'requested' back
# to the original action 'rerequested'.
# TODO: Remove after zuul 5.0
for item in config:
if item.get('action') == 'requested':
item['action'] = 'rerequested'
super().__init__(driver, connection, config=config)
def getEventFilters(self, connection_name, trigger_config,
parse_context):
efilters = []
pcontext = parse_context
new_schema = getNewSchema()
for trigger in to_list(trigger_config):
# Deprecated in 8.3.0 but warning added in 10.0
if 'require-status' in trigger:
with pcontext.confAttr(trigger, 'require-status'):
pcontext.accumulator.addError(
GithubRequireStatusDeprecation())
# Deprecated with warning in 10.0
if 'unlabel' in trigger:
with pcontext.confAttr(trigger, 'unlabel'):
pcontext.accumulator.addError(
GithubUnlabelDeprecation())
# Deprecated with warning in 10.0
if isinstance(trigger.get('event', None), list):
with pcontext.confAttr(trigger, 'event'):
pcontext.accumulator.addError(
GithubEventListDeprecation())
try:
new_schema(trigger)
except vs.Invalid as e:
pcontext.accumulator.addError(
GithubTriggerConfigurationWarning(
"The trigger configuration as supplied "
f"has an error: {e}"))
with pcontext.confAttr(trigger, 'event') as attr:
types = [make_regex(x, pcontext)
for x in to_list(attr)]
@ -55,6 +99,18 @@ class GithubTrigger(BaseTrigger):
comments = [make_regex(x, pcontext)
for x in to_list(attr)]
# This is a compatibility layer to map the action 'requested' back
# to the original action 'rerequested'.
# TODO: Remove after zuul 5.0
# Note the original backwards compat handling for this did
# not allow 'requested' as a list, so we don't do that
# here either.
if trigger.get('action') == 'requested':
trigger['action'] = 'rerequested'
with pcontext.confAttr(trigger, 'action'):
pcontext.accumulator.addError(
GithubRequestedActionDeprecation())
f = GithubEventFilter(
connection_name=connection_name,
trigger=self,
@ -80,17 +136,119 @@ class GithubTrigger(BaseTrigger):
pass
def getNewSchema():
# For now, this is only used to raise deprecation errors if the
# syntax does not match. This was added in 10.0. When we're
# ready to enforce this, rename this method getSchema.
base_schema = vs.Schema({
vs.Required('event'): vs.Any('pull_request',
'pull_request_review',
'push',
'check_run'),
'require': githubsource.getRequireSchema(),
'reject': githubsource.getRejectSchema(),
})
# Pull request
pull_request_base_schema = base_schema.extend({
vs.Required('event'): 'pull_request',
'branch': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
})
pull_request_default_schema = pull_request_base_schema.extend({
'action': scalar_or_list(vs.Any(
'opened', 'changed', 'closed', 'reopened')),
})
pull_request_comment_schema = pull_request_base_schema.extend({
vs.Required('action'): 'comment',
'comment': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
})
pull_request_labeled_schema = pull_request_base_schema.extend({
vs.Required('action'): scalar_or_list(vs.Any('labeled', 'unlabeled')),
'label': scalar_or_list(str),
})
pull_request_status_schema = pull_request_base_schema.extend({
vs.Required('action'): 'status',
'status': scalar_or_list(str),
})
# Pull request review
pull_request_review_base_schema = base_schema.extend({
vs.Required('event'): 'pull_request_review',
'branch': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
})
pull_request_review_schema = pull_request_review_base_schema.extend({
'action': scalar_or_list(vs.Any('submitted', 'dismissed')),
'state': scalar_or_list(vs.Any(
'approved', 'commented', 'changes_requested',
'dismissed', 'pending')),
})
# Push
push_schema = base_schema.extend({
vs.Required('event'): 'push',
'ref': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
})
# Check run
check_run_schema = base_schema.extend({
vs.Required('event'): 'check_run',
'action': scalar_or_list(vs.Any('requested', 'completed')),
'check': scalar_or_list(str),
})
init_schema = base_schema.extend({}, extra=vs.ALLOW_EXTRA)
def validate(data):
# This method could be a simple vs.Any() across all the
# possibilities, but sometimes the error messages are less
# intuitive (e.g., indicating that the action is wrong rather
# than that a user has added an attribute that is incompatible
# with the action). Instead, we assume the user got the event
# and/or action right first, then apply the appropriate schema
# for what they selected.
event = data.get('event')
# TODO: run this unconditionally after
# GithubEventListDeprecation is complete
if isinstance(event, str):
# First, make sure the common items are correct.
init_schema(data)
action = data.get('action')
if event == 'pull_request':
if action == 'comment':
pull_request_comment_schema(data)
elif action == 'labeled':
pull_request_labeled_schema(data)
elif action == 'status':
pull_request_status_schema(data)
else:
pull_request_default_schema(data)
elif event == 'pull_request_review':
pull_request_review_schema(data)
elif event == 'push':
push_schema(data)
elif event == 'check_run':
check_run_schema(data)
return validate
def getSchema():
github_trigger = {
v.Required('event'):
scalar_or_list(v.Any('pull_request',
'pull_request_review',
'push',
'check_run')),
old_schema = vs.Schema({
vs.Required('event'):
scalar_or_list(vs.Any('pull_request',
'pull_request_review',
'push',
'check_run')),
'action': scalar_or_list(str),
'branch': scalar_or_list(v.Any(ZUUL_REGEX, str)),
'ref': scalar_or_list(v.Any(ZUUL_REGEX, str)),
'comment': scalar_or_list(v.Any(ZUUL_REGEX, str)),
'branch': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
'ref': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
'comment': scalar_or_list(vs.Any(ZUUL_REGEX, str)),
'label': scalar_or_list(str),
'unlabel': scalar_or_list(str),
'state': scalar_or_list(str),
@ -99,6 +257,6 @@ def getSchema():
'reject': githubsource.getRejectSchema(),
'status': scalar_or_list(str),
'check': scalar_or_list(str),
}
})
return github_trigger
return old_schema