Skip to content

Commit

Permalink
[codesign] Add additional retries on notarytool commands with exponen…
Browse files Browse the repository at this point in the history 
…tial backoff (#3398)

Fixes flutter/flutter#138636

* Migrate to package:retry instead of hand writing the retry logic
  * Enables easily adding exponential backoff
* Minor cleanup
  • Loading branch information
Casey Hillers authored Jan 5, 2024
1 parent beb0768 commit f7bff76
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 116 deletions.
123 changes: 56 additions & 67 deletions cipd_packages/codesign/lib/src/file_codesign_visitor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import 'dart:async';
import 'dart:io' as io;

import 'package:file/file.dart';
import 'package:meta/meta.dart';
import 'package:process/process.dart';
import 'package:retry/retry.dart';

import 'log.dart';
import 'utils.dart';
Expand Down Expand Up @@ -34,6 +36,10 @@ class FileCodesignVisitor {
required this.codesignAppstoreIDFilePath,
required this.codesignTeamIDFilePath,
this.dryrun = true,
@visibleForTesting this.retryOptions = const RetryOptions(
maxAttempts: 5,
delayFactor: Duration(seconds: 2),
),
this.notarizationTimerDuration = const Duration(seconds: 5),
}) {
entitlementsFile = rootDirectory.childFile('Entitlements.plist')..writeAsStringSync(_entitlementsFileContents);
Expand All @@ -54,6 +60,7 @@ class FileCodesignVisitor {
final String codesignTeamIDFilePath;
final bool dryrun;
final Duration notarizationTimerDuration;
final RetryOptions retryOptions;

// 'Apple developer account email used for authentication with notary service.'
late String codesignAppstoreId;
Expand Down Expand Up @@ -287,8 +294,6 @@ update these file paths accordingly.
Future<void> visitBinaryFile({
required File binaryFile,
required String parentVirtualPath,
int retryCount = 3,
int sleepTime = 1,
}) async {
final String currentFileName = binaryFile.basename;
final String entitlementCurrentPath = joinEntitlementPaths(parentVirtualPath, currentFileName);
Expand Down Expand Up @@ -325,27 +330,19 @@ update these file paths accordingly.
],
];

io.ProcessResult? result;
while (retryCount > 0) {
await retryOptions.retry(() async {
log.info('Executing: ${args.join(' ')}\n');
result = await processManager.run(args);
final io.ProcessResult result = await processManager.run(args);
if (result.exitCode == 0) {
return;
}

log.severe(
throw CodesignException(
'Failed to codesign ${binaryFile.absolute.path} with args: ${args.join(' ')}\n'
'stdout:\n${(result.stdout as String).trim()}'
'stderr:\n${(result.stderr as String).trim()}',
);

retryCount -= 1;
await Future.delayed(Duration(seconds: sleepTime));
sleepTime *= 2;
}
throw CodesignException('Failed to codesign ${binaryFile.absolute.path} with args: ${args.join(' ')}\n'
'stdout:\n${(result!.stdout as String).trim()}\n'
'stderr:\n${(result.stderr as String).trim()}');
});
}

/// Delete codesign metadata at ALL places inside engine binary.
Expand Down Expand Up @@ -395,7 +392,7 @@ update these file paths accordingly.
/// binaries are properly codesigned, and notarize the entire archive.
Future<void> notarize(File file) async {
final Completer<void> completer = Completer<void>();
final String uuid = uploadZipToNotary(file);
final String uuid = await uploadZipToNotary(file);

Future<void> callback(Timer timer) async {
final bool notaryFinished = checkNotaryJobFinished(uuid);
Expand Down Expand Up @@ -461,57 +458,49 @@ update these file paths accordingly.
throw CodesignException('Notarization failed with: $status\n$combinedOutput');
}

/// Upload artifact to Apple notary service.
String uploadZipToNotary(File localFile, [int retryCount = 3, int sleepTime = 1]) {
while (retryCount > 0) {
final List<String> args = <String>[
'xcrun',
'notarytool',
'submit',
localFile.absolute.path,
'--apple-id',
codesignAppstoreId,
'--password',
appSpecificPassword,
'--team-id',
codesignTeamId,
'--verbose',
];

String argsWithoutCredentials = args.join(' ');
for (var key in redactedCredentials.keys) {
argsWithoutCredentials = argsWithoutCredentials.replaceAll(key, redactedCredentials[key]!);
}
log.info('uploading to notary: $argsWithoutCredentials');
final io.ProcessResult result = processManager.runSync(args);
if (result.exitCode != 0) {
throw CodesignException(
'Command "$argsWithoutCredentials" failed with exit code ${result.exitCode}\nStdout: ${result.stdout}\nStderr: ${result.stderr}',
);
}

final String combinedOutput = (result.stdout as String) + (result.stderr as String);
final RegExpMatch? match;
match = _notarytoolRequestPattern.firstMatch(combinedOutput);

if (match == null) {
log.warning('Failed to upload to the notary service with args: $argsWithoutCredentials');
log.warning('{combinedOutput.trim()}');
retryCount -= 1;
log.warning('Trying again $retryCount more time${retryCount > 1 ? 's' : ''}...');
io.sleep(Duration(seconds: sleepTime));
continue;
}

final String requestUuid = match.group(1)!;
log.info('RequestUUID for ${localFile.path} is: $requestUuid');

return requestUuid;
}
log.warning('The upload to notary service failed after retries, and'
' the output format does not match the current notary tool version.'
' If after inspecting the output, you believe the process finished '
'successfully but was not detected, please contact flutter release engineers');
throw CodesignException('Failed to upload ${localFile.path} to the notary service');
/// Upload artifact to Apple notary service and return the tracking request UUID.
Future<String> uploadZipToNotary(File localFile) {
return retryOptions.retry(
() async {
final List<String> args = <String>[
'xcrun',
'notarytool',
'submit',
localFile.absolute.path,
'--apple-id',
codesignAppstoreId,
'--password',
appSpecificPassword,
'--team-id',
codesignTeamId,
'--verbose',
];

String argsWithoutCredentials = args.join(' ');
for (var key in redactedCredentials.keys) {
argsWithoutCredentials = argsWithoutCredentials.replaceAll(key, redactedCredentials[key]!);
}
log.info('uploading to notary: $argsWithoutCredentials');
final io.ProcessResult result = processManager.runSync(args);
if (result.exitCode != 0) {
throw CodesignException(
'Command "$argsWithoutCredentials" failed with exit code ${result.exitCode}\nStdout: ${result.stdout}\nStderr: ${result.stderr}',
);
}

final String combinedOutput = (result.stdout as String) + (result.stderr as String);
final RegExpMatch? match = _notarytoolRequestPattern.firstMatch(combinedOutput);

if (match == null) {
log.warning('Failed to upload to the notary service');
log.warning('$argsWithoutCredentials\n$combinedOutput');
throw CodesignException('Failed to upload to the notary service\n$combinedOutput');
}

final String requestUuid = match.group(1)!;
log.info('RequestUUID for ${localFile.path} is: $requestUuid');
return requestUuid;
},
);
}
}
1 change: 1 addition & 0 deletions cipd_packages/codesign/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ dependencies:
meta: 1.11.0
platform: 3.1.3
process: 5.0.2
retry: 3.1.2
90 changes: 41 additions & 49 deletions cipd_packages/codesign/test/file_codesign_visitor_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:codesign/src/utils.dart';
import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:logging/logging.dart';
import 'package:retry/retry.dart';
import 'package:test/test.dart';

import './src/fake_process_manager.dart';
Expand Down Expand Up @@ -52,8 +53,9 @@ void main() {
rootDirectory: rootDirectory,
inputZipPath: inputZipPath,
outputZipPath: outputZipPath,
notarizationTimerDuration: const Duration(seconds: 0),
notarizationTimerDuration: Duration.zero,
dryrun: false,
retryOptions: const RetryOptions(maxAttempts: 0),
);
codesignVisitor.directoriesVisited.clear();
});
Expand Down Expand Up @@ -108,10 +110,11 @@ void main() {
codesignTeamIDFilePath: codesignTeamIDFilePath,
processManager: processManager,
rootDirectory: rootDirectory,
notarizationTimerDuration: const Duration(seconds: 0),
notarizationTimerDuration: Duration.zero,
dryrun: false,
inputZipPath: inputZipPath,
outputZipPath: outputZipPath,
retryOptions: const RetryOptions(maxAttempts: 0),
);
codesignVisitor.directoriesVisited.clear();
codesignVisitor.appSpecificPassword = fakePassword;
Expand Down Expand Up @@ -240,6 +243,7 @@ void main() {
inputZipPath: inputZipPath,
outputZipPath: outputZipPath,
notarizationTimerDuration: Duration.zero,
retryOptions: const RetryOptions(maxAttempts: 3, delayFactor: Duration.zero),
);
codesignVisitor.directoriesVisited.clear();
codesignVisitor.appSpecificPassword = randomString;
Expand Down Expand Up @@ -569,7 +573,8 @@ void main() {
inputZipPath: inputZipPath,
outputZipPath: outputZipPath,
dryrun: false,
notarizationTimerDuration: const Duration(seconds: 0),
notarizationTimerDuration: Duration.zero,
retryOptions: const RetryOptions(maxAttempts: 0),
);
codesignVisitor.appSpecificPassword = randomString;
codesignVisitor.codesignAppstoreId = randomString;
Expand Down Expand Up @@ -658,6 +663,7 @@ void main() {
codesignTeamIDFilePath: codesignTeamIDFilePath,
processManager: processManager,
rootDirectory: rootDirectory,
retryOptions: const RetryOptions(maxAttempts: 0),
);
codesignVisitor.directoriesVisited.clear();
codesignVisitor.appSpecificPassword = randomString;
Expand Down Expand Up @@ -765,6 +771,7 @@ file_c''',
codesignTeamIDFilePath: codesignTeamIDFilePath,
processManager: processManager,
rootDirectory: rootDirectory,
retryOptions: const RetryOptions(maxAttempts: 3, delayFactor: Duration.zero),
);
codesignVisitor.directoriesVisited.clear();
codesignVisitor.appSpecificPassword = fakePassword;
Expand Down Expand Up @@ -928,14 +935,13 @@ status: Invalid''',
],
stdout: '''Successfully uploaded file.
id: 2efe2717-52ef-43a5-96dc-0797e4ca1041
RequestUUID: 2EFE2717-52EF-43A5-96DC-0797E4CA1041
path: /Users/flutter/Desktop/OvernightTextEditor_11.6.8.zip''',
),
]);

