From 61fafcfb98b0597e678bd89c7148fb6c2450c7de Mon Sep 17 00:00:00 2001 From: Jesus Aguilar <3589801+giventocode@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:02:05 -0400 Subject: [PATCH 01/13] wip initial commit --- src/Tes.ApiClients/DrsHubApiClient.cs | 22 +++++++++ .../Storage/DrsTransformationStrategy.cs | 49 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 src/Tes.ApiClients/DrsHubApiClient.cs create mode 100644 src/Tes.Runner/Storage/DrsTransformationStrategy.cs diff --git a/src/Tes.ApiClients/DrsHubApiClient.cs b/src/Tes.ApiClients/DrsHubApiClient.cs new file mode 100644 index 000000000..57c7e17ff --- /dev/null +++ b/src/Tes.ApiClients/DrsHubApiClient.cs @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Tes.ApiClients +{ + public class DrsHubApiClient : HttpApiClient + { + private readonly Uri drsHubUrl; + + public DrsHubApiClient(Uri drsHubUrl ) { + ArgumentNullException.ThrowIfNull( drsHubUrl ); + + this.drsHubUrl = drsHubUrl; + } + } +} diff --git a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs new file mode 100644 index 000000000..b01fa0229 --- /dev/null +++ b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Azure.Storage.Sas; +using Microsoft.Extensions.Logging; +using Tes.Runner.Transfer; + +namespace Tes.Runner.Storage +{ + internal class DrsTransformationStrategy : IUrlTransformationStrategy + { + private readonly ILogger logger = PipelineLoggerFactory.Create(); + private static DrsHubApiClient drsHubApiClient; + private const string DrsScheme = "drs://"; + + + private readonly Uri drsHubUri; + + public DrsTransformationStrategy(string drsHubUrl) + { + ArgumentException.ThrowIfNullOrWhiteSpace(drsHubUrl); + drsHubApiClient = new DrsHubApiClient(drsHubUrl); + + drsHubUri = new Uri(drsHubUrl); + } + + public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPermissions blobSasPermissions = 0) + { + var sourceUri = new Uri(sourceUrl); + + if(!ContainsDrsScheme(sourceUri.Scheme)) + { + return sourceUri; + } + + + } + + private bool ContainsDrsScheme(string scheme) + { + return scheme.Equals(DrsScheme, StringComparison.OrdinalIgnoreCase); + } + } +} From 2311bc8e2691bfe7b2b6e9bf3f5abbb7151e4382 Mon Sep 17 00:00:00 2001 From: Jesus Aguilar <3589801+giventocode@users.noreply.github.com> Date: Mon, 25 Mar 2024 12:50:18 -0400 Subject: [PATCH 02/13] api models --- src/Tes.ApiClients/DrsHubApiClient.cs | 14 ++++ src/Tes.ApiClients/HttpApiClient.cs | 8 +++ .../Models/Terra/DrsResolveApiRequest.cs | 35 +++++++++ .../Models/Terra/DrsResolveApiResponse.cs | 71 +++++++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs create mode 100644 src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs diff --git a/src/Tes.ApiClients/DrsHubApiClient.cs b/src/Tes.ApiClients/DrsHubApiClient.cs index 57c7e17ff..e695cf4d3 100644 --- a/src/Tes.ApiClients/DrsHubApiClient.cs +++ b/src/Tes.ApiClients/DrsHubApiClient.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; +using Azure.Core; namespace Tes.ApiClients { @@ -18,5 +19,18 @@ public DrsHubApiClient(Uri drsHubUrl ) { this.drsHubUrl = drsHubUrl; } + + private HttpRequestMessage CreateResolveDrsRequest() { + + var requestUri = new Uri(drsHubUrl, new Uri("/api/v4/drs/resolve")); + + var request = new HttpRequestMessage(); + + + request.Method = RequestMethod.Post; + var uri = new RawRequestUriBuilder(); + uri.Reset(_endpoint); + uri.AppendPath("/api/v4/drs/resolve", false); + } } } diff --git a/src/Tes.ApiClients/HttpApiClient.cs b/src/Tes.ApiClients/HttpApiClient.cs index 58e325f4d..b1d0957f1 100644 --- a/src/Tes.ApiClients/HttpApiClient.cs +++ b/src/Tes.ApiClients/HttpApiClient.cs @@ -5,6 +5,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.Json; +using System.Text.Json.Serialization; using Azure.Core; using CommonUtilities; using Microsoft.Extensions.Logging; @@ -364,5 +365,12 @@ protected static async Task ReadResponseBodyAsync(HttpResponseMessage re { return await response.Content.ReadAsStringAsync(cancellationToken); } + + protected static HttpContent CreateJsonStringContent(T contentObject) { + var stringContent = new StringContent(JsonSerializer.Serialize(contentObject, + new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }), Encoding.UTF8, "application/json"); + + return stringContent; + } } } diff --git a/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs b/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs new file mode 100644 index 000000000..b08b6690c --- /dev/null +++ b/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace Tes.ApiClients.Models.Terra +{ + + public class DrsResolveRequestContent + { + [JsonPropertyName("url")] + public string Url { get; set; } + + [JsonPropertyName("cloudPlatform")] + [JsonConverter(typeof(JsonStringEnumConverter))] + public CloudPlatform CloudPlatform { get; set; } + + [JsonPropertyName("fields")] + public List Fields { get; set; } + } + public enum CloudPlatform + { + [JsonPropertyName("azure")] + Azure, + [JsonPropertyName("google")] + Google + } + +} + + + diff --git a/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs b/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs new file mode 100644 index 000000000..863ee3ab6 --- /dev/null +++ b/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Text.Json.Serialization; +using System.Threading.Tasks; + +namespace Tes.ApiClients.Models.Terra +{ + + public class DrsResolveApiResponse + { + [JsonPropertyName("contentType")] + public string ContentType { get; set; } + + [JsonPropertyName("size")] + public long Size { get; set; } + + [JsonPropertyName("timeCreated")] + public DateTime TimeCreated { get; set; } + + [JsonPropertyName("timeUpdated")] + public DateTime TimeUpdated { get; set; } + + [JsonPropertyName("bucket")] + public string Bucket { get; set; } + + [JsonPropertyName("name")] + public string Name { get; set; } + + [JsonPropertyName("gsUri")] + public string GsUri { get; set; } + + [JsonPropertyName("googleServiceAccount")] + public SaKeyObject GoogleServiceAccount { get; set; } + + [JsonPropertyName("fileName")] + public string FileName { get; set; } + + [JsonPropertyName("accessUrl")] + public AccessUrl AccessUrl { get; set; } + + [JsonPropertyName("hashes")] + public Dictionary Hashes { get; set; } + + [JsonPropertyName("localizationPath")] + public string LocalizationPath { get; set; } + + [JsonPropertyName("bondProvider")] + public string BondProvider { get; set; } + } + + public class SaKeyObject + { + [JsonPropertyName("data")] + public Dictionary Data { get; set; } + } + + public class AccessUrl + { + [JsonPropertyName("url")] + public string Url { get; set; } + + [JsonPropertyName("headers")] + public Dictionary Headers { get; set; } + } + +} From c26060500fcd4e9422b304f5f590984fd60a3a57 Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Tue, 26 Mar 2024 21:42:31 -0400 Subject: [PATCH 03/13] drs hub api client --- src/CommonUtilities/Models/NodeTask.cs | 1 + .../DrsHubApiClientTests.cs | 83 +++++++++++++++++ src/Tes.ApiClients/DrsHubApiClient.cs | 93 +++++++++++++++---- src/Tes.ApiClients/HttpApiClient.cs | 46 ++++++--- src/Tes.ApiClients/PriceApiClient.cs | 4 +- src/Tes.ApiClients/TerraApiClient.cs | 22 ++++- src/Tes.ApiClients/TerraWsmApiClient.cs | 21 +---- .../Storage/DrsTransformationStrategy.cs | 34 ++++--- .../Storage/TerraConfigConstants.cs | 7 ++ .../Storage/TerraUrlTransformationStrategy.cs | 12 +-- 10 files changed, 247 insertions(+), 76 deletions(-) create mode 100644 src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs create mode 100644 src/Tes.Runner/Storage/TerraConfigConstants.cs diff --git a/src/CommonUtilities/Models/NodeTask.cs b/src/CommonUtilities/Models/NodeTask.cs index d4d191bd3..8d697b209 100644 --- a/src/CommonUtilities/Models/NodeTask.cs +++ b/src/CommonUtilities/Models/NodeTask.cs @@ -63,6 +63,7 @@ public class TerraRuntimeOptions public string? WsmApiHost { get; set; } public string? LandingZoneApiHost { get; set; } public string? SasAllowedIpRange { get; set; } + public string? DrsHubApiHost { get; set; } } [JsonConverter(typeof(JsonStringEnumConverter))] diff --git a/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs b/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs new file mode 100644 index 000000000..2f668e360 --- /dev/null +++ b/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Core; +using CommonUtilities.Options; +using Microsoft.Azure.Management.Compute.Fluent.Models; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Moq; + +namespace Tes.ApiClients.Tests +{ + [TestClass] + [TestCategory("Unit")] + public class DrsHubApiClientTests + { + private Mock tokenCredentialsMock; + private CachingRetryPolicyBuilder cachingRetryPolicyBuilder; + private CommonUtilities.AzureEnvironmentConfig azureEnvironmentConfig; + private DrsHubApiClient apiClient; + + private const string DrsApiHost = "https://drshub.foo"; + + [TestInitialize] + public void Setup() + { + var retryPolicyOptions = new RetryPolicyOptions(); + var appCache = new MemoryCache(new MemoryCacheOptions()); + cachingRetryPolicyBuilder = new CachingRetryPolicyBuilder(appCache, Options.Create(retryPolicyOptions)); + + tokenCredentialsMock = new Mock(); + azureEnvironmentConfig = new CommonUtilities.AzureEnvironmentConfig() + { + TokenScope = "https://management.azure.com/.default" + }; + apiClient = new DrsHubApiClient(DrsApiHost, tokenCredentialsMock.Object, cachingRetryPolicyBuilder, azureEnvironmentConfig, NullLogger.Instance); + } + + [TestMethod] + public void CreateDrsHubApiClient_ReturnsValidDrsApiClient() + { + var drsApiClient = DrsHubApiClient.CreateDrsHubApiClient("https://drshub.foo", tokenCredentialsMock.Object, azureEnvironmentConfig); + + Assert.IsNotNull(drsApiClient); + } + + [TestMethod] + public async Task GetDrsResolveRequestContent_ValidDrsUri_ReturnsValidRequestContentWithExpectedValues() + { + var drsUriString = "drs://drs.foo"; + var drsUri = new Uri(drsUriString); + var content = await apiClient.GetDrsResolveRequestContent(drsUri).ReadAsStringAsync(); + + Assert.IsNotNull(ExpectedDrsResolveRequestJson, content); + } + + [TestMethod] + public async Task GetDrsResolveApiResponse_ResponseWithAccessUrl_CanDeserializeJSon() { + var httpResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK); + httpResponse.Content = new StringContent(ExpectedRsResolveResponseJson); + + var drsResolveResponse = await DrsHubApiClient.GetDrsResolveApiResponseAsync(httpResponse, CancellationToken.None); + + Assert.IsNotNull(drsResolveResponse); + Assert.IsNotNull(drsResolveResponse.AccessUrl); + Assert.AreEqual("https://storage.foo/bar",drsResolveResponse.AccessUrl.Url); + } + + private const string ExpectedRsResolveResponseJson = @"{ + ""accessUrl"": { + ""url"": ""https://storage.foo/bar"", + ""headers"": null + } + }"; + + private const string ExpectedDrsResolveRequestJson = @"{ + ""url"": ""drs://drs.foo"", + ""cloudPlatform"": ""azure"", + ""fields"":[""accessUrl""] + }"; + } +} diff --git a/src/Tes.ApiClients/DrsHubApiClient.cs b/src/Tes.ApiClients/DrsHubApiClient.cs index e695cf4d3..bc05d74b9 100644 --- a/src/Tes.ApiClients/DrsHubApiClient.cs +++ b/src/Tes.ApiClients/DrsHubApiClient.cs @@ -1,36 +1,93 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; using Azure.Core; +using CommonUtilities; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging; +using Tes.ApiClients.Models.Terra; namespace Tes.ApiClients { - public class DrsHubApiClient : HttpApiClient + public class DrsHubApiClient : TerraApiClient { - private readonly Uri drsHubUrl; + private const string DrsHubApiSegments = "/api/v4/drs"; + private static readonly IMemoryCache SharedMemoryCache = new MemoryCache(new MemoryCacheOptions()); - public DrsHubApiClient(Uri drsHubUrl ) { - ArgumentNullException.ThrowIfNull( drsHubUrl ); + /// + /// Constructor of DrsHubApiClient + /// + /// WSM API host + /// + /// + /// + /// + public DrsHubApiClient(string apiUrl, TokenCredential tokenCredential, CachingRetryPolicyBuilder cachingRetryPolicyBuilder, + AzureEnvironmentConfig azureCloudIdentityConfig, ILogger logger) : base(apiUrl, tokenCredential, cachingRetryPolicyBuilder, azureCloudIdentityConfig, logger) + { - this.drsHubUrl = drsHubUrl; } - private HttpRequestMessage CreateResolveDrsRequest() { + public static DrsHubApiClient CreateDrsHubApiClient(string apiUrl, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) + { + return CreateTerraApiClient(apiUrl, SharedMemoryCache, tokenCredential,azureCloudIdentityConfig); + } + + public virtual async Task ResolveDrsUriAsync(Uri drsUri, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(drsUri); + + HttpResponseMessage response = null!; + try + { + var apiUrl = GetResolveDrsApiUrl(); + + Logger.LogInformation(@"Resolving DRS URI calling: {uri}", apiUrl); + + response = + await HttpSendRequestWithRetryPolicyAsync(() => new HttpRequestMessage(HttpMethod.Post, apiUrl) { Content = GetDrsResolveRequestContent(drsUri) }, + cancellationToken, setAuthorizationHeader: true); + + var apiResponse = await GetDrsResolveApiResponseAsync(response, cancellationToken); + + Logger.LogInformation(@"Successfully resolved URI: {drsUri}", drsUri); + + return apiResponse; + } + catch (Exception ex) + { + await LogResponseContentAsync(response, "Failed to resolve DRS URI", ex, cancellationToken); + throw; + } + } + + public static async Task GetDrsResolveApiResponseAsync(HttpResponseMessage response, CancellationToken cancellationToken) + { + var apiResponse = await GetApiResponseContentAsync(response, cancellationToken); + return apiResponse; + } + + public HttpContent GetDrsResolveRequestContent(Uri drsUri) + { + ArgumentNullException.ThrowIfNull(drsUri); + + var drsResolveApiRequestBody = new DrsResolveRequestContent + { + Url = drsUri.AbsoluteUri, + CloudPlatform = CloudPlatform.Azure, + Fields = new List { "accessUrl" } + }; + + return CreateJsonStringContent(drsResolveApiRequestBody); + } - var requestUri = new Uri(drsHubUrl, new Uri("/api/v4/drs/resolve")); + private string GetResolveDrsApiUrl() + { + var apiRequestUrl = $"{ApiUrl.TrimEnd('/')}{DrsHubApiSegments}/resolve"; - var request = new HttpRequestMessage(); - + var builder = new UriBuilder(apiRequestUrl); - request.Method = RequestMethod.Post; - var uri = new RawRequestUriBuilder(); - uri.Reset(_endpoint); - uri.AppendPath("/api/v4/drs/resolve", false); + return builder.Uri.AbsoluteUri; } } } diff --git a/src/Tes.ApiClients/HttpApiClient.cs b/src/Tes.ApiClients/HttpApiClient.cs index b1d0957f1..bd5664522 100644 --- a/src/Tes.ApiClients/HttpApiClient.cs +++ b/src/Tes.ApiClients/HttpApiClient.cs @@ -28,7 +28,7 @@ public abstract class HttpApiClient private readonly SemaphoreSlim semaphore = new(1, 1); private AccessToken accessToken; - protected readonly CachingRetryHandler.CachingAsyncRetryHandlerPolicy cachingRetryHandler; + protected readonly CachingRetryHandler.CachingAsyncRetryHandlerPolicy cachingRetryPolicy; /// /// Inner http client. @@ -38,38 +38,38 @@ public abstract class HttpApiClient /// /// Constructor of base HttpApiClient /// - /// + /// /// - protected HttpApiClient(CachingRetryPolicyBuilder cachingRetryBuilder, ILogger logger) + protected HttpApiClient(CachingRetryPolicyBuilder cachingRetryPolicyBuilder, ILogger logger) { - ArgumentNullException.ThrowIfNull(cachingRetryBuilder); + ArgumentNullException.ThrowIfNull(cachingRetryPolicyBuilder); ArgumentNullException.ThrowIfNull(logger); this.Logger = logger; - cachingRetryHandler = cachingRetryBuilder + cachingRetryPolicy = cachingRetryPolicyBuilder .DefaultRetryHttpResponseMessagePolicyBuilder() .SetOnRetryBehavior(onRetry: LogRetryErrorOnRetryHttpResponseMessageHandler()) .AddCaching() .AsyncBuild(); } - + /// /// Constructor of base HttpApiClient /// /// - /// + /// /// /// protected HttpApiClient(TokenCredential tokenCredential, string tokenScope, - CachingRetryPolicyBuilder cachingRetryHandler, ILogger logger) : this(cachingRetryHandler, logger) + CachingRetryPolicyBuilder cachingRetryPolicyBuilder, ILogger logger) : this(cachingRetryPolicyBuilder, logger) { ArgumentNullException.ThrowIfNull(tokenCredential); ArgumentException.ThrowIfNullOrEmpty(tokenScope); this.tokenCredential = tokenCredential; this.tokenScope = tokenScope; - } + } /// /// Protected parameter-less constructor of HttpApiClient @@ -81,7 +81,7 @@ protected HttpApiClient() { } /// /// private RetryHandler.OnRetryHandler LogRetryErrorOnRetryHttpResponseMessageHandler() - => new((result, timeSpan, retryCount, correlationId, caller) => + => (result, timeSpan, retryCount, correlationId, caller) => { if (result.Exception is null) { @@ -93,7 +93,7 @@ private RetryHandler.OnRetryHandler LogRetryErrorOnRetryHtt Logger?.LogError(result.Exception, @"Retrying in {Method} due to '{Message}': RetryCount: {RetryCount} TimeSpan: {TimeSpan} CorrelationId: {CorrelationId}", caller, result.Exception.Message, retryCount, timeSpan.ToString("c"), correlationId.ToString("D")); } - }); + }; /// /// Sends request with a retry policy @@ -105,7 +105,7 @@ private RetryHandler.OnRetryHandler LogRetryErrorOnRetryHtt /// protected async Task HttpSendRequestWithRetryPolicyAsync( Func httpRequestFactory, CancellationToken cancellationToken, bool setAuthorizationHeader = false) - => await cachingRetryHandler.ExecuteWithRetryAsync(async ct => + => await cachingRetryPolicy.ExecuteWithRetryAsync(async ct => { var request = httpRequestFactory(); @@ -150,7 +150,7 @@ protected async Task HttpGetRequestWithCachingAndRetryPolicyAsync + return (await cachingRetryPolicy.ExecuteWithRetryConversionAndCachingAsync(cacheKey, async ct => { //request must be recreated in every retry. var httpRequest = await CreateGetHttpRequest(requestUrl, setAuthorizationHeader, ct); @@ -170,7 +170,7 @@ protected async Task HttpGetRequestWithCachingAndRetryPolicyAsync protected async Task HttpGetRequestWithRetryPolicyAsync(Uri requestUrl, CancellationToken cancellationToken, bool setAuthorizationHeader = false) - => await cachingRetryHandler.ExecuteWithRetryAndConversionAsync(async ct => + => await cachingRetryPolicy.ExecuteWithRetryAndConversionAsync(async ct => { //request must be recreated in every retry. var httpRequest = await CreateGetHttpRequest(requestUrl, setAuthorizationHeader, ct); @@ -248,7 +248,7 @@ private async Task CreateGetHttpRequest(Uri requestUrl, bool protected async Task HttpGetRequestWithRetryPolicyAsync( Func httpRequestFactory, CancellationToken cancellationToken, bool setAuthorizationHeader = false) { - return await cachingRetryHandler.ExecuteWithRetryAndConversionAsync(async ct => + return await cachingRetryPolicy.ExecuteWithRetryAndConversionAsync(async ct => { var request = httpRequestFactory(); @@ -366,11 +366,27 @@ protected static async Task ReadResponseBodyAsync(HttpResponseMessage re return await response.Content.ReadAsStringAsync(cancellationToken); } + + /// + /// Creates a string content from an object + /// protected static HttpContent CreateJsonStringContent(T contentObject) { var stringContent = new StringContent(JsonSerializer.Serialize(contentObject, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }), Encoding.UTF8, "application/json"); return stringContent; } + + protected async Task LogResponseContentAsync(HttpResponseMessage response, string errMessage, Exception ex, CancellationToken cancellationToken) + { + var responseContent = string.Empty; + + if (response is not null) + { + responseContent = await ReadResponseBodyAsync(response, cancellationToken); + } + + Logger.LogError(ex, @"{ErrorMessage}. Response content:{ResponseContent}", errMessage, responseContent); + } } } diff --git a/src/Tes.ApiClients/PriceApiClient.cs b/src/Tes.ApiClients/PriceApiClient.cs index dd4d549ef..06289a16b 100644 --- a/src/Tes.ApiClients/PriceApiClient.cs +++ b/src/Tes.ApiClients/PriceApiClient.cs @@ -17,9 +17,9 @@ public class PriceApiClient : HttpApiClient /// /// Constructor of the Price API Client. /// - /// + /// /// - public PriceApiClient(CachingRetryPolicyBuilder cachingRetryHandler, ILogger logger) : base(cachingRetryHandler, logger) + public PriceApiClient(CachingRetryPolicyBuilder cachingRetryPolicyHandler, ILogger logger) : base(cachingRetryPolicyHandler, logger) { } diff --git a/src/Tes.ApiClients/TerraApiClient.cs b/src/Tes.ApiClients/TerraApiClient.cs index 9a5df46c4..755084b1b 100644 --- a/src/Tes.ApiClients/TerraApiClient.cs +++ b/src/Tes.ApiClients/TerraApiClient.cs @@ -3,7 +3,11 @@ using Azure.Core; using CommonUtilities; +using CommonUtilities.Options; +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; +using Microsoft.WindowsAzure.Storage.Auth; +using TokenCredential = Azure.Core.TokenCredential; namespace Tes.ApiClients { @@ -24,17 +28,29 @@ protected TerraApiClient() { } /// /// API Host /// - /// + /// /// - protected TerraApiClient(string apiUrl, TokenCredential tokenCredential, CachingRetryPolicyBuilder cachingRetryHandler, AzureEnvironmentConfig azureCloudIdentityConfig, ILogger logger) : base(tokenCredential, azureCloudIdentityConfig.TokenScope, cachingRetryHandler, logger) + protected TerraApiClient(string apiUrl, TokenCredential tokenCredential, CachingRetryPolicyBuilder cachingRetryPolicyBuilder, AzureEnvironmentConfig azureCloudIdentityConfig, ILogger logger) : base(tokenCredential, azureCloudIdentityConfig.TokenScope, cachingRetryPolicyBuilder, logger) { ArgumentException.ThrowIfNullOrEmpty(apiUrl); ArgumentNullException.ThrowIfNull(tokenCredential); - ArgumentNullException.ThrowIfNull(cachingRetryHandler); + ArgumentNullException.ThrowIfNull(cachingRetryPolicyBuilder); ArgumentNullException.ThrowIfNull(azureCloudIdentityConfig); ArgumentNullException.ThrowIfNull(logger); ApiUrl = apiUrl; } + + protected static T CreateTerraApiClient(string apiUrl, IMemoryCache sharedMemoryCache, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) where T : TerraApiClient { + var retryPolicyOptions = new RetryPolicyOptions(); + var cacheRetryHandler = new CachingRetryPolicyBuilder(sharedMemoryCache, Microsoft.Extensions.Options.Options.Create(retryPolicyOptions)); + + return (T)Activator.CreateInstance(typeof(T), + apiUrl, + tokenCredential, + cacheRetryHandler, + azureCloudIdentityConfig, + ApiClientsLoggerFactory.Create()); + } } } diff --git a/src/Tes.ApiClients/TerraWsmApiClient.cs b/src/Tes.ApiClients/TerraWsmApiClient.cs index fe093c812..9ebc1381d 100644 --- a/src/Tes.ApiClients/TerraWsmApiClient.cs +++ b/src/Tes.ApiClients/TerraWsmApiClient.cs @@ -20,7 +20,7 @@ namespace Tes.ApiClients public class TerraWsmApiClient : TerraApiClient { private const string WsmApiSegments = @"/api/workspaces/v1"; - private static readonly IMemoryCache sharedMemoryCache = new MemoryCache(new MemoryCacheOptions()); + private static readonly IMemoryCache SharedMemoryCache = new MemoryCache(new MemoryCacheOptions()); /// /// Constructor of TerraWsmApiClient @@ -38,12 +38,7 @@ public TerraWsmApiClient(string apiUrl, TokenCredential tokenCredential, Caching public static TerraWsmApiClient CreateTerraWsmApiClient(string apiUrl, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) { - - var retryPolicyOptions = new RetryPolicyOptions(); - var cacheRetryHandler = new CachingRetryPolicyBuilder(sharedMemoryCache, - Microsoft.Extensions.Options.Options.Create(retryPolicyOptions)); - - return new TerraWsmApiClient(apiUrl, tokenCredential, cacheRetryHandler, azureCloudIdentityConfig, ApiClientsLoggerFactory.Create()); + return CreateTerraApiClient(apiUrl, SharedMemoryCache, tokenCredential, azureCloudIdentityConfig); } /// @@ -238,18 +233,6 @@ public virtual async Task GetLandingZoneResourc } - private async Task LogResponseContentAsync(HttpResponseMessage response, string errMessage, Exception ex, CancellationToken cancellationToken) - { - var responseContent = string.Empty; - - if (response is not null) - { - responseContent = await ReadResponseBodyAsync(response, cancellationToken); - } - - Logger.LogError(ex, @"{ErrorMessage}. Response content:{ResponseContent}", errMessage, responseContent); - } - private string GetCreateBatchPoolUrl(Guid workspaceId) { var segments = $"/resources/controlled/azure/batchpool"; diff --git a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs index b01fa0229..9555fe134 100644 --- a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs @@ -1,13 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +using Azure.Core; using Azure.Storage.Sas; +using CommonUtilities; using Microsoft.Extensions.Logging; +using Tes.ApiClients; +using Tes.Runner.Models; using Tes.Runner.Transfer; namespace Tes.Runner.Storage @@ -15,30 +14,43 @@ namespace Tes.Runner.Storage internal class DrsTransformationStrategy : IUrlTransformationStrategy { private readonly ILogger logger = PipelineLoggerFactory.Create(); - private static DrsHubApiClient drsHubApiClient; + private static DrsHubApiClient drsHubApiClient; private const string DrsScheme = "drs://"; private readonly Uri drsHubUri; - public DrsTransformationStrategy(string drsHubUrl) + public DrsTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig, int cacheExpirationInSeconds = TerraConfigConstants.CacheExpirationInSeconds) { - ArgumentException.ThrowIfNullOrWhiteSpace(drsHubUrl); - drsHubApiClient = new DrsHubApiClient(drsHubUrl); + ArgumentNullException.ThrowIfNull(terraRuntimeOptions); + ArgumentException.ThrowIfNullOrEmpty(terraRuntimeOptions.DrsHubApiHost, nameof(terraRuntimeOptions.DrsHubApiHost)); - drsHubUri = new Uri(drsHubUrl); + drsHubApiClient = DrsHubApiClient.CreateDrsHubApiClient(terraRuntimeOptions.DrsHubApiHost, tokenCredential, azureCloudIdentityConfig); } public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPermissions blobSasPermissions = 0) { var sourceUri = new Uri(sourceUrl); - if(!ContainsDrsScheme(sourceUri.Scheme)) + if (!ContainsDrsScheme(sourceUri.Scheme)) { return sourceUri; } + var response = await drsHubApiClient.ResolveDrsUriAsync(sourceUri); + try + { + var sasUri = new Uri(response.AccessUrl.Url); + + return sasUri; + } + catch (Exception e) + { + //we are not logging the response content here as it may contain sensitive information + logger.LogError(e, "The URL returned by DRSHub API is invalid."); + throw; + } } private bool ContainsDrsScheme(string scheme) diff --git a/src/Tes.Runner/Storage/TerraConfigConstants.cs b/src/Tes.Runner/Storage/TerraConfigConstants.cs new file mode 100644 index 000000000..941cbb79b --- /dev/null +++ b/src/Tes.Runner/Storage/TerraConfigConstants.cs @@ -0,0 +1,7 @@ +namespace Tes.Runner.Storage; + +public static class TerraConfigConstants +{ + public const int TokenExpirationInSeconds = 3600 * 24; //1 day, max time allowed by Terra. + public const int CacheExpirationInSeconds = TokenExpirationInSeconds - 1800; // 30 minutes less than token expiration +} \ No newline at end of file diff --git a/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs b/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs index ae510b899..31c682051 100644 --- a/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs @@ -18,9 +18,6 @@ namespace Tes.Runner.Storage { public class TerraUrlTransformationStrategy : IUrlTransformationStrategy { - public const int TokenExpirationInSeconds = 3600 * 24; //1 day, max time allowed by Terra. - public const int CacheExpirationInSeconds = TokenExpirationInSeconds - 1800; // 30 minutes less than token expiration - private const int MaxNumberOfContainerResources = 10000; private const string LzStorageAccountNamePattern = "lz[0-9a-f]*"; @@ -30,7 +27,7 @@ public class TerraUrlTransformationStrategy : IUrlTransformationStrategy private static IMemoryCache memoryCache = new MemoryCache(new MemoryCacheOptions()); private readonly int cacheExpirationInSeconds; - public TerraUrlTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig, int cacheExpirationInSeconds = CacheExpirationInSeconds) + public TerraUrlTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig, int cacheExpirationInSeconds = TerraConfigConstants.CacheExpirationInSeconds) { ArgumentNullException.ThrowIfNull(terraRuntimeOptions); ArgumentException.ThrowIfNullOrEmpty(terraRuntimeOptions.WsmApiHost, nameof(terraRuntimeOptions.WsmApiHost)); @@ -41,7 +38,7 @@ public TerraUrlTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, T } - public TerraUrlTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TerraWsmApiClient terraWsmApiClient, int cacheExpirationInSeconds = CacheExpirationInSeconds) + public TerraUrlTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TerraWsmApiClient terraWsmApiClient, int cacheExpirationInSeconds = TerraConfigConstants.CacheExpirationInSeconds) { ArgumentNullException.ThrowIfNull(terraRuntimeOptions); ArgumentNullException.ThrowIfNull(terraWsmApiClient); @@ -128,8 +125,7 @@ private async Task GetWorkspaceSasTokenFromWsmAsync(Terr private SasTokenApiParameters CreateTokenParamsFromOptions(BlobSasPermissions sasPermissions) { return new( - terraRuntimeOptions.SasAllowedIpRange ?? string.Empty, - TokenExpirationInSeconds, + terraRuntimeOptions.SasAllowedIpRange ?? string.Empty, TerraConfigConstants.TokenExpirationInSeconds, //setting blob name to empty string to get a SAS token for the container ToWsmBlobSasPermissions(sasPermissions), SasBlobName: String.Empty); } @@ -263,7 +259,7 @@ private async Task GetWsmContainerResourceIdFromCacheOrWsmAsync(Guid works var resourceId = Guid.Parse(metadata.ResourceId); - memoryCache.Set(cacheKey, resourceId, TimeSpan.FromSeconds(CacheExpirationInSeconds)); + memoryCache.Set(cacheKey, resourceId, TimeSpan.FromSeconds(TerraConfigConstants.CacheExpirationInSeconds)); return resourceId; } From 3f445dc4736cb1077162250a6a32cce146fda4b2 Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Tue, 26 Mar 2024 23:39:00 -0400 Subject: [PATCH 04/13] drs strategy and test --- .../DrsHubApiClientTests.cs | 9 ++- src/Tes.ApiClients/DrsHubApiClient.cs | 7 ++ .../Storage/DrsTransformationStrategyTests.cs | 69 +++++++++++++++++++ .../TerraUrlTransformationStrategyTests.cs | 12 +++- .../Storage/ArmUrlTransformationStrategy.cs | 9 +-- .../Storage/DrsTransformationStrategy.cs | 19 +++-- .../Storage/TerraUrlTransformationStrategy.cs | 8 +++ .../UrlTransformationStrategyFactory.cs | 2 + src/Tes.Runner/Transfer/BlobApiHttpUtils.cs | 12 ++++ 9 files changed, 127 insertions(+), 20 deletions(-) create mode 100644 src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs diff --git a/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs b/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs index 2f668e360..38310dade 100644 --- a/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs +++ b/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs @@ -3,7 +3,6 @@ using Azure.Core; using CommonUtilities.Options; -using Microsoft.Azure.Management.Compute.Fluent.Models; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; @@ -15,10 +14,10 @@ namespace Tes.ApiClients.Tests [TestCategory("Unit")] public class DrsHubApiClientTests { - private Mock tokenCredentialsMock; - private CachingRetryPolicyBuilder cachingRetryPolicyBuilder; - private CommonUtilities.AzureEnvironmentConfig azureEnvironmentConfig; - private DrsHubApiClient apiClient; + private Mock tokenCredentialsMock = null!; + private CachingRetryPolicyBuilder cachingRetryPolicyBuilder = null!; + private CommonUtilities.AzureEnvironmentConfig azureEnvironmentConfig = null!; + private DrsHubApiClient apiClient = null!; private const string DrsApiHost = "https://drshub.foo"; diff --git a/src/Tes.ApiClients/DrsHubApiClient.cs b/src/Tes.ApiClients/DrsHubApiClient.cs index bc05d74b9..02ec2cf52 100644 --- a/src/Tes.ApiClients/DrsHubApiClient.cs +++ b/src/Tes.ApiClients/DrsHubApiClient.cs @@ -14,6 +14,13 @@ public class DrsHubApiClient : TerraApiClient private const string DrsHubApiSegments = "/api/v4/drs"; private static readonly IMemoryCache SharedMemoryCache = new MemoryCache(new MemoryCacheOptions()); + /// + /// Parameterless constructor for mocking + /// + protected DrsHubApiClient() + { + } + /// /// Constructor of DrsHubApiClient /// diff --git a/src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs b/src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs new file mode 100644 index 000000000..6200974ae --- /dev/null +++ b/src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Core; +using CommonUtilities; +using Moq; +using Tes.ApiClients; +using Tes.ApiClients.Models.Terra; +using Tes.Runner.Models; +using Tes.Runner.Storage; + +namespace Tes.Runner.Tests.Storage +{ + [TestClass] + [TestCategory("Unit")] + public class DrsTransformationStrategyTests + { + private DrsTransformationStrategy drsTransformationStrategy = null!; + private Mock tokenCredentialsMock = null!; + private Mock drsHubApiClientMock = null!; + const string DrsHubApiHost = "https://drs-hub-api-host"; + + [TestInitialize] + public void SetUp() + { + tokenCredentialsMock = new Mock(); + var azureEnvConfig = new AzureEnvironmentConfig() + { + TokenScope = "https://management.azure.com/.default", + }; + var terraRuntimeOptions = new TerraRuntimeOptions() + { + DrsHubApiHost = DrsHubApiHost, + }; + drsHubApiClientMock = new Mock(); + drsTransformationStrategy = new DrsTransformationStrategy(drsHubApiClientMock.Object); + } + + [TestMethod] + public async Task TransformUriWithStrategyAsync_NonDrsUri_RerturnUriWithoutTransformation() + { + var uri = "https://non-drs-uri"; + + var transformedUri = await drsTransformationStrategy.TransformUrlWithStrategyAsync(uri); + + Assert.AreEqual(new Uri(uri), transformedUri); + } + + [TestMethod] + public async Task TransformUriWithStrategyAsync_DrsUri_CallsResolveApiAndReturnsResolvedUrl() + { + var drsUri = "drs://drs-uri"; + var resolvedUrl = "https://resolved-url"; + var response = new DrsResolveApiResponse() + { + AccessUrl = new AccessUrl() + { + Url = resolvedUrl + }, + }; + drsHubApiClientMock.Setup(x => x.ResolveDrsUriAsync(new Uri(drsUri), CancellationToken.None)).ReturnsAsync(response); + + var transformedUri = await drsTransformationStrategy.TransformUrlWithStrategyAsync(drsUri); + + Assert.AreEqual(new Uri(resolvedUrl), transformedUri); + } + + } +} diff --git a/src/Tes.Runner.Test/Storage/TerraUrlTransformationStrategyTests.cs b/src/Tes.Runner.Test/Storage/TerraUrlTransformationStrategyTests.cs index 9e4507972..5417e9400 100644 --- a/src/Tes.Runner.Test/Storage/TerraUrlTransformationStrategyTests.cs +++ b/src/Tes.Runner.Test/Storage/TerraUrlTransformationStrategyTests.cs @@ -101,7 +101,7 @@ public async Task TransformUrlWithStrategyAsync_BlobPermissionsProvided_AreConve Assert.AreEqual(wsmPermissions, capturedSasTokenApiParameters.SasPermission); } - [TestMethod] + [DataTestMethod] [DataRow("https://foo.blob.core.windows.net/container")] [DataRow("https://foo.bar.net/container")] [DataRow("https://foo.bar.net")] @@ -112,6 +112,16 @@ public async Task TransformUrlWithStrategyAsync_NonTerraStorageAccount_SourceUrl Assert.AreEqual(new Uri(sourceUrl).ToString(), sasUrl.ToString()); } + [TestMethod] + public async Task TransformUrlWithStrategyAsync_TerraStorageAccountWithSasToken_SourceUrlIsReturned() + { + var sourceUrl = $"{stubTerraBlobUrl}/blob?{StubSasToken}"; + + var sasUrl = await transformationStrategy.TransformUrlWithStrategyAsync(sourceUrl, BlobSasPermissions.Read); + + Assert.AreEqual(new Uri(sourceUrl).ToString(), sasUrl.ToString()); + } + [TestMethod] public async Task TransformUrlWithStrategyAsync_RequestsSasTokenMoreThanOnce_SasTokenIsCached() { diff --git a/src/Tes.Runner/Storage/ArmUrlTransformationStrategy.cs b/src/Tes.Runner/Storage/ArmUrlTransformationStrategy.cs index 4ff2826e5..b6b082e8f 100644 --- a/src/Tes.Runner/Storage/ArmUrlTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/ArmUrlTransformationStrategy.cs @@ -46,7 +46,7 @@ public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPe return uri; } - if (UrlContainsSasToken(sourceUrl)) + if (BlobApiHttpUtils.UrlContainsSasToken(sourceUrl)) { var uri = new Uri(sourceUrl); logger.LogWarning($"The URL provided has SAS token. The resolution strategy won't be applied. Host: {uri.Host}"); @@ -57,13 +57,6 @@ public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPe return await GetStorageUriWithSasTokenAsync(sourceUrl, blobSasPermissions); } - private bool UrlContainsSasToken(string sourceUrl) - { - var blobBuilder = new BlobUriBuilder(new Uri(sourceUrl)); - - return !string.IsNullOrWhiteSpace(blobBuilder?.Sas?.Signature); - } - private async Task GetStorageUriWithSasTokenAsync(string sourceUrl, BlobSasPermissions permissions) { try diff --git a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs index 9555fe134..bfb88fa60 100644 --- a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs @@ -11,16 +11,23 @@ namespace Tes.Runner.Storage { - internal class DrsTransformationStrategy : IUrlTransformationStrategy + /// + /// Terra DRS transformation strategy. Uses the configured DRSHub service to resolve a DRS URI. + /// If the URI is not a valid DRS URI, the URI is not transformed. + /// + public class DrsTransformationStrategy : IUrlTransformationStrategy { private readonly ILogger logger = PipelineLoggerFactory.Create(); - private static DrsHubApiClient drsHubApiClient; - private const string DrsScheme = "drs://"; + private readonly DrsHubApiClient drsHubApiClient; + private const string DrsScheme = "drs"; + public DrsTransformationStrategy(DrsHubApiClient drsHubApiClient) + { + ArgumentNullException.ThrowIfNull(drsHubApiClient); - private readonly Uri drsHubUri; - - public DrsTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig, int cacheExpirationInSeconds = TerraConfigConstants.CacheExpirationInSeconds) + this.drsHubApiClient = drsHubApiClient; + } + public DrsTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) { ArgumentNullException.ThrowIfNull(terraRuntimeOptions); ArgumentException.ThrowIfNullOrEmpty(terraRuntimeOptions.DrsHubApiHost, nameof(terraRuntimeOptions.DrsHubApiHost)); diff --git a/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs b/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs index 31c682051..94e89d0f9 100644 --- a/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/TerraUrlTransformationStrategy.cs @@ -58,6 +58,14 @@ public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPe return blobUriBuilder.ToUri(); } + if (BlobApiHttpUtils.UrlContainsSasToken(sourceUrl)) + { + var uri = new Uri(sourceUrl); + logger.LogWarning($"The URL provided has SAS token. The resolution strategy won't be applied. Host: {uri.Host}"); + + return uri; + } + var blobInfo = await GetTerraBlobInfoFromContainerNameAsync(sourceUrl); return await GetMappedSasUrlFromWsmAsync(blobInfo, blobSasPermissions); diff --git a/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs b/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs index 17cf586a9..cdc4c3062 100644 --- a/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs +++ b/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs @@ -27,12 +27,14 @@ public static IUrlTransformationStrategy CreateStrategy(TransformationStrategy t case TransformationStrategy.CombinedTerra: return new CombinedTransformationStrategy(new List { + new DrsTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), new CloudProviderSchemeConverter(), new TerraUrlTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), }); case TransformationStrategy.CombinedAzureResourceManager: return new CombinedTransformationStrategy(new List { + new DrsTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), new CloudProviderSchemeConverter(), new ArmUrlTransformationStrategy(u => new BlobServiceClient(u, TokenCredentialsManager.GetTokenCredential(runtimeOptions)), runtimeOptions) }); diff --git a/src/Tes.Runner/Transfer/BlobApiHttpUtils.cs b/src/Tes.Runner/Transfer/BlobApiHttpUtils.cs index fb67640c1..be5bcd85f 100644 --- a/src/Tes.Runner/Transfer/BlobApiHttpUtils.cs +++ b/src/Tes.Runner/Transfer/BlobApiHttpUtils.cs @@ -5,6 +5,7 @@ using System.Net.Http.Headers; using System.Net.Sockets; using System.Text; +using Azure.Storage.Blobs; using Microsoft.Extensions.Logging; using Polly; using Polly.Retry; @@ -164,6 +165,17 @@ public async Task ExecuteHttpRequestAsync(Func ExecuteHttpRequestImplAsync(requestFactory, ct), cancellationToken); } + public static bool UrlContainsSasToken(string sourceUrl) + { + if (string.IsNullOrWhiteSpace(sourceUrl)) + { + return false; + } + + var blobBuilder = new BlobUriBuilder(new Uri(sourceUrl)); + + return !string.IsNullOrWhiteSpace(blobBuilder?.Sas?.Signature); + } private async Task ExecuteHttpRequestImplAsync(Func request, CancellationToken cancellationToken) { HttpResponseMessage? response = null; From fe45ba2745ed4884f445e1724ba77d2c81eeb9cc Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Wed, 27 Mar 2024 11:55:14 -0400 Subject: [PATCH 05/13] wip commit for rebasing --- .../CachingRetryPolicyBuilder.cs | 2 +- .../TestServices/TestServiceProvider.cs | 2 +- src/TesApi.Web/Options/DrsHubOptions.cs | 21 ++++++++++ src/TesApi.Web/Options/MarthaOptions.cs | 38 ------------------- src/TesApi.Web/Runner/NodeTaskBuilder.cs | 20 ++++++++++ .../Runner/TaskToNodeTaskConverter.cs | 4 +- src/TesApi.Web/Startup.cs | 2 +- src/TesApi.Web/appsettings.json | 2 +- 8 files changed, 48 insertions(+), 43 deletions(-) create mode 100644 src/TesApi.Web/Options/DrsHubOptions.cs delete mode 100644 src/TesApi.Web/Options/MarthaOptions.cs diff --git a/src/Tes.ApiClients/CachingRetryPolicyBuilder.cs b/src/Tes.ApiClients/CachingRetryPolicyBuilder.cs index a7daf96df..ae90f8e61 100644 --- a/src/Tes.ApiClients/CachingRetryPolicyBuilder.cs +++ b/src/Tes.ApiClients/CachingRetryPolicyBuilder.cs @@ -12,7 +12,7 @@ namespace Tes.ApiClients /// /// Contains an App Cache instances and retry policies. /// - public partial class CachingRetryPolicyBuilder : RetryPolicyBuilder, CachingRetryPolicyBuilder.ICachingPolicyBuilderHandler + public class CachingRetryPolicyBuilder : RetryPolicyBuilder, CachingRetryPolicyBuilder.ICachingPolicyBuilderHandler { private readonly IMemoryCache appCache = null!; public virtual IMemoryCache AppCache => appCache; diff --git a/src/TesApi.Tests/TestServices/TestServiceProvider.cs b/src/TesApi.Tests/TestServices/TestServiceProvider.cs index 9ba776ecb..0891643fc 100644 --- a/src/TesApi.Tests/TestServices/TestServiceProvider.cs +++ b/src/TesApi.Tests/TestServices/TestServiceProvider.cs @@ -63,7 +63,7 @@ internal TestServiceProvider( .AddSingleton(BindHelper(BatchNodesOptions.SectionName)) .AddSingleton(BindHelper(BatchSchedulingOptions.SectionName)) .AddSingleton(BindHelper(StorageOptions.SectionName)) - .AddSingleton(BindHelper(MarthaOptions.SectionName)) + .AddSingleton(BindHelper(DrsHubOptions.SectionName)) .AddSingleton(s => wrapAzureProxy ? ActivatorUtilities.CreateInstance(s, GetAzureProxy(azureProxy).Object) : GetAzureProxy(azureProxy).Object) .AddSingleton(_ => GetTesTaskRepository(tesTaskRepository).Object) .AddSingleton(s => mockStorageAccessProvider ? GetStorageAccessProvider(storageAccessProvider).Object : ActivatorUtilities.CreateInstance(s)) diff --git a/src/TesApi.Web/Options/DrsHubOptions.cs b/src/TesApi.Web/Options/DrsHubOptions.cs new file mode 100644 index 000000000..fed65c46d --- /dev/null +++ b/src/TesApi.Web/Options/DrsHubOptions.cs @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace TesApi.Web.Options +{ + /// + /// Martha configuration options + /// + public class DrsHubOptions + { + /// + /// Martha configuration section + /// + public const string SectionName = "DrsHub"; + + /// + /// DrsHubUrl + /// + public string Url { get; set; } = string.Empty; + } +} diff --git a/src/TesApi.Web/Options/MarthaOptions.cs b/src/TesApi.Web/Options/MarthaOptions.cs deleted file mode 100644 index b0fb7eca9..000000000 --- a/src/TesApi.Web/Options/MarthaOptions.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -namespace TesApi.Web.Options -{ - /// - /// Martha configuration options - /// - public class MarthaOptions - { - /// - /// Martha configuration section - /// - public const string SectionName = "Martha"; - - /// - /// Default value for - /// - public const string DefaultCromwellDrsLocalizer = "broadinstitute/cromwell-drs-localizer:develop"; - - /// - /// Name of the cromwell drs locator image - /// - public string CromwellDrsLocalizer { get; set; } = DefaultCromwellDrsLocalizer; - /// - /// MarthaKeyVaultName - /// - public string KeyVaultName { get; set; } = string.Empty; - /// - /// MarthaSecretName - /// - public string SecretName { get; set; } = string.Empty; - /// - /// MarthaUrl - /// - public string Url { get; set; } = string.Empty; - } -} diff --git a/src/TesApi.Web/Runner/NodeTaskBuilder.cs b/src/TesApi.Web/Runner/NodeTaskBuilder.cs index ebdcba3e7..9107630ee 100644 --- a/src/TesApi.Web/Runner/NodeTaskBuilder.cs +++ b/src/TesApi.Web/Runner/NodeTaskBuilder.cs @@ -377,5 +377,25 @@ public NodeTaskBuilder WithLogPublisher(Uri targetUrl) return this; } + + /// + /// Adds DRS Hub URL to the node task, if the DRS Hub URL is not set, the property won't be set. + /// + /// + /// + /// + public NodeTaskBuilder WithDrsHubUrl(string drsHubApiHost) + { + if (String.IsNullOrWhiteSpace(drsHubApiHost)) + { + return this; + } + + nodeTask.RuntimeOptions ??= new RuntimeOptions(); + nodeTask.RuntimeOptions.Terra ??= new TerraRuntimeOptions(); + nodeTask.RuntimeOptions.Terra.DrsHubApiHost = drsHubApiHost; + + return this; + } } } diff --git a/src/TesApi.Web/Runner/TaskToNodeTaskConverter.cs b/src/TesApi.Web/Runner/TaskToNodeTaskConverter.cs index 5e6b8f317..0f6541e70 100644 --- a/src/TesApi.Web/Runner/TaskToNodeTaskConverter.cs +++ b/src/TesApi.Web/Runner/TaskToNodeTaskConverter.cs @@ -102,6 +102,7 @@ public virtual async Task ToNodeTaskAsync(TesTask task, NodeTaskConver .WithContainerImage(executor.Image) .WithStorageEventSink(storageAccessProvider.GetInternalTesBlobUrlWithoutSasToken(blobPath: string.Empty)) .WithLogPublisher(storageAccessProvider.GetInternalTesTaskBlobUrlWithoutSasToken(task, blobPath: string.Empty)) + .WithDrsHubUrl(nodeTaskConversionOptions.DrsHubApiHost) .WithMetricsFile(MetricsFileName); if (terraOptions is not null && !string.IsNullOrEmpty(terraOptions.WsmApiHost)) @@ -462,6 +463,7 @@ private static string AppendParentDirectoryIfSet(string inputPath, string pathPa /// /// /// + /// public record NodeTaskConversionOptions(IList AdditionalInputs = default, string DefaultStorageAccountName = default, - string GlobalManagedIdentity = default); + string GlobalManagedIdentity = default, string DrsHubApiHost = default); } diff --git a/src/TesApi.Web/Startup.cs b/src/TesApi.Web/Startup.cs index 79d6022ee..97b2d1bde 100644 --- a/src/TesApi.Web/Startup.cs +++ b/src/TesApi.Web/Startup.cs @@ -81,7 +81,7 @@ public void ConfigureServices(IServiceCollection services) .Configure(configuration.GetSection(BatchNodesOptions.SectionName)) .Configure(configuration.GetSection(BatchSchedulingOptions.SectionName)) .Configure(configuration.GetSection(StorageOptions.SectionName)) - .Configure(configuration.GetSection(MarthaOptions.SectionName)) + .Configure(configuration.GetSection(DrsHubOptions.SectionName)) .Configure(configuration.GetSection(DeploymentOptions.SectionName)) .AddMemoryCache(o => o.ExpirationScanFrequency = TimeSpan.FromHours(12)) diff --git a/src/TesApi.Web/appsettings.json b/src/TesApi.Web/appsettings.json index f115e5aeb..3609b54b0 100644 --- a/src/TesApi.Web/appsettings.json +++ b/src/TesApi.Web/appsettings.json @@ -43,7 +43,7 @@ "DatabaseUserLogin": "", "DatabaseUserPassword": "" }, - "Martha": { + "DrsHub": { "CromwellDrsLocalizer": "broadinstitute/cromwell-drs-localizer:develop" }, "RetryPolicy": { From 9f7d8dbba4bef37bb61e494b5ef6a67692ec94fc Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:03:50 -0400 Subject: [PATCH 06/13] drs resolution --- .../DrsHubApiClientTests.cs | 5 +++-- src/Tes.ApiClients/DrsHubApiClient.cs | 14 +++++++------- src/Tes.ApiClients/HttpApiClient.cs | 7 ++++--- .../Models/Terra/DrsResolveApiRequest.cs | 2 +- src/Tes.ApiClients/TerraApiClient.cs | 13 +++++++------ .../Storage/DrsTransformationStrategy.cs | 2 +- src/Tes.Runner/Storage/TerraConfigConstants.cs | 5 ++++- .../Runner/TaskToNodeTaskConverterTests.cs | 17 +++++++++++++++-- src/TesApi.Web/BatchScheduler.cs | 15 +++++++-------- 9 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs b/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs index 38310dade..42881ff1e 100644 --- a/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs +++ b/src/Tes.ApiClients.Tests/DrsHubApiClientTests.cs @@ -55,7 +55,8 @@ public async Task GetDrsResolveRequestContent_ValidDrsUri_ReturnsValidRequestCon } [TestMethod] - public async Task GetDrsResolveApiResponse_ResponseWithAccessUrl_CanDeserializeJSon() { + public async Task GetDrsResolveApiResponse_ResponseWithAccessUrl_CanDeserializeJSon() + { var httpResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK); httpResponse.Content = new StringContent(ExpectedRsResolveResponseJson); @@ -63,7 +64,7 @@ public async Task GetDrsResolveApiResponse_ResponseWithAccessUrl_CanDeserializeJ Assert.IsNotNull(drsResolveResponse); Assert.IsNotNull(drsResolveResponse.AccessUrl); - Assert.AreEqual("https://storage.foo/bar",drsResolveResponse.AccessUrl.Url); + Assert.AreEqual("https://storage.foo/bar", drsResolveResponse.AccessUrl.Url); } private const string ExpectedRsResolveResponseJson = @"{ diff --git a/src/Tes.ApiClients/DrsHubApiClient.cs b/src/Tes.ApiClients/DrsHubApiClient.cs index 02ec2cf52..e292e2eae 100644 --- a/src/Tes.ApiClients/DrsHubApiClient.cs +++ b/src/Tes.ApiClients/DrsHubApiClient.cs @@ -11,7 +11,7 @@ namespace Tes.ApiClients { public class DrsHubApiClient : TerraApiClient { - private const string DrsHubApiSegments = "/api/v4/drs"; + private const string DrsHubApiSegments = "/api/v4/drs"; private static readonly IMemoryCache SharedMemoryCache = new MemoryCache(new MemoryCacheOptions()); /// @@ -36,8 +36,8 @@ public DrsHubApiClient(string apiUrl, TokenCredential tokenCredential, CachingRe } public static DrsHubApiClient CreateDrsHubApiClient(string apiUrl, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) - { - return CreateTerraApiClient(apiUrl, SharedMemoryCache, tokenCredential,azureCloudIdentityConfig); + { + return CreateTerraApiClient(apiUrl, SharedMemoryCache, tokenCredential, azureCloudIdentityConfig); } public virtual async Task ResolveDrsUriAsync(Uri drsUri, CancellationToken cancellationToken = default) @@ -75,14 +75,14 @@ public static async Task GetDrsResolveApiResponseAsync(Ht } public HttpContent GetDrsResolveRequestContent(Uri drsUri) - { + { ArgumentNullException.ThrowIfNull(drsUri); var drsResolveApiRequestBody = new DrsResolveRequestContent { - Url = drsUri.AbsoluteUri, - CloudPlatform = CloudPlatform.Azure, - Fields = new List { "accessUrl" } + Url = drsUri.AbsoluteUri, + CloudPlatform = CloudPlatform.Azure, + Fields = new List { "accessUrl" } }; return CreateJsonStringContent(drsResolveApiRequestBody); diff --git a/src/Tes.ApiClients/HttpApiClient.cs b/src/Tes.ApiClients/HttpApiClient.cs index bd5664522..c3eb805a3 100644 --- a/src/Tes.ApiClients/HttpApiClient.cs +++ b/src/Tes.ApiClients/HttpApiClient.cs @@ -53,7 +53,7 @@ protected HttpApiClient(CachingRetryPolicyBuilder cachingRetryPolicyBuilder, ILo .AddCaching() .AsyncBuild(); } - + /// /// Constructor of base HttpApiClient /// @@ -69,7 +69,7 @@ protected HttpApiClient(TokenCredential tokenCredential, string tokenScope, this.tokenCredential = tokenCredential; this.tokenScope = tokenScope; - } + } /// /// Protected parameter-less constructor of HttpApiClient @@ -370,7 +370,8 @@ protected static async Task ReadResponseBodyAsync(HttpResponseMessage re /// /// Creates a string content from an object /// - protected static HttpContent CreateJsonStringContent(T contentObject) { + protected static HttpContent CreateJsonStringContent(T contentObject) + { var stringContent = new StringContent(JsonSerializer.Serialize(contentObject, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }), Encoding.UTF8, "application/json"); diff --git a/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs b/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs index b08b6690c..19905befd 100644 --- a/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs +++ b/src/Tes.ApiClients/Models/Terra/DrsResolveApiRequest.cs @@ -8,7 +8,7 @@ namespace Tes.ApiClients.Models.Terra { - + public class DrsResolveRequestContent { [JsonPropertyName("url")] diff --git a/src/Tes.ApiClients/TerraApiClient.cs b/src/Tes.ApiClients/TerraApiClient.cs index 755084b1b..bdcc392f8 100644 --- a/src/Tes.ApiClients/TerraApiClient.cs +++ b/src/Tes.ApiClients/TerraApiClient.cs @@ -41,15 +41,16 @@ protected TerraApiClient(string apiUrl, TokenCredential tokenCredential, Caching ApiUrl = apiUrl; } - protected static T CreateTerraApiClient(string apiUrl, IMemoryCache sharedMemoryCache, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) where T : TerraApiClient { + protected static T CreateTerraApiClient(string apiUrl, IMemoryCache sharedMemoryCache, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) where T : TerraApiClient + { var retryPolicyOptions = new RetryPolicyOptions(); var cacheRetryHandler = new CachingRetryPolicyBuilder(sharedMemoryCache, Microsoft.Extensions.Options.Options.Create(retryPolicyOptions)); - return (T)Activator.CreateInstance(typeof(T), - apiUrl, - tokenCredential, - cacheRetryHandler, - azureCloudIdentityConfig, + return (T)Activator.CreateInstance(typeof(T), + apiUrl, + tokenCredential, + cacheRetryHandler, + azureCloudIdentityConfig, ApiClientsLoggerFactory.Create()); } } diff --git a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs index bfb88fa60..0364b0963 100644 --- a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/DrsTransformationStrategy.cs @@ -21,7 +21,7 @@ public class DrsTransformationStrategy : IUrlTransformationStrategy private readonly DrsHubApiClient drsHubApiClient; private const string DrsScheme = "drs"; - public DrsTransformationStrategy(DrsHubApiClient drsHubApiClient) + public DrsTransformationStrategy(DrsHubApiClient drsHubApiClient) { ArgumentNullException.ThrowIfNull(drsHubApiClient); diff --git a/src/Tes.Runner/Storage/TerraConfigConstants.cs b/src/Tes.Runner/Storage/TerraConfigConstants.cs index 941cbb79b..855ca3a1e 100644 --- a/src/Tes.Runner/Storage/TerraConfigConstants.cs +++ b/src/Tes.Runner/Storage/TerraConfigConstants.cs @@ -1,7 +1,10 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + namespace Tes.Runner.Storage; public static class TerraConfigConstants { public const int TokenExpirationInSeconds = 3600 * 24; //1 day, max time allowed by Terra. public const int CacheExpirationInSeconds = TokenExpirationInSeconds - 1800; // 30 minutes less than token expiration -} \ No newline at end of file +} diff --git a/src/TesApi.Tests/Runner/TaskToNodeTaskConverterTests.cs b/src/TesApi.Tests/Runner/TaskToNodeTaskConverterTests.cs index 69429ccd5..ea934693f 100644 --- a/src/TesApi.Tests/Runner/TaskToNodeTaskConverterTests.cs +++ b/src/TesApi.Tests/Runner/TaskToNodeTaskConverterTests.cs @@ -41,6 +41,7 @@ public class TaskToNodeTaskConverterTests static readonly Uri InternalBlobUrl = new("http://foo.bar/tes-internal"); static readonly Uri InternalBlobUrlWithSas = new($"{InternalBlobUrl}?{SasToken}"); const string GlobalManagedIdentity = $@"/subscriptions/{SubscriptionId}/resourceGroups/{ResourceGroup}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/globalId"; + private const string DrsHubApiHost = "https://drshub.foo.bar"; const string ExternalStorageAccountName = "external"; @@ -142,7 +143,7 @@ public async Task ToNodeTaskAsync_DuplicateInputsAreRemoved() Assert.IsNotNull(nodeTask); - Assert.IsTrue(nodeTask.Inputs.Count == tesTask.Inputs.Count - 1); + Assert.IsTrue(nodeTask.Inputs!.Count == tesTask.Inputs.Count - 1); } @@ -332,6 +333,17 @@ public async Task ToNodeTaskAsync_InputUrlIsAExternalLocalPath_UrlIsConvertedUse Assert.AreEqual("file", url.BlobName); } + [TestMethod] + public async Task ToNodeTaskAsync_DrsHubApiHostIsProvided_DrsHubApiHostIsSetInNodeTask() + { + var options = OptionsWithoutAdditionalInputs(); + + var nodeTask = await taskToNodeTaskConverter.ToNodeTaskAsync(tesTask, options, CancellationToken.None); + + Assert.IsNotNull(nodeTask); + Assert.AreEqual(DrsHubApiHost, nodeTask.RuntimeOptions.Terra!.DrsHubApiHost); + } + private static NodeTaskConversionOptions OptionsWithAdditionalInputs() { var inputs = new[] @@ -360,7 +372,8 @@ private static NodeTaskConversionOptions OptionsWithoutAdditionalInputs() { return new NodeTaskConversionOptions(AdditionalInputs: null, DefaultStorageAccountName: DefaultStorageAccountName, - GlobalManagedIdentity: GlobalManagedIdentity); + GlobalManagedIdentity: GlobalManagedIdentity, + DrsHubApiHost: DrsHubApiHost); } private static TesTask GetTestTesTask() diff --git a/src/TesApi.Web/BatchScheduler.cs b/src/TesApi.Web/BatchScheduler.cs index d08347e70..fe42368f1 100644 --- a/src/TesApi.Web/BatchScheduler.cs +++ b/src/TesApi.Web/BatchScheduler.cs @@ -16,7 +16,6 @@ using Newtonsoft.Json; using Tes.Extensions; using Tes.Models; -using Tes.TaskSubmitters; using TesApi.Web.Extensions; using TesApi.Web.Management; using TesApi.Web.Management.Models.Quotas; @@ -56,7 +55,7 @@ public partial class BatchScheduler : IBatchScheduler private const string StartTaskScriptFilename = "start-task.sh"; private const string NodeTaskRunnerFilename = "tes-runner"; private const string NodeTaskRunnerMD5HashFilename = NodeTaskRunnerFilename + ".md5"; - private readonly string cromwellDrsLocalizerImageName; + // private readonly string cromwellDrsLocalizerImageName; private readonly ILogger logger; private readonly IAzureProxy azureProxy; private readonly IStorageAccessProvider storageAccessProvider; @@ -77,6 +76,7 @@ public partial class BatchScheduler : IBatchScheduler private readonly IAllowedVmSizesService allowedVmSizesService; private readonly TaskExecutionScriptingManager taskExecutionScriptingManager; private readonly string runnerMD5; + private readonly string drsHubApiHost; private HashSet onlyLogBatchTaskStateOnce = []; @@ -86,7 +86,7 @@ public partial class BatchScheduler : IBatchScheduler /// Logger /// Configuration of /// Configuration of - /// Configuration of + /// Configuration of /// Configuration of /// Configuration of /// Configuration of @@ -101,7 +101,7 @@ public BatchScheduler( ILogger logger, IOptions batchGen1Options, IOptions batchGen2Options, - IOptions marthaOptions, + IOptions drsHubOptions, IOptions storageOptions, IOptions batchNodesOptions, IOptions batchSchedulingOptions, @@ -129,8 +129,6 @@ public BatchScheduler( this.usePreemptibleVmsOnly = batchSchedulingOptions.Value.UsePreemptibleVmsOnly; this.batchNodesSubnetId = batchNodesOptions.Value.SubnetId; - this.cromwellDrsLocalizerImageName = marthaOptions.Value.CromwellDrsLocalizer; - if (string.IsNullOrWhiteSpace(this.cromwellDrsLocalizerImageName)) { this.cromwellDrsLocalizerImageName = Options.MarthaOptions.DefaultCromwellDrsLocalizer; } this.disableBatchNodesPublicIpAddress = batchNodesOptions.Value.DisablePublicIpAddress; this.poolLifetime = TimeSpan.FromDays(batchSchedulingOptions.Value.PoolRotationForcedDays == 0 ? Options.BatchSchedulingOptions.DefaultPoolRotationForcedDays : batchSchedulingOptions.Value.PoolRotationForcedDays); this.defaultStorageAccountName = storageOptions.Value.DefaultAccountName; @@ -161,7 +159,7 @@ public BatchScheduler( BatchImageVersion = batchGen1Options.Value.Version, BatchNodeAgentSkuId = batchGen1Options.Value.NodeAgentSkuId }; - + drsHubApiHost = drsHubOptions.Value?.Url; logger.LogInformation("usePreemptibleVmsOnly: {UsePreemptibleVmsOnly}", usePreemptibleVmsOnly); static bool tesTaskIsQueuedInitializingOrRunning(TesTask tesTask) => tesTask.State == TesState.QUEUEDEnum || tesTask.State == TesState.INITIALIZINGEnum || tesTask.State == TesState.RUNNINGEnum; @@ -916,7 +914,8 @@ private async Task GetNodeTaskConversionOptionsAsync( var nodeTaskCreationOptions = new NodeTaskConversionOptions( DefaultStorageAccountName: defaultStorageAccountName, AdditionalInputs: await GetAdditionalCromwellInputsAsync(task, cancellationToken), - GlobalManagedIdentity: globalManagedIdentity + GlobalManagedIdentity: globalManagedIdentity, + DrsHubApiHost: drsHubApiHost ); return nodeTaskCreationOptions; } From 896f24a9062eeb779fb1ff9629b24acab092794b Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Wed, 27 Mar 2024 15:02:57 -0400 Subject: [PATCH 07/13] minor naming and config changes --- src/Tes.ApiClients/HttpApiClient.cs | 12 ++++++------ src/Tes.ApiClients/PriceApiClient.cs | 4 ++-- src/TesApi.Web/appsettings.json | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Tes.ApiClients/HttpApiClient.cs b/src/Tes.ApiClients/HttpApiClient.cs index c3eb805a3..811e1b7d3 100644 --- a/src/Tes.ApiClients/HttpApiClient.cs +++ b/src/Tes.ApiClients/HttpApiClient.cs @@ -28,7 +28,7 @@ public abstract class HttpApiClient private readonly SemaphoreSlim semaphore = new(1, 1); private AccessToken accessToken; - protected readonly CachingRetryHandler.CachingAsyncRetryHandlerPolicy cachingRetryPolicy; + protected readonly CachingRetryHandler.CachingAsyncRetryHandlerPolicy cachingRetryHandler; /// /// Inner http client. @@ -47,7 +47,7 @@ protected HttpApiClient(CachingRetryPolicyBuilder cachingRetryPolicyBuilder, ILo this.Logger = logger; - cachingRetryPolicy = cachingRetryPolicyBuilder + cachingRetryHandler = cachingRetryPolicyBuilder .DefaultRetryHttpResponseMessagePolicyBuilder() .SetOnRetryBehavior(onRetry: LogRetryErrorOnRetryHttpResponseMessageHandler()) .AddCaching() @@ -105,7 +105,7 @@ private RetryHandler.OnRetryHandler LogRetryErrorOnRetryHtt /// protected async Task HttpSendRequestWithRetryPolicyAsync( Func httpRequestFactory, CancellationToken cancellationToken, bool setAuthorizationHeader = false) - => await cachingRetryPolicy.ExecuteWithRetryAsync(async ct => + => await cachingRetryHandler.ExecuteWithRetryAsync(async ct => { var request = httpRequestFactory(); @@ -150,7 +150,7 @@ protected async Task HttpGetRequestWithCachingAndRetryPolicyAsync + return (await cachingRetryHandler.ExecuteWithRetryConversionAndCachingAsync(cacheKey, async ct => { //request must be recreated in every retry. var httpRequest = await CreateGetHttpRequest(requestUrl, setAuthorizationHeader, ct); @@ -170,7 +170,7 @@ protected async Task HttpGetRequestWithCachingAndRetryPolicyAsync protected async Task HttpGetRequestWithRetryPolicyAsync(Uri requestUrl, CancellationToken cancellationToken, bool setAuthorizationHeader = false) - => await cachingRetryPolicy.ExecuteWithRetryAndConversionAsync(async ct => + => await cachingRetryHandler.ExecuteWithRetryAndConversionAsync(async ct => { //request must be recreated in every retry. var httpRequest = await CreateGetHttpRequest(requestUrl, setAuthorizationHeader, ct); @@ -248,7 +248,7 @@ private async Task CreateGetHttpRequest(Uri requestUrl, bool protected async Task HttpGetRequestWithRetryPolicyAsync( Func httpRequestFactory, CancellationToken cancellationToken, bool setAuthorizationHeader = false) { - return await cachingRetryPolicy.ExecuteWithRetryAndConversionAsync(async ct => + return await cachingRetryHandler.ExecuteWithRetryAndConversionAsync(async ct => { var request = httpRequestFactory(); diff --git a/src/Tes.ApiClients/PriceApiClient.cs b/src/Tes.ApiClients/PriceApiClient.cs index 06289a16b..64d49d4da 100644 --- a/src/Tes.ApiClients/PriceApiClient.cs +++ b/src/Tes.ApiClients/PriceApiClient.cs @@ -17,9 +17,9 @@ public class PriceApiClient : HttpApiClient /// /// Constructor of the Price API Client. /// - /// + /// /// - public PriceApiClient(CachingRetryPolicyBuilder cachingRetryPolicyHandler, ILogger logger) : base(cachingRetryPolicyHandler, logger) + public PriceApiClient(CachingRetryPolicyBuilder cachingRetryPolicyBuilder, ILogger logger) : base(cachingRetryPolicyBuilder, logger) { } diff --git a/src/TesApi.Web/appsettings.json b/src/TesApi.Web/appsettings.json index 3609b54b0..532d5942f 100644 --- a/src/TesApi.Web/appsettings.json +++ b/src/TesApi.Web/appsettings.json @@ -44,7 +44,7 @@ "DatabaseUserPassword": "" }, "DrsHub": { - "CromwellDrsLocalizer": "broadinstitute/cromwell-drs-localizer:develop" + "Url": "" }, "RetryPolicy": { "MaxRetryCount": 5, From 4d076745486b675f9d0a2a8a394581ca45a97878 Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Wed, 27 Mar 2024 15:10:15 -0400 Subject: [PATCH 08/13] clean up --- ...s => DrsUriTransformationStrategyTests.cs} | 28 +++++-------------- ...egy.cs => DrsUriTransformationStrategy.cs} | 8 +++--- .../UrlTransformationStrategyFactory.cs | 4 +-- 3 files changed, 13 insertions(+), 27 deletions(-) rename src/Tes.Runner.Test/Storage/{DrsTransformationStrategyTests.cs => DrsUriTransformationStrategyTests.cs} (55%) rename src/Tes.Runner/Storage/{DrsTransformationStrategy.cs => DrsUriTransformationStrategy.cs} (82%) diff --git a/src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs b/src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs similarity index 55% rename from src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs rename to src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs index 6200974ae..5c28e54a4 100644 --- a/src/Tes.Runner.Test/Storage/DrsTransformationStrategyTests.cs +++ b/src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs @@ -1,39 +1,25 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using Azure.Core; -using CommonUtilities; using Moq; using Tes.ApiClients; using Tes.ApiClients.Models.Terra; -using Tes.Runner.Models; using Tes.Runner.Storage; -namespace Tes.Runner.Tests.Storage +namespace Tes.Runner.Test.Storage { [TestClass] [TestCategory("Unit")] - public class DrsTransformationStrategyTests + public class DrsUriTransformationStrategyTests { - private DrsTransformationStrategy drsTransformationStrategy = null!; - private Mock tokenCredentialsMock = null!; + private DrsUriTransformationStrategy drsUriTransformationStrategy = null!; private Mock drsHubApiClientMock = null!; - const string DrsHubApiHost = "https://drs-hub-api-host"; [TestInitialize] public void SetUp() - { - tokenCredentialsMock = new Mock(); - var azureEnvConfig = new AzureEnvironmentConfig() - { - TokenScope = "https://management.azure.com/.default", - }; - var terraRuntimeOptions = new TerraRuntimeOptions() - { - DrsHubApiHost = DrsHubApiHost, - }; + { drsHubApiClientMock = new Mock(); - drsTransformationStrategy = new DrsTransformationStrategy(drsHubApiClientMock.Object); + drsUriTransformationStrategy = new DrsUriTransformationStrategy(drsHubApiClientMock.Object); } [TestMethod] @@ -41,7 +27,7 @@ public async Task TransformUriWithStrategyAsync_NonDrsUri_RerturnUriWithoutTrans { var uri = "https://non-drs-uri"; - var transformedUri = await drsTransformationStrategy.TransformUrlWithStrategyAsync(uri); + var transformedUri = await drsUriTransformationStrategy.TransformUrlWithStrategyAsync(uri); Assert.AreEqual(new Uri(uri), transformedUri); } @@ -60,7 +46,7 @@ public async Task TransformUriWithStrategyAsync_DrsUri_CallsResolveApiAndReturns }; drsHubApiClientMock.Setup(x => x.ResolveDrsUriAsync(new Uri(drsUri), CancellationToken.None)).ReturnsAsync(response); - var transformedUri = await drsTransformationStrategy.TransformUrlWithStrategyAsync(drsUri); + var transformedUri = await drsUriTransformationStrategy.TransformUrlWithStrategyAsync(drsUri); Assert.AreEqual(new Uri(resolvedUrl), transformedUri); } diff --git a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs b/src/Tes.Runner/Storage/DrsUriTransformationStrategy.cs similarity index 82% rename from src/Tes.Runner/Storage/DrsTransformationStrategy.cs rename to src/Tes.Runner/Storage/DrsUriTransformationStrategy.cs index 0364b0963..2ead32f8a 100644 --- a/src/Tes.Runner/Storage/DrsTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/DrsUriTransformationStrategy.cs @@ -15,19 +15,19 @@ namespace Tes.Runner.Storage /// Terra DRS transformation strategy. Uses the configured DRSHub service to resolve a DRS URI. /// If the URI is not a valid DRS URI, the URI is not transformed. /// - public class DrsTransformationStrategy : IUrlTransformationStrategy + public class DrsUriTransformationStrategy : IUrlTransformationStrategy { - private readonly ILogger logger = PipelineLoggerFactory.Create(); + private readonly ILogger logger = PipelineLoggerFactory.Create(); private readonly DrsHubApiClient drsHubApiClient; private const string DrsScheme = "drs"; - public DrsTransformationStrategy(DrsHubApiClient drsHubApiClient) + public DrsUriTransformationStrategy(DrsHubApiClient drsHubApiClient) { ArgumentNullException.ThrowIfNull(drsHubApiClient); this.drsHubApiClient = drsHubApiClient; } - public DrsTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) + public DrsUriTransformationStrategy(TerraRuntimeOptions terraRuntimeOptions, TokenCredential tokenCredential, AzureEnvironmentConfig azureCloudIdentityConfig) { ArgumentNullException.ThrowIfNull(terraRuntimeOptions); ArgumentException.ThrowIfNullOrEmpty(terraRuntimeOptions.DrsHubApiHost, nameof(terraRuntimeOptions.DrsHubApiHost)); diff --git a/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs b/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs index cdc4c3062..ff8b914ca 100644 --- a/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs +++ b/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs @@ -27,14 +27,14 @@ public static IUrlTransformationStrategy CreateStrategy(TransformationStrategy t case TransformationStrategy.CombinedTerra: return new CombinedTransformationStrategy(new List { - new DrsTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), + new DrsUriTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), new CloudProviderSchemeConverter(), new TerraUrlTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), }); case TransformationStrategy.CombinedAzureResourceManager: return new CombinedTransformationStrategy(new List { - new DrsTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), + new DrsUriTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), new CloudProviderSchemeConverter(), new ArmUrlTransformationStrategy(u => new BlobServiceClient(u, TokenCredentialsManager.GetTokenCredential(runtimeOptions)), runtimeOptions) }); From 70b39b4ea8537b723d81b402e32189ce30a68b51 Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Wed, 27 Mar 2024 15:20:45 -0400 Subject: [PATCH 09/13] formatting fixes --- .../Storage/DrsUriTransformationStrategyTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs b/src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs index 5c28e54a4..a07a4ab99 100644 --- a/src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs +++ b/src/Tes.Runner.Test/Storage/DrsUriTransformationStrategyTests.cs @@ -17,7 +17,7 @@ public class DrsUriTransformationStrategyTests [TestInitialize] public void SetUp() - { + { drsHubApiClientMock = new Mock(); drsUriTransformationStrategy = new DrsUriTransformationStrategy(drsHubApiClientMock.Object); } From c96bb0f1fdce173426ad60e62daff9755257ce82 Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Fri, 29 Mar 2024 11:11:50 -0400 Subject: [PATCH 10/13] pr feedback --- src/TesApi.Web/BatchScheduler.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/TesApi.Web/BatchScheduler.cs b/src/TesApi.Web/BatchScheduler.cs index fe42368f1..5d7bb64ed 100644 --- a/src/TesApi.Web/BatchScheduler.cs +++ b/src/TesApi.Web/BatchScheduler.cs @@ -55,7 +55,6 @@ public partial class BatchScheduler : IBatchScheduler private const string StartTaskScriptFilename = "start-task.sh"; private const string NodeTaskRunnerFilename = "tes-runner"; private const string NodeTaskRunnerMD5HashFilename = NodeTaskRunnerFilename + ".md5"; - // private readonly string cromwellDrsLocalizerImageName; private readonly ILogger logger; private readonly IAzureProxy azureProxy; private readonly IStorageAccessProvider storageAccessProvider; From 74d94159052d4bdaee6ed45e034c035c5425fc7d Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Fri, 29 Mar 2024 11:22:08 -0400 Subject: [PATCH 11/13] rebase --- src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs b/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs index 863ee3ab6..2345feb71 100644 --- a/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs +++ b/src/Tes.ApiClients/Models/Terra/DrsResolveApiResponse.cs @@ -1,12 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Text.Json.Serialization; -using System.Threading.Tasks; namespace Tes.ApiClients.Models.Terra { @@ -20,10 +15,10 @@ public class DrsResolveApiResponse public long Size { get; set; } [JsonPropertyName("timeCreated")] - public DateTime TimeCreated { get; set; } + public DateTimeOffset TimeCreated { get; set; } [JsonPropertyName("timeUpdated")] - public DateTime TimeUpdated { get; set; } + public DateTimeOffset TimeUpdated { get; set; } [JsonPropertyName("bucket")] public string Bucket { get; set; } From 7191645e7276c62f6891642385f336bff87ff40b Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Fri, 29 Mar 2024 21:32:57 -0400 Subject: [PATCH 12/13] DrsHub deployer config --- src/TesApi.Web/Options/DrsHubOptions.cs | 4 ++-- src/deploy-tes-on-azure/KubernetesManager.cs | 14 +++++--------- .../scripts/env-04-settings.txt | 4 +--- .../scripts/helm/templates/tes-deployment.yaml | 8 ++------ .../scripts/helm/values-template.yaml | 4 +--- 5 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/TesApi.Web/Options/DrsHubOptions.cs b/src/TesApi.Web/Options/DrsHubOptions.cs index fed65c46d..0c8362f02 100644 --- a/src/TesApi.Web/Options/DrsHubOptions.cs +++ b/src/TesApi.Web/Options/DrsHubOptions.cs @@ -4,12 +4,12 @@ namespace TesApi.Web.Options { /// - /// Martha configuration options + /// DrsHub configuration options /// public class DrsHubOptions { /// - /// Martha configuration section + /// DrsHub configuration section /// public const string SectionName = "DrsHub"; diff --git a/src/deploy-tes-on-azure/KubernetesManager.cs b/src/deploy-tes-on-azure/KubernetesManager.cs index b11c8ab12..1eb466541 100644 --- a/src/deploy-tes-on-azure/KubernetesManager.cs +++ b/src/deploy-tes-on-azure/KubernetesManager.cs @@ -386,7 +386,7 @@ private static void UpdateValuesFromSettings(HelmValues values, Dictionary(); var batchImageGen2 = GetObjectFromConfig(values, "batchImageGen2") ?? new Dictionary(); var batchImageGen1 = GetObjectFromConfig(values, "batchImageGen1") ?? new Dictionary(); - var martha = GetObjectFromConfig(values, "martha") ?? new Dictionary(); + var drsHub = GetObjectFromConfig(values, "drsHub") ?? new Dictionary(); var deployment = GetObjectFromConfig(values, "deployment") ?? new Dictionary(); values.Config["azureCloudName"] = GetValueOrDefault(settings, "AzureCloudName"); @@ -408,9 +408,7 @@ private static void UpdateValuesFromSettings(HelmValues values, Dictionary ValuesToSettings(HelmValues values) var batchScheduling = GetObjectFromConfig(values, "batchScheduling") ?? new Dictionary(); var batchImageGen2 = GetObjectFromConfig(values, "batchImageGen2") ?? new Dictionary(); var batchImageGen1 = GetObjectFromConfig(values, "batchImageGen1") ?? new Dictionary(); - var martha = GetObjectFromConfig(values, "martha") ?? new Dictionary(); + var drsHub = GetObjectFromConfig(values, "drsHub") ?? new Dictionary(); var deployment = GetObjectFromConfig(values, "deployment") ?? new Dictionary(); return new() @@ -480,9 +478,7 @@ private static Dictionary ValuesToSettings(HelmValues values) ["Gen1BatchImageSku"] = GetValueOrDefault(batchImageGen1, "sku"), ["Gen1BatchImageVersion"] = GetValueOrDefault(batchImageGen1, "version"), ["Gen1BatchNodeAgentSkuId"] = GetValueOrDefault(batchImageGen1, "nodeAgentSkuId"), - ["MarthaUrl"] = GetValueOrDefault(martha, "url"), - ["MarthaKeyVaultName"] = GetValueOrDefault(martha, "keyVaultName"), - ["MarthaSecretName"] = GetValueOrDefault(martha, "secretName"), + ["DrsHubUrl"] = GetValueOrDefault(drsHub, "url"), ["BatchPrefix"] = GetValueOrDefault(batchScheduling, "prefix"), ["CrossSubscriptionAKSDeployment"] = GetValueOrDefault(values.Config, "crossSubscriptionAKSDeployment") as string, ["UsePostgreSqlSingleServer"] = GetValueOrDefault(values.Config, "usePostgreSqlSingleServer") as string, diff --git a/src/deploy-tes-on-azure/scripts/env-04-settings.txt b/src/deploy-tes-on-azure/scripts/env-04-settings.txt index 7c88237af..e00e35148 100644 --- a/src/deploy-tes-on-azure/scripts/env-04-settings.txt +++ b/src/deploy-tes-on-azure/scripts/env-04-settings.txt @@ -11,8 +11,6 @@ Gen1BatchImageVersion=latest Gen1BatchNodeAgentSkuId=batch.node.ubuntu 20.04 BatchPoolRotationForcedDays=7 BatchPrefix={DefaultName} -MarthaUrl= -MarthaKeyVaultName= -MarthaSecretName= +DrsHubUrl= GlobalStartTaskPath=/configuration/start-task.sh GlobalManagedIdentity= diff --git a/src/deploy-tes-on-azure/scripts/helm/templates/tes-deployment.yaml b/src/deploy-tes-on-azure/scripts/helm/templates/tes-deployment.yaml index 16bb8babb..fbf46209f 100644 --- a/src/deploy-tes-on-azure/scripts/helm/templates/tes-deployment.yaml +++ b/src/deploy-tes-on-azure/scripts/helm/templates/tes-deployment.yaml @@ -108,12 +108,8 @@ spec: value: {{ .Values.service.tesHostname }} - name: Deployment__LetsEncryptEmail value: {{ .Values.config.letsEncryptEmail }} - - name: Martha__Url - value: {{ .Values.config.martha.url }} - - name: Martha__KeyVaultName - value: {{ .Values.config.martha.keyVaultName }} - - name: Martha__SecretName - value: {{ .Values.config.martha.secretName }} + - name: DrsHub__Url + value: {{ .Values.config.drsHub.url }} - name: Storage__ExternalStorageContainers {{- $containers := list -}} {{- range .Values.externalSasContainers }} diff --git a/src/deploy-tes-on-azure/scripts/helm/values-template.yaml b/src/deploy-tes-on-azure/scripts/helm/values-template.yaml index 217cd849a..e835f2d66 100644 --- a/src/deploy-tes-on-azure/scripts/helm/values-template.yaml +++ b/src/deploy-tes-on-azure/scripts/helm/values-template.yaml @@ -38,10 +38,8 @@ config: sku: RUNTIME_PARAMETER version: RUNTIME_PARAMETER nodeAgentSkuId: RUNTIME_PARAMETER - martha: + drsHub: url: RUNTIME_PARAMETER - keyVaultName: RUNTIME_PARAMETER - secretName: RUNTIME_PARAMETER letsEncryptEmail: RUNTIME_PARAMETER tesDatabase: From 3ff508f4c5fa9b3c6b61efce9b3a76b09a9c8b65 Mon Sep 17 00:00:00 2001 From: giventocode <3589801+giventocode@users.noreply.github.com> Date: Mon, 1 Apr 2024 11:55:21 -0400 Subject: [PATCH 13/13] tests and fixes --- .../CombinedTransformationStrategyTests.cs | 28 +++++ .../UrlTransformationStrategyFactoryTests.cs | 105 ++++++++++++++++++ .../Storage/CombinedTransformationStrategy.cs | 12 ++ .../UrlTransformationStrategyFactory.cs | 49 ++++++-- src/Tes.RunnerCLI/Tes.RunnerCLI.csproj | 1 + 5 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 src/Tes.Runner.Test/Storage/UrlTransformationStrategyFactoryTests.cs diff --git a/src/Tes.Runner.Test/Storage/CombinedTransformationStrategyTests.cs b/src/Tes.Runner.Test/Storage/CombinedTransformationStrategyTests.cs index dd0874c3b..c781feae6 100644 --- a/src/Tes.Runner.Test/Storage/CombinedTransformationStrategyTests.cs +++ b/src/Tes.Runner.Test/Storage/CombinedTransformationStrategyTests.cs @@ -48,5 +48,33 @@ public async Task CombinedTransformationStrategy_TwoStrategiesProvided_BothAreAp Assert.AreEqual(secondTransformation, result.ToString()); } + + [TestMethod] + public async Task CombinedTransformationStrategy_InsertsStrategyFirst_InsertedStrategyIsApplied() + { + var sourceUrl = "https://source.foo/"; + var firstTransformation = "https://first.foo/"; + var secondTransformation = "https://second.foo/"; + var thirdTransformation = "https://third.foo/"; + + mockStrategy1.Setup(s => s.TransformUrlWithStrategyAsync(firstTransformation, It.IsAny())) + .ReturnsAsync(() => new Uri(secondTransformation)); + mockStrategy2.Setup(s => s.TransformUrlWithStrategyAsync(secondTransformation, It.IsAny())) + .ReturnsAsync(() => new Uri(thirdTransformation)); + + var mockStrategy3 = new Mock(); + mockStrategy3.Setup(s => s.TransformUrlWithStrategyAsync(sourceUrl, It.IsAny())) + .ReturnsAsync(() => new Uri(firstTransformation)); + + strategy.InsertStrategy(0, mockStrategy3.Object); + + var result = await strategy.TransformUrlWithStrategyAsync(sourceUrl, new BlobSasPermissions()); + + mockStrategy3.Verify(s => s.TransformUrlWithStrategyAsync(sourceUrl, It.IsAny()), Times.Once); + mockStrategy1.Verify(s => s.TransformUrlWithStrategyAsync(firstTransformation, It.IsAny()), Times.Once); + mockStrategy2.Verify(s => s.TransformUrlWithStrategyAsync(secondTransformation, It.IsAny()), Times.Once); + + Assert.AreEqual(thirdTransformation, result.ToString()); + } } } diff --git a/src/Tes.Runner.Test/Storage/UrlTransformationStrategyFactoryTests.cs b/src/Tes.Runner.Test/Storage/UrlTransformationStrategyFactoryTests.cs new file mode 100644 index 000000000..18cf0e43c --- /dev/null +++ b/src/Tes.Runner.Test/Storage/UrlTransformationStrategyFactoryTests.cs @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Core; +using CommonUtilities; +using Moq; +using Tes.Runner.Models; +using Tes.Runner.Storage; + +namespace Tes.Runner.Test.Storage +{ + [TestClass] + [TestCategory("Unit")] + public class UrlTransformationStrategyFactoryTests + { + private RuntimeOptions runtimeOptions = null!; + private Mock tokenCredentialMock = null!; + const string? DrsHubHost = "https://drsHub.bio"; + + [TestInitialize] + public void SetUp() + { + runtimeOptions = CreateRuntimeOptions(); + + tokenCredentialMock = new Mock(); + } + + [TestMethod] + public void CreateCombinedArmTransformationStrategy_WithDrsHubSettings_ReturnsExpectedStrategies() + { + runtimeOptions.Terra = CreateTerraRuntimeOptions(DrsHubHost); + + var combinedStrategy = UrlTransformationStrategyFactory.CreateCombinedArmTransformationStrategy(runtimeOptions, tokenCredentialMock.Object); + + var internalStrategy = combinedStrategy.GetStrategies(); + + Assert.IsNotNull(internalStrategy); + Assert.IsInstanceOfType(internalStrategy.ElementAt(0)); + Assert.IsInstanceOfType(internalStrategy.ElementAt(1)); + Assert.IsInstanceOfType(internalStrategy.ElementAt(2)); + } + + [TestMethod] + public void CreateCombinedArmTransformationStrategy_WithOutDrsHubSettings_ReturnsExpectedStrategies() + { + var combinedStrategy = UrlTransformationStrategyFactory.CreateCombinedArmTransformationStrategy(runtimeOptions, tokenCredentialMock.Object); + + var internalStrategy = combinedStrategy.GetStrategies(); + + Assert.IsNotNull(internalStrategy); + Assert.IsInstanceOfType(internalStrategy.ElementAt(0)); + Assert.IsInstanceOfType(internalStrategy.ElementAt(1)); + } + + [TestMethod] + public void CreateCombinedTerraTransformationStrategy_WithOutDrsHubSettings_ReturnsExpectedStrategies() + { + runtimeOptions.Terra = CreateTerraRuntimeOptions(); + + var combinedStrategy = UrlTransformationStrategyFactory.CreateCombineTerraTransformationStrategy(runtimeOptions, tokenCredentialMock.Object); + + var internalStrategy = combinedStrategy.GetStrategies(); + + Assert.IsNotNull(internalStrategy); + Assert.IsInstanceOfType(internalStrategy.ElementAt(0)); + Assert.IsInstanceOfType(internalStrategy.ElementAt(1)); + } + + [TestMethod] + public void CreateCombinedTerraTransformationStrategy_WithDrsHubSettings_ReturnsExpectedStrategies() + { + runtimeOptions.Terra = CreateTerraRuntimeOptions(DrsHubHost); + + var combinedStrategy = UrlTransformationStrategyFactory.CreateCombineTerraTransformationStrategy(runtimeOptions, tokenCredentialMock.Object); + + var internalStrategy = combinedStrategy.GetStrategies(); + + Assert.IsNotNull(internalStrategy); + Assert.IsInstanceOfType(internalStrategy.ElementAt(0)); + Assert.IsInstanceOfType(internalStrategy.ElementAt(1)); + Assert.IsInstanceOfType(internalStrategy.ElementAt(2)); + } + + private static RuntimeOptions CreateRuntimeOptions() + { + return new RuntimeOptions + { + AzureEnvironmentConfig = new AzureEnvironmentConfig() + { + StorageUrlSuffix = @"core.windows.net", + TokenScope = ".default" + } + }; + } + private static TerraRuntimeOptions CreateTerraRuntimeOptions(string? drsHubHost = default) + { + return new TerraRuntimeOptions + { + DrsHubApiHost = drsHubHost, + WsmApiHost = "https://wsmhost.bio", + LandingZoneApiHost = "https://lzhost.bio", + }; + } + } +} diff --git a/src/Tes.Runner/Storage/CombinedTransformationStrategy.cs b/src/Tes.Runner/Storage/CombinedTransformationStrategy.cs index aa554a0c9..5801ed6fd 100644 --- a/src/Tes.Runner/Storage/CombinedTransformationStrategy.cs +++ b/src/Tes.Runner/Storage/CombinedTransformationStrategy.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.ObjectModel; using Azure.Storage.Sas; namespace Tes.Runner.Storage @@ -20,6 +21,12 @@ public CombinedTransformationStrategy(List strategie this.strategies = strategies; } + public void InsertStrategy(int index, IUrlTransformationStrategy strategy) + { + ArgumentNullException.ThrowIfNull(strategy); + strategies.Insert(index, strategy); + } + public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPermissions blobSasPermissions) { ArgumentException.ThrowIfNullOrEmpty(sourceUrl, nameof(sourceUrl)); @@ -33,5 +40,10 @@ public async Task TransformUrlWithStrategyAsync(string sourceUrl, BlobSasPe return result; } + + public ReadOnlyCollection GetStrategies() + { + return strategies.AsReadOnly(); + } } } diff --git a/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs b/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs index ff8b914ca..4ad0d8b47 100644 --- a/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs +++ b/src/Tes.Runner/Storage/UrlTransformationStrategyFactory.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Azure.Core; using Azure.Storage.Blobs; using Azure.Storage.Sas; using Tes.Runner.Authentication; @@ -25,24 +26,48 @@ public static IUrlTransformationStrategy CreateStrategy(TransformationStrategy t case TransformationStrategy.TerraWsm: return new TerraUrlTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!); case TransformationStrategy.CombinedTerra: - return new CombinedTransformationStrategy(new List - { - new DrsUriTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), - new CloudProviderSchemeConverter(), - new TerraUrlTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), - }); + return CreateCombineTerraTransformationStrategy(runtimeOptions, TokenCredentialsManager.GetTokenCredential(runtimeOptions)); case TransformationStrategy.CombinedAzureResourceManager: - return new CombinedTransformationStrategy(new List - { - new DrsUriTransformationStrategy(runtimeOptions.Terra!, TokenCredentialsManager.GetTokenCredential(runtimeOptions), runtimeOptions.AzureEnvironmentConfig!), - new CloudProviderSchemeConverter(), - new ArmUrlTransformationStrategy(u => new BlobServiceClient(u, TokenCredentialsManager.GetTokenCredential(runtimeOptions)), runtimeOptions) - }); + return CreateCombinedArmTransformationStrategy(runtimeOptions, TokenCredentialsManager.GetTokenCredential(runtimeOptions)); } throw new NotImplementedException(); } + public static CombinedTransformationStrategy CreateCombinedArmTransformationStrategy(RuntimeOptions runtimeOptions, TokenCredential tokenCredential) + { + var strategy = new CombinedTransformationStrategy(new List + { + new CloudProviderSchemeConverter(), + new ArmUrlTransformationStrategy(u => new BlobServiceClient(u, tokenCredential), runtimeOptions) + }); + + InsertDrsStrategyIfTerraSettingsAreSet(strategy, runtimeOptions, tokenCredential); + + return strategy; + } + + private static void InsertDrsStrategyIfTerraSettingsAreSet(CombinedTransformationStrategy strategy, RuntimeOptions runtime, TokenCredential tokenCredential) + { + if (runtime.Terra != null && !String.IsNullOrWhiteSpace(runtime.Terra.DrsHubApiHost)) + { + strategy.InsertStrategy(0, new DrsUriTransformationStrategy(runtime.Terra, tokenCredential, runtime.AzureEnvironmentConfig!)); + } + } + + public static CombinedTransformationStrategy CreateCombineTerraTransformationStrategy(RuntimeOptions runtimeOptions, TokenCredential tokenCredential) + { + var strategy = new CombinedTransformationStrategy(new List + { + new CloudProviderSchemeConverter(), + new TerraUrlTransformationStrategy(runtimeOptions.Terra!, tokenCredential, runtimeOptions.AzureEnvironmentConfig!), + }); + + InsertDrsStrategyIfTerraSettingsAreSet(strategy, runtimeOptions, tokenCredential); + + return strategy; + } + public static async Task GetTransformedUrlAsync(RuntimeOptions runtimeOptions, StorageTargetLocation location, BlobSasPermissions permissions) { ArgumentNullException.ThrowIfNull(location); diff --git a/src/Tes.RunnerCLI/Tes.RunnerCLI.csproj b/src/Tes.RunnerCLI/Tes.RunnerCLI.csproj index c5dba3f16..471cd23a5 100644 --- a/src/Tes.RunnerCLI/Tes.RunnerCLI.csproj +++ b/src/Tes.RunnerCLI/Tes.RunnerCLI.csproj @@ -39,6 +39,7 @@ +