Skip to content

Commit 9d0362f

Browse files
authored
170476 Implementation of PR base expiration validation on specific branch only (#4862)
Implemented logic to validate PR base expiration on specific branch only require configuration change from: ``` base_commit_allowed_days: 7 ``` to: ``` base_commit_expiration: branch: "main" allowed_days: 7 ``` Issue: flutter/flutter#170476
1 parent 5c70bac commit 9d0362f

File tree

7 files changed

+224
-66
lines changed

7 files changed

+224
-66
lines changed

app_dart/integration_test/data/cocoon_config.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,9 @@
160160
},
161161
{
162162
"name": "Linux ci_yaml roller",
163-
"backfill": false,
163+
"properties": {
164+
"backfill": "false"
165+
},
164166
"runIf": [
165167
".ci.yaml"
166168
],

auto_submit/lib/configuration/repository_configuration.dart

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ class RepositoryConfiguration {
2121
static const String supportNoReviewRevertKey = 'support_no_review_revert';
2222
static const String requiredCheckRunsOnRevertKey =
2323
'required_checkruns_on_revert';
24-
static const String baseCommitAllowedDaysKey = 'base_commit_allowed_days';
24+
static const String stalePrProtectionInDaysForBaseRefsKey =
25+
'stale_pr_protection_in_days_for_base_refs';
2526

2627
static const String defaultBranchStr = 'default';
2728

@@ -34,7 +35,7 @@ class RepositoryConfiguration {
3435
bool? runCi,
3536
bool? supportNoReviewReverts,
3637
Set<String>? requiredCheckRunsOnRevert,
37-
int? baseCommitAllowedDays,
38+
Map<String, int>? stalePrProtectionInDaysForBaseRefs,
3839
}) : allowConfigOverride = allowConfigOverride ?? false,
3940
defaultBranch = defaultBranch ?? defaultBranchStr,
4041
autoApprovalAccounts = autoApprovalAccounts ?? <String>{},
@@ -43,7 +44,8 @@ class RepositoryConfiguration {
4344
runCi = runCi ?? true,
4445
supportNoReviewReverts = supportNoReviewReverts ?? true,
4546
requiredCheckRunsOnRevert = requiredCheckRunsOnRevert ?? <String>{},
46-
baseCommitAllowedDays = baseCommitAllowedDays ?? 0;
47+
stalePrProtectionInDaysForBaseRefs =
48+
stalePrProtectionInDaysForBaseRefs ?? <String, int>{};
4749

4850
/// This flag allows the repository to override the org level configuration.
4951
bool allowConfigOverride;
@@ -74,9 +76,9 @@ class RepositoryConfiguration {
7476
/// merged.
7577
Set<String> requiredCheckRunsOnRevert;
7678

77-
/// The number of days that the base commit of the pull request can not be
78-
/// older than. If less than 1 then it will not be checked.
79-
int baseCommitAllowedDays;
79+
/// A map of [slug]/[branch] as a key and number of days to validate PR base
80+
/// date is not older than on that [slug]/[branch].
81+
Map<String, int> stalePrProtectionInDaysForBaseRefs;
8082

8183
@override
8284
String toString() {
@@ -95,7 +97,13 @@ class RepositoryConfiguration {
9597
for (var checkrun in requiredCheckRunsOnRevert) {
9698
stringBuffer.writeln(' - $checkrun');
9799
}
98-
stringBuffer.writeln('$baseCommitAllowedDaysKey: $baseCommitAllowedDays');
100+
if (stalePrProtectionInDaysForBaseRefs.isNotEmpty) {
101+
stringBuffer.writeln('$stalePrProtectionInDaysForBaseRefsKey:');
102+
for (final MapEntry(key: branch, value: days)
103+
in stalePrProtectionInDaysForBaseRefs.entries) {
104+
stringBuffer.writeln(' $branch: $days');
105+
}
106+
}
99107
return stringBuffer.toString();
100108
}
101109

@@ -124,6 +132,27 @@ class RepositoryConfiguration {
124132
}
125133
}
126134

135+
final stalePrProtectionInDaysForBaseRefs = <String, int>{};
136+
final yamlstalePrProtectionInDaysForBaseRefs =
137+
yamlDoc[stalePrProtectionInDaysForBaseRefsKey] as YamlMap?;
138+
if (yamlstalePrProtectionInDaysForBaseRefs != null) {
139+
for (var entry in yamlstalePrProtectionInDaysForBaseRefs.entries) {
140+
if (entry.value is! int) {
141+
throw ConfigurationException(
142+
'The value for ${entry.key} must be an integer.',
143+
);
144+
}
145+
if (entry.value as int <= 0) {
146+
throw ConfigurationException(
147+
'The value for ${entry.key} must be greater than zero.',
148+
);
149+
}
150+
151+
stalePrProtectionInDaysForBaseRefs[entry.key as String] =
152+
entry.value as int;
153+
}
154+
}
155+
127156
return RepositoryConfiguration(
128157
allowConfigOverride: yamlDoc[allowConfigOverrideKey] as bool?,
129158
defaultBranch: yamlDoc[defaultBranchKey] as String?,
@@ -133,7 +162,7 @@ class RepositoryConfiguration {
133162
runCi: yamlDoc[runCiKey] as bool?,
134163
supportNoReviewReverts: yamlDoc[supportNoReviewRevertKey] as bool?,
135164
requiredCheckRunsOnRevert: requiredCheckRunsOnRevert,
136-
baseCommitAllowedDays: yamlDoc[baseCommitAllowedDaysKey] as int?,
165+
stalePrProtectionInDaysForBaseRefs: stalePrProtectionInDaysForBaseRefs,
137166
);
138167
}
139168
}

auto_submit/lib/configuration/repository_configuration_manager.dart

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ class RepositoryConfigurationManager {
141141
runCi: globalConfiguration.runCi,
142142
supportNoReviewReverts: globalConfiguration.supportNoReviewReverts,
143143
requiredCheckRunsOnRevert: globalConfiguration.requiredCheckRunsOnRevert,
144-
baseCommitAllowedDays: globalConfiguration.baseCommitAllowedDays,
144+
stalePrProtectionInDaysForBaseRefs:
145+
globalConfiguration.stalePrProtectionInDaysForBaseRefs,
145146
);
146147

147148
// auto approval accounts, they should be empty if nothing was defined
@@ -187,13 +188,12 @@ class RepositoryConfigurationManager {
187188
);
188189
}
189190

190-
// if the local configuration is not the default value then use it.
191-
// Can be setted to negative value in local configuration to turn-off
192-
// base commit allowed days validation.
193-
final localBaseCommitAllowedDays = localConfiguration.baseCommitAllowedDays;
194-
if (localBaseCommitAllowedDays != 0) {
195-
mergedRepositoryConfiguration.baseCommitAllowedDays =
196-
localConfiguration.baseCommitAllowedDays;
191+
// if the local configuration is not empty then use it.
192+
final localstalePrProtectionInDaysForBaseRefs =
193+
localConfiguration.stalePrProtectionInDaysForBaseRefs;
194+
if (localstalePrProtectionInDaysForBaseRefs.isNotEmpty) {
195+
mergedRepositoryConfiguration.stalePrProtectionInDaysForBaseRefs =
196+
localConfiguration.stalePrProtectionInDaysForBaseRefs;
197197
}
198198

199199
return mergedRepositoryConfiguration;

auto_submit/lib/validations/base_commit_date_allowed.dart

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,31 @@ class BaseCommitDateAllowed extends Validation {
2929
slug,
3030
);
3131

32-
// If the base commit allowed days is less than or equal to 0 then the
33-
// validation is turned off and the base commit creation date is ignored.
34-
if (repositoryConfiguration.baseCommitAllowedDays <= 0) {
32+
// If the base_commit_expiration is empty then the validation is turned off
33+
// and the base commit creation date is ignored.
34+
if (repositoryConfiguration.stalePrProtectionInDaysForBaseRefs.isEmpty) {
35+
log.info('PR base commit creation date validation turned off');
36+
return ValidationResult(
37+
true,
38+
Action.IGNORE_FAILURE,
39+
'PR base commit creation date validation turned off',
40+
);
41+
}
42+
43+
// Check if PR base expiration validation is configured for this branch.
44+
if (!repositoryConfiguration.stalePrProtectionInDaysForBaseRefs.containsKey(
45+
'${slug.fullName}/${messagePullRequest.base!.ref}',
46+
)) {
3547
log.info(
36-
'PR base commit creation date validation turned off by setting '
37-
'base_commit_allowed_days to ${repositoryConfiguration.baseCommitAllowedDays}.',
48+
'PR ${slug.fullName}/${messagePullRequest.number} is for branch: '
49+
'${messagePullRequest.base!.ref} which does not match any configured PR '
50+
'base for stale protection',
3851
);
3952
return ValidationResult(
4053
true,
4154
Action.IGNORE_FAILURE,
42-
'PR base commit creation date validation turned off by setting '
43-
'base_commit_allowed_days to ${repositoryConfiguration.baseCommitAllowedDays}.',
55+
'The base commit expiration validation is not configured for this '
56+
'branch.',
4457
);
4558
}
4659

@@ -61,23 +74,26 @@ class BaseCommitDateAllowed extends Validation {
6174
}
6275

6376
log.info(
64-
'PR ${slug.fullName}/${messagePullRequest.number} base creation date is: '
65-
'${commit.commit?.author?.date}. Allowed age is '
66-
'${repositoryConfiguration.baseCommitAllowedDays} days.',
77+
'PR ${slug.fullName}/${messagePullRequest.number} requested for branch: '
78+
'${messagePullRequest.base!.ref} with base creation date: '
79+
'${commit.commit?.author?.date}.',
6780
);
6881

6982
final isBaseRecent = commit.commit!.author!.date!.isAfter(
7083
DateTime.now().subtract(
71-
Duration(days: repositoryConfiguration.baseCommitAllowedDays),
84+
Duration(
85+
days: repositoryConfiguration
86+
.stalePrProtectionInDaysForBaseRefs['${slug.fullName}/${messagePullRequest.base!.ref}']!,
87+
),
7288
),
7389
);
7490

7591
final message = isBaseRecent
7692
? 'The base commit of the PR is recent enough for merging.'
7793
: 'The base commit of the PR is older than '
78-
'${repositoryConfiguration.baseCommitAllowedDays} days and can not '
79-
'be merged. Please merge the latest changes from the main into '
80-
'this branch and resubmit the PR.';
94+
'${repositoryConfiguration.stalePrProtectionInDaysForBaseRefs['${slug.fullName}/${messagePullRequest.base!.ref}']} '
95+
'days and can not be merged. Please merge the latest changes '
96+
'from the main into this branch and resubmit the PR.';
8197

8298
return ValidationResult(isBaseRecent, Action.REMOVE_LABEL, message);
8399
}

auto_submit/test/configuration/repository_configuration_data.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,7 @@ const String sampleConfigWithOverride = '''
4545
support_no_review_revert: true
4646
required_checkruns_on_revert:
4747
- ci.yaml validation
48-
base_commit_allowed_days: 7
48+
stale_pr_protection_in_days_for_base_refs:
49+
flutter/flutter/main: 7
50+
flutter/cocoon/main: 30
4951
''';

0 commit comments

Comments
 (0)