-
Notifications
You must be signed in to change notification settings - Fork 464
adding a read-only ContainerApps secret repository #11202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
8a735ad
0a877f7
5b0f23e
910372d
0481ddf
bba7bad
c97c068
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT License. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace Microsoft.Azure.WebJobs.Script.WebHost; | ||
|
||
public class ContainerAppsSecretsRepository : ISecretsRepository | ||
{ | ||
internal const string ContainerAppsSecretsDir = "/run/secrets/functions-keys"; | ||
|
||
// host.master = value | ||
private const string MasterKey = "host.master"; | ||
// host.function.{keyName} = value | ||
private const string HostFunctionKeyPrefix = "host.function."; | ||
// host.systemKey.{keyName} = value | ||
private const string SystemKeyPrefix = "host.systemKey."; | ||
// functions.{functionName}.{keyName} = value | ||
private const string FunctionKeyPrefix = "functions."; | ||
|
||
private readonly ILogger<ContainerAppsSecretsRepository> _logger; | ||
|
||
public ContainerAppsSecretsRepository(ILogger<ContainerAppsSecretsRepository> logger) | ||
{ | ||
_logger = logger; | ||
brettsam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// explicitly implementing this to avoid "unused" warnings on build | ||
event EventHandler<SecretsChangedEventArgs> ISecretsRepository.SecretsChanged | ||
{ | ||
add { } | ||
remove { } | ||
} | ||
|
||
public bool IsEncryptionSupported => false; | ||
|
||
public string Name => nameof(ContainerAppsSecretsRepository); | ||
|
||
public async Task<ScriptSecrets> ReadAsync(ScriptSecretsType type, string functionName) | ||
{ | ||
if (type == ScriptSecretsType.Function && string.IsNullOrEmpty(functionName)) | ||
{ | ||
throw new ArgumentNullException(nameof(functionName), $"{nameof(functionName)} cannot be null or empty with {nameof(type)} = {nameof(ScriptSecretsType.Function)}"); | ||
} | ||
|
||
return type == ScriptSecretsType.Host ? await ReadHostSecretsAsync() : await ReadFunctionSecretsAsync(functionName.ToLowerInvariant()); | ||
} | ||
|
||
public Task WriteAsync(ScriptSecretsType type, string functionName, ScriptSecrets secrets) | ||
=> throw new NotImplementedException(); | ||
|
||
private async Task<ScriptSecrets> ReadHostSecretsAsync() | ||
{ | ||
var secrets = await GetFromFilesAsync(ContainerAppsSecretsDir); | ||
|
||
HostSecrets hostSecrets = new HostSecrets() | ||
{ | ||
FunctionKeys = [], | ||
SystemKeys = [] | ||
}; | ||
|
||
foreach (var pair in secrets) | ||
{ | ||
if (pair.Key.StartsWith(MasterKey, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
hostSecrets.MasterKey = new Key("master", pair.Value); | ||
} | ||
else if (pair.Key.StartsWith(HostFunctionKeyPrefix, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
hostSecrets.FunctionKeys.Add(ParseKeyWithPrefix(HostFunctionKeyPrefix, pair.Key, pair.Value)); | ||
} | ||
else if (pair.Key.StartsWith(SystemKeyPrefix)) | ||
{ | ||
hostSecrets.SystemKeys.Add(ParseKeyWithPrefix(SystemKeyPrefix, pair.Key, pair.Value)); | ||
} | ||
} | ||
|
||
// Always return a HostSecrets object, even if empty. This will prevent the SecretManager from thinking | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So who is responsible for ensuring that require keys like the master key are present? What happens if the key isn't present but /admin APIs are called by control plane? |
||
// it needs to create and persist new secrets, which is not supported in Container Apps. | ||
return hostSecrets; | ||
} | ||
|
||
private async Task<ScriptSecrets> ReadFunctionSecretsAsync(string functionName) | ||
{ | ||
var secrets = await GetFromFilesAsync(ContainerAppsSecretsDir); | ||
|
||
var prefix = $"{FunctionKeyPrefix}{functionName}."; | ||
|
||
var functionSecrets = new FunctionSecrets() | ||
{ | ||
Keys = secrets | ||
.Where(p => p.Key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) | ||
.Select(p => ParseKeyWithPrefix(prefix, p.Key, p.Value)) | ||
.ToList() | ||
}; | ||
|
||
// Always return a FunctionSecrets object, even if empty. This will prevent the SecretManager from thinking | ||
// it needs to create and persist new secrets, which is not supported in Container Apps. | ||
return functionSecrets; | ||
} | ||
|
||
private async Task<IDictionary<string, string>> GetFromFilesAsync(string path) | ||
{ | ||
string[] files = await FileUtility.GetFilesAsync(path, "*"); | ||
var secrets = new Dictionary<string, string>(files.Length); | ||
|
||
StringBuilder sb = new StringBuilder("Loaded secrets from files:"); | ||
|
||
foreach (var file in files) | ||
{ | ||
secrets.Add(Path.GetFileName(file), await FileUtility.ReadAsync(file)); | ||
sb.AppendLine($" {file}"); | ||
} | ||
|
||
_logger.LogDebug(sb.ToString()); | ||
return secrets; | ||
} | ||
|
||
/// <summary> | ||
/// no-op - allow stale secrets to remain. | ||
/// </summary> | ||
public async Task PurgeOldSecretsAsync(IList<string> currentFunctions, ILogger logger) | ||
=> await Task.Yield(); | ||
|
||
/// <summary> | ||
/// Runtime is not responsible for encryption so this code will never be executed. | ||
/// </summary> | ||
public Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, ScriptSecrets secrets) | ||
=> throw new NotSupportedException(); | ||
|
||
/// <summary> | ||
/// Runtime is not responsible for encryption so this code will never be executed. | ||
/// </summary> | ||
public Task<string[]> GetSecretSnapshots(ScriptSecretsType type, string functionName) | ||
=> throw new NotSupportedException(); | ||
|
||
private static Key ParseKeyWithPrefix(string prefix, string key, string value) | ||
=> new(key.Substring(prefix.Length), value); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,6 @@ | |
using Microsoft.Azure.WebJobs.Script.WebHost.Properties; | ||
using Microsoft.Azure.WebJobs.Script.WebHost.Security; | ||
using Microsoft.Extensions.Logging; | ||
using static Microsoft.Azure.WebJobs.Script.WebHost.Models.FunctionAppSecrets; | ||
using DataProtectionConstants = Microsoft.Azure.Web.DataProtection.Constants; | ||
|
||
namespace Microsoft.Azure.WebJobs.Script.WebHost | ||
|
@@ -140,7 +139,7 @@ public async virtual Task<HostSecretsInfo> GetHostSecretsAsync() | |
} | ||
|
||
// before caching any secrets, validate them | ||
string masterKeyValue = hostSecrets.MasterKey.Value; | ||
string masterKeyValue = hostSecrets.MasterKey?.Value; | ||
Comment on lines
141
to
+142
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change to make masterKeyValue nullable without updating the ValidateHostSecrets method call could cause issues. The ValidateHostSecrets method may not be designed to handle null masterKeyValue parameter. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
var functionKeys = hostSecrets.FunctionKeys.ToDictionary(p => p.Name, p => p.Value); | ||
var systemKeys = hostSecrets.SystemKeys.ToDictionary(p => p.Name, p => p.Value); | ||
ValidateHostSecrets(masterKeyValue, functionKeys, systemKeys); | ||
|
@@ -740,7 +739,7 @@ private HostSecrets ReadHostSecrets(HostSecrets hostSecrets) | |
{ | ||
return new HostSecrets | ||
{ | ||
MasterKey = _keyValueConverterFactory.ReadKey(hostSecrets.MasterKey), | ||
MasterKey = hostSecrets.MasterKey is null ? null : _keyValueConverterFactory.ReadKey(hostSecrets.MasterKey), | ||
brettsam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
FunctionKeys = hostSecrets.FunctionKeys.Select(k => _keyValueConverterFactory.ReadKey(k)).ToList(), | ||
SystemKeys = hostSecrets.SystemKeys?.Select(k => _keyValueConverterFactory.ReadKey(k)).ToList() ?? new List<Key>() | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT License. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.IO.Abstractions; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using Microsoft.Azure.WebJobs.Script.WebHost; | ||
using Microsoft.Extensions.Logging.Abstractions; | ||
using Moq; | ||
using Xunit; | ||
|
||
namespace Microsoft.Azure.WebJobs.Script.Tests; | ||
|
||
public class ContainerAppsSecretsRepositoryTests : IDisposable | ||
{ | ||
private Dictionary<string, Func<MemoryStream>> _fileContentMap; | ||
private ContainerAppsSecretsRepository _repo; | ||
|
||
public ContainerAppsSecretsRepositoryTests() | ||
{ | ||
// Mock the file system to return predefined secrets | ||
var mockFileSystem = new Mock<IFileSystem>(MockBehavior.Strict); | ||
var mockFile = new Mock<FileBase>(MockBehavior.Strict); | ||
var mockDirectory = new Mock<DirectoryBase>(MockBehavior.Strict); | ||
|
||
// Setup directory and file existence | ||
mockDirectory.Setup(d => d.Exists(It.IsAny<string>())).Returns(true); | ||
mockFileSystem.SetupGet(fs => fs.Directory).Returns(mockDirectory.Object); | ||
mockFileSystem.SetupGet(fs => fs.File).Returns(mockFile.Object); | ||
|
||
// Return all files when asked | ||
mockDirectory | ||
.Setup(d => d.GetFiles(ContainerAppsSecretsRepository.ContainerAppsSecretsDir, "*")) | ||
.Returns(() => _fileContentMap.Keys.ToArray()); | ||
|
||
// Setup file existence checks | ||
mockFile | ||
.Setup(f => f.Exists(It.IsAny<string>())) | ||
.Returns((string f) => _fileContentMap.ContainsKey(f)); | ||
|
||
// Return content when asked | ||
mockFile | ||
.Setup(f => f.Open(It.IsAny<string>(), FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) | ||
.Returns((string f, FileMode _, FileAccess _, FileShare _) => _fileContentMap[f]()); | ||
|
||
FileUtility.Instance = mockFileSystem.Object; | ||
|
||
_repo = new ContainerAppsSecretsRepository(NullLogger<ContainerAppsSecretsRepository>.Instance); | ||
} | ||
|
||
[Fact] | ||
public async Task Read_Host_Secrets() | ||
{ | ||
_fileContentMap = new() | ||
{ | ||
{ "/run/secrets/functions-keys/host.master", () => GetStream("mk") }, | ||
{ "/run/secrets/functions-keys/host.function.default", () => GetStream("hfd") }, | ||
{ "/run/secrets/functions-keys/host.function.key1", () => GetStream("hfk1") }, | ||
{ "/run/secrets/functions-keys/host.systemKey.key1", () => GetStream("hsk1") }, | ||
}; | ||
|
||
var result = await _repo.ReadAsync(ScriptSecretsType.Host, null); | ||
|
||
var hostSecrets = result as HostSecrets; | ||
Assert.NotNull(hostSecrets); | ||
Assert.Equal("mk", hostSecrets.MasterKey.Value); | ||
Assert.Equal("hfd", hostSecrets.GetFunctionKey("default", HostKeyScopes.FunctionKeys).Value); | ||
Assert.Equal("hfk1", hostSecrets.GetFunctionKey("Key1", HostKeyScopes.FunctionKeys).Value); | ||
Assert.Equal("hsk1", hostSecrets.GetFunctionKey("Key1", HostKeyScopes.SystemKeys).Value); | ||
} | ||
|
||
[Fact] | ||
public async Task Read_Function_Secrets() | ||
{ | ||
_fileContentMap = new() | ||
{ | ||
{ "/run/secrets/functions-keys/functions.function1.key1", () => GetStream("f1k1") }, | ||
{ "/run/secrets/functions-keys/functions.function1.key2", () => GetStream("f1k2") }, | ||
{ "/run/secrets/functions-keys/functions.function2.key1", () => GetStream("f2k1") }, | ||
{ "/run/secrets/functions-keys/functions.function2.key2", () => GetStream("f2k2") } | ||
}; | ||
|
||
var result = await _repo.ReadAsync(ScriptSecretsType.Function, "function1"); | ||
|
||
var functionSecrets = result as FunctionSecrets; | ||
Assert.NotNull(functionSecrets); | ||
Assert.Equal("f1k1", functionSecrets.GetFunctionKey("Key1", "function1").Value); | ||
Assert.Equal("f1k2", functionSecrets.GetFunctionKey("key2", "Function1").Value); | ||
|
||
result = await _repo.ReadAsync(ScriptSecretsType.Function, "function2"); | ||
functionSecrets = result as FunctionSecrets; | ||
Assert.NotNull(functionSecrets); | ||
Assert.Equal("f2k1", functionSecrets.GetFunctionKey("Key1", "funcTion2").Value); | ||
Assert.Equal("f2k2", functionSecrets.GetFunctionKey("key2", "function2").Value); | ||
} | ||
|
||
[Fact] | ||
public async Task No_HostKeys_Returns_Empty_HostSecrets() | ||
{ | ||
_fileContentMap = []; | ||
|
||
var result = await _repo.ReadAsync(ScriptSecretsType.Host, null); | ||
|
||
var hostSecrets = result as HostSecrets; | ||
Assert.NotNull(hostSecrets); | ||
Assert.Null(hostSecrets.MasterKey); | ||
Assert.Empty(hostSecrets.FunctionKeys); | ||
Assert.Empty(hostSecrets.SystemKeys); | ||
} | ||
|
||
[Fact] | ||
public async Task No_FunctionKeys_Returns_Empty_FunctionSecrets() | ||
{ | ||
_fileContentMap = []; | ||
|
||
var result = await _repo.ReadAsync(ScriptSecretsType.Function, "function1"); | ||
|
||
var hostSecrets = result as FunctionSecrets; | ||
Assert.NotNull(hostSecrets); | ||
Assert.Empty(hostSecrets.Keys); | ||
} | ||
|
||
[Fact] | ||
public async Task SecretManager_DoesNotCreate_HostSecrets() | ||
{ | ||
// no keys; we don't want the SecretManager to try to create new ones | ||
_fileContentMap = []; | ||
|
||
var testEnvironment = new TestEnvironment(); | ||
var mockHostNameProvider = new Mock<HostNameProvider>(MockBehavior.Strict, testEnvironment); | ||
var startupContextProvider = new StartupContextProvider(testEnvironment, NullLogger<StartupContextProvider>.Instance); | ||
|
||
var secretManager = new SecretManager(_repo, NullLogger.Instance, new TestMetricsLogger(), mockHostNameProvider.Object, startupContextProvider); | ||
|
||
var hostSecrets = await secretManager.GetHostSecretsAsync(); | ||
|
||
Assert.NotNull(hostSecrets); | ||
Assert.Null(hostSecrets.MasterKey); | ||
Assert.Empty(hostSecrets.FunctionKeys); | ||
Assert.Empty(hostSecrets.SystemKeys); | ||
} | ||
|
||
[Fact] | ||
public async Task SecretManager_DoesNotCreate_FunctionSecrets() | ||
{ | ||
// no keys; we don't want the SecretManager to try to create new ones | ||
_fileContentMap = []; | ||
|
||
var testEnvironment = new TestEnvironment(); | ||
var mockHostNameProvider = new Mock<HostNameProvider>(MockBehavior.Strict, testEnvironment); | ||
var startupContextProvider = new StartupContextProvider(testEnvironment, NullLogger<StartupContextProvider>.Instance); | ||
|
||
var secretManager = new SecretManager(_repo, NullLogger.Instance, new TestMetricsLogger(), mockHostNameProvider.Object, startupContextProvider); | ||
|
||
var functionSecrets = await secretManager.GetFunctionSecretsAsync("function1"); | ||
|
||
Assert.NotNull(functionSecrets); | ||
Assert.Empty(functionSecrets); | ||
} | ||
|
||
private static MemoryStream GetStream(string content) | ||
{ | ||
return new MemoryStream(Encoding.UTF8.GetBytes(content)); | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
FileUtility.Instance = null; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.