final String uuid = codesignVisitor.uploadZipToNotary(
final String uuid = await codesignVisitor.uploadZipToNotary(
fileSystem.file('${rootDirectory.absolute.path}/temp'),
3,
0,
);
expect(uuid, '2efe2717-52ef-43a5-96dc-0797e4ca1041');
final List<String> messages = records
Expand All @@ -944,46 +950,38 @@ status: Invalid''',
.toList();
expect(
messages,
contains('Failed to upload to the notary service with args: '
'xcrun notarytool submit ${rootDirectory.absolute.path}/temp '
'--apple-id <appleID-redacted> --password <appSpecificPassword-redacted> --team-id <teamID-redacted> '
'--verbose'),
);
expect(
messages,
contains('Trying again 2 more times...'),
contains('Failed to upload to the notary service'),
);
});

test('upload notary throws exception if exit code is unnormal', () async {
fileSystem.file('${rootDirectory.absolute.path}/temp').createSync();
processManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
'xcrun',
'notarytool',
'submit',
'${rootDirectory.absolute.path}/temp',
'--apple-id',
fakeAppleID,
'--password',
fakePassword,
'--team-id',
fakeTeamID,
'--verbose',
],
stdout: '''Error uploading file.
for (int i = 0; i < 3; i++) {
processManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
'xcrun',
'notarytool',
'submit',
'${rootDirectory.absolute.path}/temp',
'--apple-id',
fakeAppleID,
'--password',
fakePassword,
'--team-id',
fakeTeamID,
'--verbose',
],
stdout: '''Error uploading file.
Id: something that causes failure
path: /Users/flutter/Desktop/OvernightTextEditor_11.6.8.zip''',
exitCode: -1,
),
]);

