Skip to content

Commit 708ed02

Browse files
mathewcmathewc
committed
Enable Key APIs for functions in error (#1253)
1 parent dd3a0a5 commit 708ed02

File tree

5 files changed

+186
-14
lines changed

5 files changed

+186
-14
lines changed

‎src/WebJobs.Script.WebHost/Controllers/KeysController.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public KeysController(WebScriptHostManager scriptHostManager, ISecretManager sec
3535
[Route("admin/functions/{name}/keys")]
3636
public async Task<IHttpActionResult> Get(string name)
3737
{
38-
if (!_scriptHostManager.Instance.Functions.Any(f => string.Equals(f.Name, name, StringComparison.OrdinalIgnoreCase)))
38+
if (!_scriptHostManager.Instance.IsFunction(name))
3939
{
4040
return NotFound();
4141
}
@@ -101,7 +101,7 @@ private async Task<IHttpActionResult> PutKeyAsync(string keyName, Key key, strin
101101
private async Task<IHttpActionResult> AddOrUpdateFunctionSecretAsync(string keyName, string value, string functionName = null)
102102
{
103103
if (functionName != null &&
104-
!_scriptHostManager.Instance.Functions.Any(f => string.Equals(f.Name, functionName, StringComparison.OrdinalIgnoreCase)))
104+
!_scriptHostManager.Instance.IsFunction(functionName))
105105
{
106106
return NotFound();
107107
}
@@ -147,7 +147,7 @@ private async Task<IHttpActionResult> DeleteFunctionSecretAsync(string keyName,
147147
return BadRequest("Invalid key name.");
148148
}
149149

150-
if ((functionName != null && !_scriptHostManager.Instance.Functions.Any(f => string.Equals(f.Name, functionName, StringComparison.OrdinalIgnoreCase))) ||
150+
if ((functionName != null && !_scriptHostManager.Instance.IsFunction(functionName)) ||
151151
!await _secretManager.DeleteSecretAsync(keyName, functionName))
152152
{
153153
// Either the function or the key were not found

‎src/WebJobs.Script/Host/ScriptHost.cs‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,36 +3,31 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Collections.Immutable;
67
using System.Collections.ObjectModel;
8+
using System.Configuration;
79
using System.Diagnostics;
810
using System.Globalization;
911
using System.IO;
12+
using System.IO.Abstractions;
1013
using System.Linq;
1114
using System.Reflection;
1215
using System.Reflection.Emit;
1316
using System.Text.RegularExpressions;
1417
using System.Threading;
1518
using System.Threading.Tasks;
1619
using Microsoft.Azure.WebJobs.Extensions;
17-
using Microsoft.Azure.WebJobs.Extensions.ApiHub;
18-
using Microsoft.Azure.WebJobs.Extensions.Bindings;
1920
using Microsoft.Azure.WebJobs.Extensions.BotFramework.Bindings;
20-
using Microsoft.Azure.WebJobs.Extensions.DocumentDB;
21-
using Microsoft.Azure.WebJobs.Extensions.MobileApps;
22-
using Microsoft.Azure.WebJobs.Extensions.NotificationHubs;
2321
using Microsoft.Azure.WebJobs.Host;
2422
using Microsoft.Azure.WebJobs.Host.Indexers;
23+
using Microsoft.Azure.WebJobs.Script.Binding;
2524
using Microsoft.Azure.WebJobs.Script.Config;
2625
using Microsoft.Azure.WebJobs.Script.Description;
2726
using Microsoft.Azure.WebJobs.Script.Diagnostics;
2827
using Microsoft.Azure.WebJobs.Script.Extensibility;
2928
using Microsoft.Azure.WebJobs.Script.IO;
3029
using Newtonsoft.Json;
3130
using Newtonsoft.Json.Linq;
32-
using System.Collections.Immutable;
33-
using System.Configuration;
34-
using System.IO.Abstractions;
35-
using Microsoft.Azure.WebJobs.Script.Binding;
3631

3732
namespace Microsoft.Azure.WebJobs.Script
3833
{
@@ -54,7 +49,7 @@ public class ScriptHost : JobHost
5449
private ScriptSettingsManager _settingsManager;
5550
private bool _shutdownScheduled;
5651

57-
protected ScriptHost(IScriptHostEnvironment environment, ScriptHostConfiguration scriptConfig = null, ScriptSettingsManager settingsManager = null)
52+
protected internal ScriptHost(IScriptHostEnvironment environment, ScriptHostConfiguration scriptConfig = null, ScriptSettingsManager settingsManager = null)
5853
: base(scriptConfig.HostConfig)
5954
{
6055
scriptConfig = scriptConfig ?? new ScriptHostConfiguration();
@@ -98,9 +93,31 @@ public string InstanceId
9893

9994
public ScriptHostConfiguration ScriptConfig { get; private set; }
10095

96+
/// <summary>
97+
/// Gets the collection of all valid Functions. For functions that are in error
98+
/// and were unable to load successfully, consult the <see cref="FunctionErrors"/> collection.
99+
/// </summary>
101100
public virtual Collection<FunctionDescriptor> Functions { get; private set; }
102101

103-
public Dictionary<string, Collection<string>> FunctionErrors { get; private set; }
102+
public virtual Dictionary<string, Collection<string>> FunctionErrors { get; private set; }
103+
104+
/// <summary>
105+
/// Returns true if the specified name is the name of a known function,
106+
/// regardless of whether the function is in error.
107+
/// </summary>
108+
/// <param name="name">The name of the function to check for.</param>
109+
/// <returns></returns>
110+
public bool IsFunction(string name)
111+
{
112+
if (!string.IsNullOrEmpty(name) &&
113+
(Functions.Any(f => string.Equals(f.Name, name, StringComparison.OrdinalIgnoreCase)) ||
114+
FunctionErrors.ContainsKey(name)))
115+
{
116+
return true;
117+
};
118+
119+
return false;
120+
}
104121

105122
public virtual bool IsPrimary
106123
{

‎test/WebJobs.Script.Tests/Controllers/Admin/KeysControllerTests.cs‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Collections.ObjectModel;
7+
using System.Diagnostics;
8+
using System.Net;
9+
using System.Net.Http;
10+
using System.Threading.Tasks;
11+
using System.Web.Http.Results;
12+
using Microsoft.Azure.WebJobs.Script.Config;
13+
using Microsoft.Azure.WebJobs.Script.Description;
14+
using Microsoft.Azure.WebJobs.Script.WebHost;
15+
using Microsoft.Azure.WebJobs.Script.WebHost.Controllers;
16+
using Microsoft.Azure.WebJobs.Script.WebHost.Models;
17+
using Moq;
18+
using Newtonsoft.Json.Linq;
19+
using WebJobs.Script.Tests;
20+
using Xunit;
21+
22+
namespace Microsoft.Azure.WebJobs.Script.Tests
23+
{
24+
public class KeysControllerTests : IDisposable
25+
{
26+
private readonly ScriptSettingsManager _settingsManager;
27+
private readonly TempDirectory _secretsDirectory = new TempDirectory();
28+
private Mock<ScriptHost> _hostMock;
29+
private Mock<WebScriptHostManager> _managerMock;
30+
private Collection<FunctionDescriptor> _testFunctions;
31+
private Dictionary<string, Collection<string>> _testFunctionErrors;
32+
private KeysController _testController;
33+
private Mock<ISecretManager> _secretsManagerMock;
34+
35+
public KeysControllerTests()
36+
{
37+
_settingsManager = ScriptSettingsManager.Instance;
38+
_testFunctions = new Collection<FunctionDescriptor>();
39+
_testFunctionErrors = new Dictionary<string, Collection<string>>();
40+
41+
var config = new ScriptHostConfiguration();
42+
var environment = new NullScriptHostEnvironment();
43+
_hostMock = new Mock<ScriptHost>(MockBehavior.Strict, new object[] { environment, config, null });
44+
_hostMock.Setup(p => p.Functions).Returns(_testFunctions);
45+
_hostMock.Setup(p => p.FunctionErrors).Returns(_testFunctionErrors);
46+
47+
WebHostSettings settings = new WebHostSettings();
48+
settings.SecretsPath = _secretsDirectory.Path;
49+
_secretsManagerMock = new Mock<ISecretManager>(MockBehavior.Strict);
50+
_managerMock = new Mock<WebScriptHostManager>(MockBehavior.Strict, new object[] { config, new TestSecretManagerFactory(_secretsManagerMock.Object), _settingsManager, settings });
51+
_managerMock.SetupGet(p => p.Instance).Returns(_hostMock.Object);
52+
53+
var traceWriter = new TestTraceWriter(TraceLevel.Verbose);
54+
55+
_testController = new KeysController(_managerMock.Object, _secretsManagerMock.Object, traceWriter);
56+
57+
// setup some test functions
58+
string errorFunction = "ErrorFunction";
59+
var errors = new Collection<string>();
60+
errors.Add("A really really bad error!");
61+
_testFunctionErrors.Add(errorFunction, errors);
62+
63+
var keys = new Dictionary<string, string>
64+
{
65+
{ "key1", "secret1" }
66+
};
67+
_secretsManagerMock.Setup(p => p.GetFunctionSecretsAsync(errorFunction, false)).ReturnsAsync(keys);
68+
}
69+
70+
[Fact]
71+
public async Task GetKeys_FunctionInError_ReturnsKeys()
72+
{
73+
_testController.Request = new HttpRequestMessage(HttpMethod.Get, "https://local/admin/functions/keys");
74+
var result = (OkNegotiatedContentResult<ApiModel>)(await _testController.Get("ErrorFunction"));
75+
76+
var content = (JObject)result.Content;
77+
var keys = content["keys"];
78+
Assert.Equal("key1", keys[0]["name"]);
79+
Assert.Equal("secret1", keys[0]["value"]);
80+
}
81+
82+
[Fact]
83+
public async Task PutKey_FunctionInError_Succeeds()
84+
{
85+
_testController.Request = new HttpRequestMessage(HttpMethod.Get, "https://local/admin/functions/keys/key2");
86+
87+
var key = new Key("key2", "secret2");
88+
var keyOperationResult = new KeyOperationResult(key.Value, OperationResult.Updated);
89+
_secretsManagerMock.Setup(p => p.AddOrUpdateFunctionSecretAsync(key.Name, key.Value, "ErrorFunction")).ReturnsAsync(keyOperationResult);
90+
91+
var result = (OkNegotiatedContentResult<ApiModel>)(await _testController.Put("ErrorFunction", key.Name, key));
92+
var content = (JObject)result.Content;
93+
Assert.Equal("key2", content["name"]);
94+
Assert.Equal("secret2", content["value"]);
95+
}
96+
97+
[Fact]
98+
public async Task DeleteKey_FunctionInError_Succeeds()
99+
{
100+
_testController.Request = new HttpRequestMessage(HttpMethod.Get, "https://local/admin/functions/keys/key2");
101+
102+
_secretsManagerMock.Setup(p => p.DeleteSecretAsync("key2", "ErrorFunction")).ReturnsAsync(true);
103+
104+
var result = (StatusCodeResult)(await _testController.Delete("ErrorFunction", "key2"));
105+
Assert.Equal(HttpStatusCode.NoContent, result.StatusCode);
106+
}
107+
108+
protected virtual void Dispose(bool disposing)
109+
{
110+
if (disposing)
111+
{
112+
_secretsDirectory.Dispose();
113+
}
114+
}
115+
116+
public void Dispose()
117+
{
118+
// Do not change this code. Put cleanup code in Dispose(bool disposing) above.
119+
Dispose(true);
120+
}
121+
}
122+
}

‎test/WebJobs.Script.Tests/ScriptHostTests.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,38 @@ public void TryParseFunctionMetadata_ValidatesHttpRoutes()
934934
Assert.Equal("The specified route conflicts with one or more built in routes.", functionError);
935935
}
936936

937+
[Fact]
938+
public void IsFunction_ReturnsExpectedResult()
939+
{
940+
Mock<IScriptHostEnvironment> mockEnvironment = new Mock<IScriptHostEnvironment>(MockBehavior.Strict);
941+
var config = new ScriptHostConfiguration();
942+
943+
var mockHost = new Mock<ScriptHost>(MockBehavior.Strict, new object[] { mockEnvironment.Object, config, null });
944+
945+
var functions = new Collection<FunctionDescriptor>();
946+
var functionErrors = new Dictionary<string, Collection<string>>();
947+
mockHost.Setup(p => p.Functions).Returns(functions);
948+
mockHost.Setup(p => p.FunctionErrors).Returns(functionErrors);
949+
950+
var parameters = new Collection<ParameterDescriptor>();
951+
parameters.Add(new ParameterDescriptor("param1", typeof(string)));
952+
var metadata = new FunctionMetadata();
953+
var invoker = new TestInvoker();
954+
var function = new FunctionDescriptor("TestFunction", invoker, metadata, parameters);
955+
functions.Add(function);
956+
957+
var errors = new Collection<string>();
958+
errors.Add("A really really bad error!");
959+
functionErrors.Add("ErrorFunction", errors);
960+
961+
var host = mockHost.Object;
962+
Assert.True(host.IsFunction("TestFunction"));
963+
Assert.True(host.IsFunction("ErrorFunction"));
964+
Assert.False(host.IsFunction("DoesNotExist"));
965+
Assert.False(host.IsFunction(string.Empty));
966+
Assert.False(host.IsFunction(null));
967+
}
968+
937969
public class AssemblyMock : Assembly
938970
{
939971
public override object[] GetCustomAttributes(Type attributeType, bool inherit)

‎test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@
446446
<Compile Include="Binding\SendGridScriptBindingProviderTests.cs" />
447447
<Compile Include="Binding\TwilioScriptBindingProviderTests.cs" />
448448
<Compile Include="Controllers\Admin\AdminControllerTests.cs" />
449+
<Compile Include="Controllers\Admin\KeysControllerTests.cs" />
449450
<Compile Include="Description\DotNet\CSharp\CSharpCompilationServiceTests.cs" />
450451
<Compile Include="Description\Script\ScriptFunctionInvokerBaseTests.cs" />
451452
<Compile Include="Filters\AuthorizationLevelAttributeTests.cs" />

0 commit comments

Comments
 (0)