Handle empty response for secrets from Kubernetes

ahmelsayed · ahmelsayed · commit 2efc38495701 · 2021-01-26T08:48:41.000-08:00
diff --git a/Dockerfile b/Dockerfile
@@ -1,21 +1,26 @@
-FROM mcr.microsoft.com/dotnet/core/sdk:3.0 AS installer-env
+FROM mcr.microsoft.com/dotnet/core/sdk:3.1 AS installer-env
 
 ENV PublishWithAspNetCoreTargetManifest false
 
 COPY . /workingdir
 
 RUN cd workingdir && \
-    dotnet build WebJobs.Script.sln && \
     dotnet publish src/WebJobs.Script.WebHost/WebJobs.Script.WebHost.csproj --output /azure-functions-host
 
 # Runtime image
-FROM mcr.microsoft.com/azure-functions/python:2.0
+FROM mcr.microsoft.com/azure-functions/python:3.0
 
 RUN apt-get update && \
     apt-get install -y gnupg && \
-    curl -sL https://deb.nodesource.com/setup_8.x | bash - && \
+    curl -sL https://deb.nodesource.com/setup_12.x | bash - && \
     apt-get update && \
-    apt-get install -y nodejs dotnet-sdk-3.0
+    apt-get install -y nodejs dotnet-sdk-3.1
+
+# Install the dependencies for Visual Studio Remote Debugger
+RUN apt-get update && apt-get install -y --no-install-recommends unzip procps
+
+# Install Visual Studio Remote Debugger
+RUN curl -sSL https://aka.ms/getvsdbgsh | bash /dev/stdin -v latest -l ~/vsdbg
 
 COPY --from=installer-env ["/azure-functions-host", "/azure-functions-host"]
 
diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/DefaultSecretManagerProvider.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/DefaultSecretManagerProvider.cs
@@ -72,7 +72,7 @@ internal ISecretsRepository CreateSecretsRepository()
             }
             else if (secretStorageType != null && secretStorageType.Equals("kubernetes", StringComparison.OrdinalIgnoreCase))
             {
-                return new KubernetesSecretsRepository(_environment, new SimpleKubernetesClient(_environment));
+                return new KubernetesSecretsRepository(_environment, new SimpleKubernetesClient(_environment, _loggerFactory.CreateLogger<SimpleKubernetesClient>()));
             }
             else if (secretStorageSas != null)
             {
diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/SimpleKubernetesClient.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/SimpleKubernetesClient.cs
@@ -13,6 +13,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.WebJobs.Script.IO;
+using Microsoft.Extensions.Logging;
 using Microsoft.Net.Http.Headers;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -27,18 +28,20 @@ public class SimpleKubernetesClient : IKubernetesClient, IDisposable
         private const string KubernetesSecretsDir = "/run/secrets/functions-keys";
         private readonly HttpClient _httpClient;
         private readonly IEnvironment _environment;
+        private readonly ILogger _logger;
         private Action _watchCallback;
         private bool _disposed;
         private AutoRecoveringFileSystemWatcher _fileWatcher;
 
-        public SimpleKubernetesClient(IEnvironment environment) : this(environment, CreateHttpClient())
+        public SimpleKubernetesClient(IEnvironment environment, ILogger logger) : this(environment, CreateHttpClient(), logger)
         { }
 
         // for testing
-        internal SimpleKubernetesClient(IEnvironment environment, HttpClient client)
+        internal SimpleKubernetesClient(IEnvironment environment, HttpClient client, ILogger logger)
         {
             _httpClient = client;
             _environment = environment;
+            _logger = logger;
             Task.Run(() => RunWatcher());
         }
 
@@ -58,7 +61,9 @@ public async Task<IDictionary<string, string>> GetSecrets()
             }
             else
             {
-                throw new InvalidOperationException($"{nameof(KubernetesSecretsRepository)} requires setting {EnvironmentSettingNames.AzureWebJobsKubernetesSecretName} or mounting secrets to {KubernetesSecretsDir}");
+                var exception = new InvalidOperationException($"{nameof(KubernetesSecretsRepository)} requires setting {EnvironmentSettingNames.AzureWebJobsKubernetesSecretName} or mounting secrets to {KubernetesSecretsDir}");
+                _logger.LogError(exception, "Error geting secrets");
+                throw exception;
             }
         }
 