exitCode: 1,
),
]);
}
expect(
() => codesignVisitor.uploadZipToNotary(
() async => codesignVisitor.uploadZipToNotary(
fileSystem.file('${rootDirectory.absolute.path}/temp'),
1,
0,
),
throwsA(
isA<CodesignException>(),
Expand Down Expand Up @@ -1053,23 +1051,15 @@ status: Invalid''',
expect(
() => codesignVisitor.uploadZipToNotary(
fileSystem.file('${rootDirectory.absolute.path}/temp'),
3,
0,
),
throwsA(
isA<CodesignException>(),
),
);
final List<String> messages = records
.where((LogRecord record) => record.level == Level.WARNING)
.map((LogRecord record) => record.message)
.toList();
final List<String> messages = records.map((LogRecord record) => record.message).toList();
expect(
messages,
contains('The upload to notary service failed after retries, and'
' the output format does not match the current notary tool version.'
' If after inspecting the output, you believe the process finished '
'successfully but was not detected, please contact flutter release engineers'),
contains('Failed to upload to the notary service'),
);
});
});
Expand All @@ -1086,7 +1076,8 @@ status: Invalid''',
codesignTeamIDFilePath: codesignTeamIDFilePath,
processManager: processManager,
rootDirectory: rootDirectory,
notarizationTimerDuration: const Duration(seconds: 0),
notarizationTimerDuration: Duration.zero,
retryOptions: const RetryOptions(maxAttempts: 0),
);
codesignVisitor.directoriesVisited.clear();
codesignVisitor.appSpecificPassword = fakePassword;
Expand Down Expand Up @@ -1237,7 +1228,8 @@ status: Invalid''',
codesignTeamIDFilePath: codesignTeamIDFilePath,
processManager: processManager,
rootDirectory: rootDirectory,
notarizationTimerDuration: const Duration(seconds: 0),
notarizationTimerDuration: Duration.zero,
retryOptions: const RetryOptions(maxAttempts: 0),
dryrun: false,
);
codesignVisitor.appSpecificPassword = fakePassword;
Expand Down

0 comments on commit f7bff76

Please sign in to comment.