-
Notifications
You must be signed in to change notification settings - Fork 1
/
dangerfile.py
509 lines (471 loc) · 20.4 KB
/
dangerfile.py
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
############################################################################
# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
#
# SPDX-License-Identifier: MPL-2.0
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, you can obtain one at https://mozilla.org/MPL/2.0/.
#
# See the COPYRIGHT file distributed with this work for additional
# information regarding copyright ownership.
############################################################################
import glob
import os
import re
import gitlab
# Helper functions and variables
def added_lines(target_branch, paths):
import subprocess
# Hazard fetches the target branch itself, so there is no need to fetch it
# explicitly using `git fetch --depth 1000 origin <target_branch>`. The
# refs/remotes/origin/<target_branch> ref is also expected to be readily
# usable by the time this file is executed.
diff = subprocess.check_output(
["/usr/bin/git", "diff", f"origin/{target_branch}...", "--"] + paths
)
added_lines = []
for line in diff.splitlines():
if line.startswith(b"+") and not line.startswith(b"+++"):
added_lines.append(line)
return added_lines
def lines_containing(lines, string):
return [l for l in lines if bytes(string, "utf-8") in l]
changes_issue_or_mr_id_regex = re.compile(rb"\[(GL [#!]|RT #)[0-9]+\]")
rdata_regex = re.compile(r"lib/dns/rdata/")
mr_issue_link_regex = re.compile(r"^(Closes|Fixes):?\s*[^\n]*#[0-9]+", re.MULTILINE)
modified_files = danger.git.modified_files
affected_files = (
danger.git.modified_files + danger.git.created_files + danger.git.deleted_files
)
mr_title = re.sub(r"^Draft:\s*", r"", danger.gitlab.mr.title)
mr_labels = danger.gitlab.mr.labels
source_branch = danger.gitlab.mr.source_branch
target_branch = danger.gitlab.mr.target_branch
is_backport = "Backport" in mr_labels or "Backport::Partial" in mr_labels
is_full_backport = is_backport and "Backport::Partial" not in mr_labels
gl = gitlab.Gitlab(
url=f"https://{os.environ['CI_SERVER_HOST']}",
private_token=os.environ["BIND_TEAM_API_TOKEN"],
)
proj = gl.projects.get(os.environ["CI_PROJECT_ID"])
mr = proj.mergerequests.get(os.environ["CI_MERGE_REQUEST_IID"])
###############################################################################
# MERGE REQUEST INFORMATION
###############################################################################
#
# - FAIL if the MR title doesn't have the expected format
#
# - FAIL if the MR title doesn't contain changelog action
#
# - FAIL if the MR title doesn't contain changelog audience
MR_TITLE_RE = re.compile(
r"^(\[9\.[0-9]{2}(-S)?\])?\s*(\[[^]]*\]\s*)?((chg|fix|new|rem|sec):)?\s*((dev|usr|pkg|doc|test|ci|nil)\s*:)?\s*([^\n]*)$",
)
mr_title_match = MR_TITLE_RE.match(mr_title)
mr_title_cve = mr_title_match.group(3) if mr_title_match else None
mr_title_action = mr_title_match.group(5) if mr_title_match else None
mr_title_audience = mr_title_match.group(7) if mr_title_match else None
if not mr_title_match:
fail("Merge request's title is invalid. Fix it or contact QA for assistance.")
else:
if mr_title_action is None:
fail(
"Add one of `chg:`|`fix:`|`new:`|`rem:`|`sec:` to the MR title to [categorize this change](https://gitlab.isc.org/isc-projects/bind9/-/wikis/BIND-development-workflow#mr-title)."
)
if mr_title_audience is None:
fail(
"Add one of `dev:`|`usr:`|`pkg:`|`doc:`|`test:`|`ci:`|`nil:` to the MR title as [intended audience](https://gitlab.isc.org/isc-projects/bind9/-/wikis/BIND-development-workflow#mr-title)."
)
###############################################################################
# BRANCH NAME
###############################################################################
#
# - FAIL if the source branch of the merge request includes an old-style
# "-v9_x" or "-v9.x" suffix.
branch_name_regex = r"^(?P<base>.*?)(?P<suffix>-v9[_.](?P<version>[0-9]+))$"
match = re.match(branch_name_regex, source_branch)
if match:
fail(
f"Source branch name `{source_branch}` includes an old-style version "
f"suffix (`{match.group('suffix')}`). Using such suffixes is now "
"deprecated. Please resubmit the merge request with the branch name "
f"set to `{match.group('base')}-bind-9.{match.group('version')}`."
)
###############################################################################
# COMMIT MESSAGES
###############################################################################
#
# - FAIL if any of the following is true for any commit on the MR branch:
#
# * The subject line starts with "fixup!", "amend!" or "Apply suggestion".
#
# * The subject line starts with a prohibited word indicating a work in
# progress commit (e.g. "WIP").
#
# * The subject line contains a changelog action.
#
# * The subject line contains a trailing dot.
#
# * There is no empty line between the subject line and the log message.
#
# - WARN if any of the following is true for any commit on the MR branch:
#
# * The length of the subject line for a non-merge commit exceeds 72
# characters.
#
# * There is no log message present (i.e. commit only has a subject) and
# the subject line does not contain any of the following strings:
# "fixup!", " CHANGES ", " release note".
#
# * Any line of the log message is longer than 72 characters. This rule is
# not evaluated for:
#
# - lines starting with four spaces, which allows long lines to be
# included in the commit log message by prefixing them with four
# spaces (useful for pasting compiler warnings, static analyzer
# messages, log lines, etc.),
#
# - lines which contain references (i.e. those starting with "[1]",
# "[2]", etc.) which allows e.g. long URLs to be included in the
# commit log message.
PROHIBITED_WORDS_RE = re.compile(
"^(WIP|wip|DROP|drop|DROPME|checkpoint|experiment|TODO|todo)[^a-zA-Z]"
)
fixup_error_logged = False
for commit in danger.git.commits:
message_lines = commit.message.splitlines()
subject = message_lines[0]
is_merge = len(commit.parents) >= 2
is_fixup = (
subject.startswith("fixup!")
or subject.startswith("amend!")
or subject.startswith("Apply suggestion")
)
if not fixup_error_logged and is_fixup:
fail(
"Fixup commits are still present in this merge request. "
"Please squash them before merging."
)
fixup_error_logged = True
match = PROHIBITED_WORDS_RE.search(subject)
if match:
fail(
f"Prohibited keyword `{match.groups()[0]}` detected "
f"at the start of a subject line in commit {commit.sha}."
)
match = MR_TITLE_RE.match(subject)
if match and match.group(5) is not None and not is_merge:
fail(
f"Changelog action `{match.group(5)}` detected in non-merge"
f"commit {commit.sha}. Use MR title instead."
)
if len(subject) > 72 and not is_merge and not is_fixup:
warn(
f"Subject line for commit {commit.sha} is too long: "
f"```{subject}``` ({len(subject)} > 72 characters)."
)
if subject[-1] == ".":
fail(f"Trailing dot found in the subject of commit {commit.sha}.")
if len(message_lines) > 1 and message_lines[1]:
fail(f"No empty line after subject for commit {commit.sha}.")
if (
len(message_lines) < 3
and "fixup! " not in subject
and "CHANGES " not in subject
and "release note" not in subject.lower()
and "GL #" not in subject
):
warn(f"Please write a log message for commit {commit.sha}.")
for line in message_lines[2:]:
if (
len(line) > 72
and not line.startswith(" ")
and not re.match(r"\[[0-9]+\]", line)
):
warn(
f"Line too long in log message for commit {commit.sha}: "
f"```{line}``` ({len(line)} > 72 characters)."
)
###############################################################################
# BACKPORT & VERSION LABELS
###############################################################################
#
# FAIL if any of the following is true for the merge request:
#
# * The MR has any "Affects v9.x" label(s) set. These should only be used for
# issues.
#
# * The MR is marked as Backport and the number of version labels set is
# different than 1. (For backports, the version label is used for indicating
# its target branch. This is a rather ugly attempt to address a UI
# deficiency - the target branch for each MR is not visible on milestone
# dashboards.)
#
# * The MR is not marked as "Backport" nor any version label is set. (If the
# merge request is not a backport, version labels are used for indicating
# backporting preferences.)
#
# * The Backport MR doesn't have target branch in the merge request title.
#
# * The Backport MR doesn't link to the original MR is its description.
#
# * The original MR linked to from Backport MR hasn't been merged.
BACKPORT_OF_RE = re.compile(
r"Backport\s+of.*(merge_requests/|!)([0-9]+)", flags=re.IGNORECASE
)
VERSION_LABEL_RE = re.compile(r"v9.([0-9]+)(-S)?")
version_labels = [l for l in mr_labels if l.startswith("v9.")]
affects_labels = [l for l in mr_labels if l.startswith("Affects v9.")]
if affects_labels:
fail(
"This MR is marked with at least one *Affects v9.x* label. "
"Please remove them as they should only be used for issues."
)
if is_backport:
if len(version_labels) != 1:
fail(
"This MR was marked as *Backport*. "
"Please also set exactly one version label (*v9.x*)."
)
else:
minor_ver, edition = VERSION_LABEL_RE.search(version_labels[0]).groups()
edition = "" if edition is None else edition
title_re = f"^\\[9.{minor_ver}{edition}\\]"
match = re.search(title_re, mr_title)
if match is None:
fail(
"Backport MRs must have their target version in the title. "
f"Please put `[9.{minor_ver}{edition}]` at the start of the MR title."
)
backport_desc = BACKPORT_OF_RE.search(danger.gitlab.mr.description or "")
if backport_desc is None:
fail(
"Backport MRs must link to the original MR. Please put "
"`Backport of MR !XXXX` in the MR description."
)
else: # backport MR is linked to original MR
original_mr_id = backport_desc.groups()[1]
original_mr = proj.mergerequests.get(original_mr_id)
if original_mr.state != "merged":
fail(
f"Original MR !{original_mr_id} has not been merged. "
"Please re-run `danger` check once it's merged."
)
else: # check for commit IDs once original MR is merged
original_mr_commits = list(original_mr.commits(all=True))
backport_mr_commits = list(mr.commits(all=True))
for orig_commit in original_mr_commits:
for backport_commit in backport_mr_commits:
if orig_commit.id in backport_commit.message:
break
else:
msg = (
f"Commit {orig_commit.id} from original MR !{original_mr_id} "
"is not referenced in any of the backport commits."
)
if not is_full_backport:
message(msg)
else:
msg += (
" Please use `-x` when cherry-picking to include "
"the full original commit ID. Alternately, use the "
"`Backport::Partial` label if not all original "
"commits are meant to be backported."
)
fail(msg)
else:
if not version_labels:
fail(
"If this merge request is a backport, set the *Backport* label and "
"a single version label (*v9.x*) indicating the target branch. "
"If not, set version labels for all targeted backport branches."
)
###############################################################################
# OTHER LABELS
###############################################################################
#
# WARN if any of the following is true for the merge request:
#
# * The "Review" label is not set. (It may be intentional, but rarely is.)
#
# * The "Review" label is set, but the "LGTM" label is not set. (This aims to
# remind developers about the need to set the latter on merge requests which
# passed review.)
approved = mr.approvals.get().approved
if "Review" not in mr_labels:
warn(
"This merge request does not have the *Review* label set. "
"Please set it if you would like the merge request to be reviewed."
)
elif not approved:
warn(
"This merge request is currently in review. "
"It should not be merged until it is approved."
)
###############################################################################
# RELEASE NOTES
###############################################################################
#
# - FAIL if any of the following is true:
#
# * The merge request does not update release notes and has the "Release
# Notes" label set. (This attempts to point out missing release notes.)
#
# * The merge request updates release notes but does not have the "Release
# Notes" label set. (This ensures that merge requests updating release
# notes can be easily found using the "Release Notes" label.)
#
# * A file was added to or deleted from the lib/dns/rdata/ subdirectory but
# release notes were not modified. This is probably a mistake because new
# RR types are a user-visible change (and so is removing support for
# existing ones).
#
# * "Release notes" and "No CHANGES" labels are both set at the same time.
# (If something is worth a release note, it should surely show up in
# CHANGES.) MRs with certain labels set ("Documentation", "Release") are
# exempt because these are typically used during release process.
#
# - WARN if any of the following is true:
#
# * This merge request does not update release notes and has the "Customer"
# label set. (Except for trivial changes, all merge requests which may
# be of interest to customers should include a release note.)
#
# * This merge request updates release notes, but no GitLab issue was
# linked with the `Closes` or `Fixes` keyword in the MR description.
release_notes_changed = mr_title_audience in ["usr", "pkg"]
if not release_notes_changed:
if "Customer" in mr_labels and not release_notes_changed:
warn(
"This merge request has the *Customer* label set. "
"Update the MR title to include `usr:`|`pkg:` audience "
"unless the changes introduced are trivial."
)
rdata_types_add_rm = list(
filter(rdata_regex.match, danger.git.created_files + danger.git.deleted_files)
)
if rdata_types_add_rm:
fail(
"This merge request adds new files to `lib/dns/rdata/` and/or "
"deletes existing files from that directory, which almost certainly "
"means that it adds support for a new RR type or removes support "
"for an existing one. Update the MR title to include `usr:` audience."
)
if release_notes_changed and not mr_issue_link_regex.search(
danger.gitlab.mr.description
):
warn("No issue was linked via `Closes`|`Fixes` in the MR description.")
###############################################################################
# CVE IDENTIFIERS
###############################################################################
#
# WARN if the merge request title indicates a security issue, but there is no
# CVE identifier in the MR title.
if mr_title_action == "sec" and (mr_title_cve is None or "CVE-20" not in mr_title_cve):
warn(
"This merge request fixes a security issue. "
"Please add `[CVE-XXXX-YYYY]` to the MR title if a CVE was issued."
)
###############################################################################
# PAIRWISE TESTING
###############################################################################
#
# FAIL if the merge request adds any new ./configure switch without an
# associated annotation used for pairwise testing.
configure_added_lines = added_lines(target_branch, ["configure.ac"])
switches_added = lines_containing(
configure_added_lines, "AC_ARG_ENABLE"
) + lines_containing(configure_added_lines, "AC_ARG_WITH")
annotations_added = lines_containing(configure_added_lines, "# [pairwise: ")
if switches_added:
if len(switches_added) > len(annotations_added):
fail(
"This merge request adds at least one new `./configure` switch that "
"is not annotated for pairwise testing purposes."
)
else:
message(
"**Before merging**, please start a full CI pipeline for this "
"branch with the `PAIRWISE_TESTING` variable set to any "
"non-empty value (e.g. `1`). This will cause the `pairwise` "
"job to exercise the new `./configure` switches."
)
###############################################################################
# PRE-RELEASE TESTING
###############################################################################
#
# WARN if the merge request is marked with the "Security" label, but not with
# the label used for marking merge requests for pre-release testing (if the
# latter is defined by the relevant environment variable).
pre_release_testing_label = os.getenv("PRE_RELEASE_TESTING_LABEL")
if (
pre_release_testing_label
and "Security" in mr_labels
and pre_release_testing_label not in mr_labels
):
warn(
"This merge request is marked with the *Security* label, but it is not "
f"marked for pre-release testing (*{pre_release_testing_label}*)."
)
###############################################################################
# USER-VISIBLE LOG LEVELS
###############################################################################
#
# WARN if the merge request adds new user-visible log messages (INFO or above)
user_visible_log_levels = [
"ISC_LOG_INFO",
"ISC_LOG_NOTICE",
"ISC_LOG_WARNING",
"ISC_LOG_ERROR",
"ISC_LOG_CRITICAL",
]
source_added_lines = added_lines(target_branch, ["*.[ch]"])
for log_level in user_visible_log_levels:
if lines_containing(source_added_lines, log_level):
warn(
"This merge request adds new user-visible log messages with "
"level INFO or above. Please double-check log levels and make "
"sure none of the messages added is a leftover debug message."
)
break
###############################################################################
# SYSTEM TEST FILES
###############################################################################
#
# FAIL if newly added system test directory contains an underscore (invalid char)
# FAIL if there are no pytest files in the system test directory
# FAIL if the pytest glue file for tests.sh is missing
TESTNAME_CANDIDATE_RE = re.compile(r"bin/tests/system/([^/]+)")
testnames = set()
for path in affected_files:
match = TESTNAME_CANDIDATE_RE.search(path)
if match is not None:
testnames.add(match.groups()[0])
for testname in testnames:
dirpath = f"bin/tests/system/{testname}"
if (
not os.path.isdir(dirpath)
or testname.startswith(".")
or testname.startswith("_")
or testname == "isctest"
):
continue
if "_" in testname:
fail(
f"System test directory `{testname}` may not contain an underscore, "
"use hyphen instead."
)
if not glob.glob(f"{dirpath}/**/tests_*.py", recursive=True):
fail(
f"System test directory `{testname}` doesn't contain any "
"`tests_*.py` pytest file."
)
tests_sh_exists = os.path.exists(f"{dirpath}/tests.sh")
glue_file_name = f"tests_sh_{testname.replace('-', '_')}.py"
tests_sh_py_exists = os.path.exists(f"{dirpath}/{glue_file_name}")
if tests_sh_exists and not tests_sh_py_exists:
fail(
f"System test directory `{testname}` is missing the "
f"`{glue_file_name}` pytest glue file."
)