@@ -74,6 +79,12 @@ public async Task UpdateSecrets(IDictionary<string, string> data)
             using (var request = await GetRequest(HttpMethod.Patch, url, new[] { new { op = "replace", path = "/data", value = data } }, "application/json-patch+json"))
             {
                 var response = await _httpClient.SendAsync(request);
+                if (!response.IsSuccessStatusCode)
+                {
+                    _logger.LogError("Error updating Kubernetes secrets. {StatusCode}: {Content}",
+                        response.StatusCode,
+                        await response.Content.ReadAsStringAsync());
+                }
                 response.EnsureSuccessStatusCode();
             }
         }
@@ -84,6 +95,15 @@ public void OnSecretChange(Action callback)
         }
 
         private async Task RunWatcher()
+        {
+            while (!_disposed)
+            {
+                // watch API requests terminate after 4 minutes
+                await RunWatcherInternal();
+            }
+        }
+
+        private async Task RunWatcherInternal()
         {
             if (string.IsNullOrEmpty(KubernetesObjectName) && FileUtility.DirectoryExists(KubernetesSecretsDir))
             {
@@ -124,13 +144,15 @@ private async Task<IDictionary<string, string>> GetFromApiServer(string objectNa
                 if (response.IsSuccessStatusCode)
                 {
                     var obj = await response.Content.ReadAsAsync<JObject>();
-                    return obj["data"]
-                        ?.ToObject<IDictionary<string, string>>()
-                        ?.ToDictionary(
+                    return obj.ContainsKey("data")
+                    ? obj["data"]
+                        .ToObject<IDictionary<string, string>>()
+                        .ToDictionary(
                             k => k.Key,
                             v => decode
                                 ? Encoding.UTF8.GetString(Convert.FromBase64String(v.Value))
-                                : v.Value);
+                                : v.Value)
+                    : new Dictionary<string, string>();
                 }
                 else if (response.StatusCode == HttpStatusCode.NotFound)
                 {
@@ -139,7 +161,9 @@ private async Task<IDictionary<string, string>> GetFromApiServer(string objectNa
                 }
                 else
                 {
-                    throw new HttpRequestException($"Error calling GET {url}, Status: {response.StatusCode}, Content: {await response.Content.ReadAsStringAsync()}");
+                    var exception = new HttpRequestException($"Error calling GET {url}, Status: {response.StatusCode}, Content: {await response.Content.ReadAsStringAsync()}");
+                    _logger.LogError(exception, "Error reading secrets from Kubernetes");
+                    throw exception;
                 }
             }
         }
@@ -165,6 +189,12 @@ private async Task CreateIfDoesntExist(string url, bool isSecret)
                     using (var createRequest = await GetRequest(HttpMethod.Post, url, payload))
                     {
                         var createResponse = await _httpClient.SendAsync(createRequest);
+                        if (!createResponse.IsSuccessStatusCode)
+                        {
+                            _logger.LogError("Error creating Kubernetes secrets. {StatusCode}: {Content}",
+                                createResponse.StatusCode,
+                                await createResponse.Content.ReadAsStringAsync());
+                        }
                         createResponse.EnsureSuccessStatusCode();
                     }
                 }
diff --git a/test/WebJobs.Script.Tests/Security/SimpleKubernetesClientTests.cs b/test/WebJobs.Script.Tests/Security/SimpleKubernetesClientTests.cs
@@ -0,0 +1,90 @@
+// 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.IO;
+using System.IO.Abstractions;
+using System.Net;
+using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Azure.WebJobs.Script.WebHost;
+using Microsoft.Extensions.Logging;
+using Microsoft.WebJobs.Script.Tests;
+using Moq;
+using Moq.Protected;
+using Xunit;
+
+namespace Microsoft.Azure.WebJobs.Script.Tests
+{
+    public class SimpleKubernetesClientTests : IDisposable
+    {
+        [Theory]
+        [InlineData(HttpStatusCode.OK, "{}", 0)]
+        [InlineData(HttpStatusCode.OK, "{'data': {}}", 0)]
+        [InlineData(HttpStatusCode.OK, "{'data': {'key': 'dmFsdWU='}}", 1)]
+        public async Task Get_From_ApiServer_No_Data(HttpStatusCode statusCode, string content, int length)
+        {
+            var environment = new TestEnvironment();
+            environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebJobsKubernetesSecretName, "test");
+            environment.SetEnvironmentVariable(EnvironmentSettingNames.KubernetesServiceHost, "127.0.0.1");
+            environment.SetEnvironmentVariable(EnvironmentSettingNames.KubernetesServiceHttpsPort, "443");
+
+            var fullFileSystem = new FileSystem();
+            var fileSystem = new Mock<IFileSystem>();
+            var fileBase = new Mock<FileBase>();
+            var directoryBase = new Mock<DirectoryBase>();
+
+            fileSystem.SetupGet(f => f.Path).Returns(fullFileSystem.Path);
+            fileSystem.SetupGet(f => f.File).Returns(fileBase.Object);
+            fileSystem.SetupGet(f => f.Directory).Returns(directoryBase.Object);
+            fileBase.Setup(f => f.Exists("/run/secrets/kubernetes.io/serviceaccount/namespace")).Returns(true);
+            fileBase.Setup(f => f.Exists("/run/secrets/kubernetes.io/serviceaccount/token")).Returns(true);
+            fileBase.Setup(f => f.Exists("/run/secrets/kubernetes.io/serviceaccount/ca.crt")).Returns(true);
+
+            fileBase
+                .Setup(f => f.Open("/run/secrets/kubernetes.io/serviceaccount/token", It.IsAny<FileMode>(), It.IsAny<FileAccess>(), It.IsAny<FileShare>()))
+                .Returns(() =>
+                {
+                    var token = new MemoryStream(Encoding.UTF8.GetBytes("test_token"));
+                    token.Position = 0;
+                    return token;
+                });
+            fileBase
+                .Setup(f => f.Open("/run/secrets/kubernetes.io/serviceaccount/namespace", It.IsAny<FileMode>(), It.IsAny<FileAccess>(), It.IsAny<FileShare>()))
+                .Returns(() =>
+                {
+                    var ns = new MemoryStream(Encoding.UTF8.GetBytes("namespace"));
+                    ns.Position = 0;
+                    return ns;
+                });
+
+            FileUtility.Instance = fileSystem.Object;
+
+            var loggerFactory = new LoggerFactory();
+            var loggerProvider = new TestLoggerProvider();
+            loggerFactory.AddProvider(loggerProvider);
+
+            var handlerMock = new Mock<HttpMessageHandler>(MockBehavior.Strict);
+            handlerMock.Protected().Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>()).ReturnsAsync(new HttpResponseMessage
+            {
+                StatusCode = statusCode,
+
+                Content = new StringContent(content, Encoding.UTF8, "application/json")
+            });
+
+            var client = new SimpleKubernetesClient(environment, new HttpClient(handlerMock.Object), loggerFactory.CreateLogger<SimpleKubernetesClient>());
+            var secrets = await client.GetSecrets();
+
+            Assert.NotNull(secrets);
+            Assert.Equal(secrets.Count, length);
+        }
+
+        public void Dispose()
+        {
+            FileUtility.Instance = null;
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ internal ISecretsRepository CreateSecretsRepository()`
`72`	`72`	`}`
`73`	`73`	`else if (secretStorageType != null && secretStorageType.Equals("kubernetes", StringComparison.OrdinalIgnoreCase))`
`74`	`74`	`{`
`75`		`- return new KubernetesSecretsRepository(_environment, new SimpleKubernetesClient(_environment));`
	`75`	`+ return new KubernetesSecretsRepository(_environment, new SimpleKubernetesClient(_environment, _loggerFactory.CreateLogger<SimpleKubernetesClient>()));`
`76`	`76`	`}`
`77`	`77`	`else if (secretStorageSas != null)`
`78`	`78`	`{`
Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@`
`13`	`13`	`using System.Threading;`
`14`	`14`	`using System.Threading.Tasks;`
`15`	`15`	`using Microsoft.Azure.WebJobs.Script.IO;`
	`16`	`+using Microsoft.Extensions.Logging;`
`16`	`17`	`using Microsoft.Net.Http.Headers;`
`17`	`18`	`using Newtonsoft.Json;`
`18`	`19`	`using Newtonsoft.Json.Linq;`
`@@ -27,18 +28,20 @@ public class SimpleKubernetesClient : IKubernetesClient, IDisposable`
`27`	`28`	`private const string KubernetesSecretsDir = "/run/secrets/functions-keys";`
`28`	`29`	`private readonly HttpClient _httpClient;`
`29`	`30`	`private readonly IEnvironment _environment;`
	`31`	`+ private readonly ILogger _logger;`
`30`	`32`	`private Action _watchCallback;`
`31`	`33`	`private bool _disposed;`
`32`	`34`	`private AutoRecoveringFileSystemWatcher _fileWatcher;`
`33`	`35`
`34`		`- public SimpleKubernetesClient(IEnvironment environment) : this(environment, CreateHttpClient())`
	`36`	`+ public SimpleKubernetesClient(IEnvironment environment, ILogger logger) : this(environment, CreateHttpClient(), logger)`
`35`	`37`	`{ }`
`36`	`38`
`37`	`39`	`// for testing`
`38`		`- internal SimpleKubernetesClient(IEnvironment environment, HttpClient client)`
	`40`	`+ internal SimpleKubernetesClient(IEnvironment environment, HttpClient client, ILogger logger)`
`39`	`41`	`{`
`40`	`42`	`_httpClient = client;`
`41`	`43`	`_environment = environment;`
	`44`	`+ _logger = logger;`
`42`	`45`	`Task.Run(() => RunWatcher());`
`43`	`46`	`}`
`44`	`47`
`@@ -58,7 +61,9 @@ public async Task<IDictionary<string, string>> GetSecrets()`
`58`	`61`	`}`
`59`	`62`	`else`
`60`	`63`	`{`
`61`		`- throw new InvalidOperationException($"{nameof(KubernetesSecretsRepository)} requires setting {EnvironmentSettingNames.AzureWebJobsKubernetesSecretName} or mounting secrets to {KubernetesSecretsDir}");`
	`64`	`+ var exception = new InvalidOperationException($"{nameof(KubernetesSecretsRepository)} requires setting {EnvironmentSettingNames.AzureWebJobsKubernetesSecretName} or mounting secrets to {KubernetesSecretsDir}");`
	`65`	`+ _logger.LogError(exception, "Error geting secrets");`
	`66`	`+ throw exception;`
`62`	`67`	`}`
`63`	`68`	`}`
`64`	`69`
`@@ -74,6 +79,12 @@ public async Task UpdateSecrets(IDictionary<string, string> data)`
`74`	`79`	`using (var request = await GetRequest(HttpMethod.Patch, url, new[] { new { op = "replace", path = "/data", value = data } }, "application/json-patch+json"))`
`75`	`80`	`{`
`76`	`81`	`var response = await _httpClient.SendAsync(request);`
	`82`	`+ if (!response.IsSuccessStatusCode)`
	`83`	`+ {`
	`84`	`+ _logger.LogError("Error updating Kubernetes secrets. {StatusCode}: {Content}",`
	`85`	`+ response.StatusCode,`
	`86`	`+ await response.Content.ReadAsStringAsync());`
	`87`	`+ }`
`77`	`88`	`response.EnsureSuccessStatusCode();`
`78`	`89`	`}`
`79`	`90`	`}`
`@@ -84,6 +95,15 @@ public void OnSecretChange(Action callback)`
`84`	`95`	`}`
`85`	`96`
`86`	`97`	`private async Task RunWatcher()`
	`98`	`+ {`
	`99`	`+ while (!_disposed)`
	`100`	`+ {`
	`101`	`+ // watch API requests terminate after 4 minutes`
	`102`	`+ await RunWatcherInternal();`
	`103`	`+ }`
	`104`	`+ }`
	`105`	`+`
	`106`	`+ private async Task RunWatcherInternal()`
`87`	`107`	`{`
`88`	`108`	`if (string.IsNullOrEmpty(KubernetesObjectName) && FileUtility.DirectoryExists(KubernetesSecretsDir))`
`89`	`109`	`{`
`@@ -124,13 +144,15 @@ private async Task<IDictionary<string, string>> GetFromApiServer(string objectNa`
`124`	`144`	`if (response.IsSuccessStatusCode)`
`125`	`145`	`{`
`126`	`146`	`var obj = await response.Content.ReadAsAsync<JObject>();`
`127`		`- return obj["data"]`
`128`		`- ?.ToObject<IDictionary<string, string>>()`
`129`		`- ?.ToDictionary(`
	`147`	`+ return obj.ContainsKey("data")`
	`148`	`+ ? obj["data"]`
	`149`	`+ .ToObject<IDictionary<string, string>>()`
	`150`	`+ .ToDictionary(`
`130`	`151`	`k => k.Key,`
`131`	`152`	`v => decode`
`132`	`153`	`? Encoding.UTF8.GetString(Convert.FromBase64String(v.Value))`
`133`		`- : v.Value);`
	`154`	`+ : v.Value)`
	`155`	`+ : new Dictionary<string, string>();`
`134`	`156`	`}`
`135`	`157`	`else if (response.StatusCode == HttpStatusCode.NotFound)`
`136`	`158`	`{`
`@@ -139,7 +161,9 @@ private async Task<IDictionary<string, string>> GetFromApiServer(string objectNa`
`139`	`161`	`}`
`140`	`162`	`else`
`141`	`163`	`{`
`142`		`- throw new HttpRequestException($"Error calling GET {url}, Status: {response.StatusCode}, Content: {await response.Content.ReadAsStringAsync()}");`
	`164`	`+ var exception = new HttpRequestException($"Error calling GET {url}, Status: {response.StatusCode}, Content: {await response.Content.ReadAsStringAsync()}");`
	`165`	`+ _logger.LogError(exception, "Error reading secrets from Kubernetes");`
	`166`	`+ throw exception;`
`143`	`167`	`}`
`144`	`168`	`}`
`145`	`169`	`}`
`@@ -165,6 +189,12 @@ private async Task CreateIfDoesntExist(string url, bool isSecret)`
`165`	`189`	`using (var createRequest = await GetRequest(HttpMethod.Post, url, payload))`
`166`	`190`	`{`
`167`	`191`	`var createResponse = await _httpClient.SendAsync(createRequest);`
	`192`	`+ if (!createResponse.IsSuccessStatusCode)`
	`193`	`+ {`
	`194`	`+ _logger.LogError("Error creating Kubernetes secrets. {StatusCode}: {Content}",`
	`195`	`+ createResponse.StatusCode,`
	`196`	`+ await createResponse.Content.ReadAsStringAsync());`
	`197`	`+ }`
`168`	`198`	`createResponse.EnsureSuccessStatusCode();`
`169`	`199`	`}`
`170`	`200`	`}`