Skip to content

Commit dc66ddf

Browse files
alrodalrod
committed
Improve resilience of secret file management. Fixes #2093 
1 parent 0829b90 commit dc66ddf

File tree

2 files changed

+169
-3
lines changed

2 files changed

+169
-3
lines changed

‎src/WebJobs.Script.WebHost/Security/FileSystemSecretsRepository.cs‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ public sealed class FileSystemSecretsRepository : ISecretsRepository, IDisposabl
2020
{
2121
private readonly string _secretsPath;
2222
private readonly string _hostSecretsPath;
23+
private readonly int _retryCount = 5;
24+
private readonly int _retryDelay = 100;
2325
private readonly AutoRecoveringFileSystemWatcher _fileWatcher;
2426
private bool _disposed = false;
2527

@@ -77,16 +79,46 @@ public async Task<string> ReadAsync(ScriptSecretsType type, string functionName)
7779
string secretsContent = null;
7880
if (File.Exists(filePath))
7981
{
80-
// load the secrets file
81-
secretsContent = await FileUtility.ReadAsync(filePath);
82+
for (int currentRetry = 0; ; currentRetry++)
83+
{
84+
try
85+
{
86+
// load the secrets file
87+
secretsContent = await FileUtility.ReadAsync(filePath);
88+
break;
89+
}
90+
catch (IOException)
91+
{
92+
if (currentRetry > _retryCount)
93+
{
94+
throw;
95+
}
96+
}
97+
await Task.Delay(_retryDelay);
98+
}
8299
}
83100
return secretsContent;
84101
}
85102

86103
public async Task WriteAsync(ScriptSecretsType type, string functionName, string secretsContent)
87104
{
88105
string filePath = GetSecretsFilePath(type, functionName);
89-
await FileUtility.WriteAsync(filePath, secretsContent);
106+
for (int currentRetry = 0; ; currentRetry++)
107+
{
108+
try
109+
{
110+
await FileUtility.WriteAsync(filePath, secretsContent);
111+
break;
112+
}
113+
catch (IOException)
114+
{
115+
if (currentRetry > _retryCount)
116+
{
117+
throw;
118+
}
119+
}
120+
await Task.Delay(_retryDelay);
121+
}
90122
}
91123

92124
public async Task PurgeOldSecretsAsync(IList<string> currentFunctions, TraceWriter traceWriter, ILogger logger)

‎test/WebJobs.Script.Tests/Security/SecretManagerTests.cs‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,140 @@ public void Constructor_WithCreateHostSecretsIfMissingSet_CreatesHostSecret()
498498
}
499499
}
500500

501+
[Fact]
502+
public async Task GetHostSecretsAsync_WaitsForNewSecrets()
503+
{
504+
using (var directory = new TempDirectory())
505+
{
506+
string hostSecretsJson = @"{
507+
'masterKey': {
508+
'name': 'master',
509+
'value': '1234',
510+
'encrypted': false
511+
},
512+
'functionKeys': [],
513+
'systemKeys': []
514+
}";
515+
string filePath = Path.Combine(directory.Path, ScriptConstants.HostMetadataFileName);
516+
File.WriteAllText(filePath, hostSecretsJson);
517+
518+
HostSecretsInfo hostSecrets = null;
519+
var traceWriter = new TestTraceWriter(TraceLevel.Verbose);
520+
ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path);
521+
522+
using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null))
523+
{
524+
await Task.WhenAll(
525+
Task.Run(async () =>
526+
{
527+
// Lock the file
528+
using (FileStream fs = new FileStream(filePath, FileMode.Open, FileAccess.Write))
529+
{
530+
await Task.Delay(500);
531+
}
532+
}),
533+
Task.Run(async () =>
534+
{
535+
await Task.Delay(100);
536+
hostSecrets = await secretManager.GetHostSecretsAsync();
537+
}));
538+
539+
Assert.Equal(hostSecrets.MasterKey, "1234");
540+
}
541+
542+
using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null))
543+
{
544+
await Assert.ThrowsAsync<IOException>(async () =>
545+
{
546+
await Task.WhenAll(
547+
Task.Run(async () =>
548+
{
549+
// Lock the file
550+
using (FileStream fs = new FileStream(filePath, FileMode.Open, FileAccess.Write))
551+
{
552+
await Task.Delay(3000);
553+
}
554+
}),
555+
Task.Run(async () =>
556+
{
557+
await Task.Delay(100);
558+
hostSecrets = await secretManager.GetHostSecretsAsync();
559+
}));
560+
});
561+
}
562+
}
563+
}
564+
565+
[Fact]
566+
public async Task GetFunctionSecretsAsync_WaitsForNewSecrets()
567+
{
568+
using (var directory = new TempDirectory())
569+
{
570+
string functionName = "testfunction";
571+
string functionSecretsJson =
572+
@"{
573+
'keys': [
574+
{
575+
'name': 'Key1',
576+
'value': 'FunctionValue1',
577+
'encrypted': false
578+
},
579+
{
580+
'name': 'Key2',
581+
'value': 'FunctionValue2',
582+
'encrypted': false
583+
}
584+
]
585+
}";
586+
string filePath = Path.Combine(directory.Path, functionName + ".json");
587+
File.WriteAllText(filePath, functionSecretsJson);
588+
589+
IDictionary<string, string> functionSecrets = null;
590+
var traceWriter = new TestTraceWriter(TraceLevel.Verbose);
591+
ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path);
592+
using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null))
593+
{
594+
await Task.WhenAll(
595+
Task.Run(async () =>
596+
{
597+
// Lock the file
598+
using (FileStream fs = new FileStream(filePath, FileMode.Open, FileAccess.Write))
599+
{
600+
await Task.Delay(500);
601+
}
602+
}),
603+
Task.Run(async () =>
604+
{
605+
await Task.Delay(100);
606+
functionSecrets = await secretManager.GetFunctionSecretsAsync(functionName);
607+
}));
608+
609+
Assert.Equal(functionSecrets["Key1"], "FunctionValue1");
610+
}
611+
612+
using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null))
613+
{
614+
await Assert.ThrowsAsync<IOException>(async () =>
615+
{
616+
await Task.WhenAll(
617+
Task.Run(async () =>
618+
{
619+
// Lock the file
620+
using (FileStream fs = new FileStream(filePath, FileMode.Open, FileAccess.Write))
621+
{
622+
await Task.Delay(3000);
623+
}
624+
}),
625+
Task.Run(async () =>
626+
{
627+
await Task.Delay(100);
628+
functionSecrets = await secretManager.GetFunctionSecretsAsync(functionName);
629+
}));
630+
});
631+
}
632+
}
633+
}
634+
501635
private Mock<IKeyValueConverterFactory> GetConverterFactoryMock(bool simulateWriteConversion = true, bool setStaleValue = true)
502636
{
503637
var mockValueReader = new Mock<IKeyValueReader>();

0 commit comments

Comments
 (0)