From 9d752db175841929658db7734f7829646bb0d43f Mon Sep 17 00:00:00 2001 From: Meera Ruxmohan Date: Tue, 11 Jun 2024 05:24:17 +0000 Subject: [PATCH] Merged PR 786216: Use grpcs instead of grpc scheme for machine location Use grpcs instead of grpc scheme upon location creation, as this is resulting in the encrypted gRPC port to be identified as -1 instead of 7092: ``` 2024-05-21 14:23:36,202 [14] DEBUG Creating Ephemeral cache. Type=[DatacenterWideEphemeral] RootPath=[F:\dbs\sh\cb_m\0521_141826\TempFileStore\097a2a56-be5a-40b9-82b2-8eea9ae54557] MaxCacheSizeMb=[52428] Location=[grpc://61fb6450c000004:7092/] Leader=[61fb6450c000004 -> grpc://61fb6450c000004:7092/] Id=[CacheId { Universe = default, Namespace = default }] ... 2024-05-21 14:23:36,640 [17] DEBUG 0d000adf-5f53-4c98-bbc7-42842594cb8e GrpcCoreServerHost.TryGetEncryptedCredentials: Found gRPC Encryption Certificate. 2024-05-21 14:23:36,642 [23] DEBUG 0d000adf-5f53-4c98-bbc7-42842594cb8e GrpcCoreServerHost.StartAsync: Server creating Encrypted Grpc channel on port -1 ``` Related work items: #2181670 --- .../Utilities/GrpcFileCopierConfiguration.cs | 2 +- .../NuCache/CheckpointManagerTests.cs | 4 +++- .../NuCache/MachineLocationTests.cs | 7 +++++-- .../Library/Service/Grpc/MachineLocation.cs | 15 ++++++++++++++- .../Utilities.Core/ExceptionUtilities.cs | 2 +- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/Public/Src/Cache/ContentStore/Distributed/Utilities/GrpcFileCopierConfiguration.cs b/Public/Src/Cache/ContentStore/Distributed/Utilities/GrpcFileCopierConfiguration.cs index 506d63d28..0bcf81631 100644 --- a/Public/Src/Cache/ContentStore/Distributed/Utilities/GrpcFileCopierConfiguration.cs +++ b/Public/Src/Cache/ContentStore/Distributed/Utilities/GrpcFileCopierConfiguration.cs @@ -54,7 +54,7 @@ namespace BuildXL.Cache.ContentStore.Distributed.Utilities public IReadOnlyDictionary JunctionsByDirectory { get; set; } /// - /// Indicates whether machine locations should use universal format (i.e. uri of form 'grpc://{machineName}:{port}/') which + /// Indicates whether machine locations should use universal format (i.e. uri of form 'grpcs://{machineName}:{port}/' or 'grpc://{machineName}:{port}/') which /// allows communication across machines of different platforms /// public bool UseUniversalLocations { get; set; } diff --git a/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/CheckpointManagerTests.cs b/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/CheckpointManagerTests.cs index b1d3e7f8b..f3a49ec36 100644 --- a/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/CheckpointManagerTests.cs +++ b/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/CheckpointManagerTests.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using BuildXL.Cache.ContentStore.Distributed.NuCache; using BuildXL.Cache.ContentStore.Distributed.NuCache.EventStreaming; using BuildXL.Cache.ContentStore.Distributed.Utilities; +using BuildXL.Cache.ContentStore.Grpc; using BuildXL.Cache.ContentStore.Hashing; using BuildXL.Cache.ContentStore.Interfaces.Tracing; using BuildXL.Cache.ContentStore.InterfacesTest.FileSystem; @@ -95,7 +96,8 @@ namespace BuildXL.Cache.ContentStore.Distributed.Test.ContentLocation.NuCache [Fact] public void CanJsonSerializeMachineLocation() { - TestJsonRoundtrip(MachineLocation.Parse("grpc://machine:12334/")); + TestJsonRoundtrip(MachineLocation.Parse($"grpc://machine:{GrpcConstants.DefaultGrpcPort}/")); + TestJsonRoundtrip(MachineLocation.Parse($"grpcs://machine:{GrpcConstants.DefaultEncryptedGrpcPort}/")); TestJsonRoundtrip(MachineLocation.Parse(@"\\DS4PNPF000066FA\D$\DBS\CACHE\CONTENTADDRESSABLESTORE\SHARED")); TestJsonRoundtrip(MachineLocation.Parse("node1:1234")); TestJsonRoundtrip(MachineLocation.Parse("node1")); diff --git a/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/MachineLocationTests.cs b/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/MachineLocationTests.cs index a3fdf68df..5f821acdd 100644 --- a/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/MachineLocationTests.cs +++ b/Public/Src/Cache/ContentStore/DistributedTest/ContentLocation/NuCache/MachineLocationTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using BuildXL.Cache.ContentStore.Grpc; using BuildXL.Cache.ContentStore.Interfaces.FileSystem; using BuildXL.Cache.ContentStore.Interfaces.Utils; using FluentAssertions; @@ -58,7 +59,8 @@ public class MachineLocationTests [Theory] [InlineData(null, null)] [InlineData(@"\\DS4PNPF000066FA\D$\DBS\CACHE\CONTENTADDRESSABLESTORE\SHARED", @"\\DS4PNPF000066FA\D$\dbs\CACHE\CONTENTADDRESSABLESTORE\SHARED")] - [InlineData("grpc://a.com:123", "grpc://A.CoM:123")] + [InlineData("grpcs://a.com:7090", "grpcs://A.CoM:7090")] + [InlineData("grpc://localhost:7089", "grpc://lOcAlHoST:7089")] public void EqualityTests(string lhs, string rhs) { var left = MachineLocation.Parse(lhs); @@ -81,7 +83,8 @@ public class MachineLocationTests } [Theory] - [InlineData("grpc://a.com:123", "a.com", 123)] + [InlineData("grpcs://a.com:7090", "a.com", 7090)] + [InlineData("grpc://localhost:7089", "localhost", 7089)] [InlineData(@"\\DS4PNPF000066FA\D$\DBS\CACHE\CONTENTADDRESSABLESTORE\SHARED", "DS4PNPF000066FA", 7089)] [InlineData(@"node1:1234", "node1", 1234)] [InlineData(@"node1", "node1", 7089)] diff --git a/Public/Src/Cache/ContentStore/Library/Service/Grpc/MachineLocation.cs b/Public/Src/Cache/ContentStore/Library/Service/Grpc/MachineLocation.cs index b0a0f6b20..13345180d 100644 --- a/Public/Src/Cache/ContentStore/Library/Service/Grpc/MachineLocation.cs +++ b/Public/Src/Cache/ContentStore/Library/Service/Grpc/MachineLocation.cs @@ -11,6 +11,7 @@ using System.Text.RegularExpressions; using BuildXL.Cache.ContentStore.Grpc; using BuildXL.Cache.ContentStore.Interfaces.FileSystem; using BuildXL.Cache.ContentStore.Interfaces.Utils; +using BuildXL.Cache.ContentStore.Service; namespace BuildXL.Cache.ContentStore.Distributed; @@ -148,6 +149,11 @@ public readonly record struct MachineLocation return Invalid; } + if (HasEncryptedPort(port)) + { + return new MachineLocation(new Uri($"grpcs://{machineName}:{port}/")); + } + return new MachineLocation(new Uri($"grpc://{machineName}:{port}/")); } @@ -180,6 +186,11 @@ public readonly record struct MachineLocation var match = _hostRegex.Match(uri); if (match.Success) { + if (int.TryParse(match.Groups["port"].Value, out int port) && HasEncryptedPort(port)) + { + url = new Uri($"grpcs://{uri}/"); + } + url = new Uri($"grpc://{uri}/"); } else @@ -214,7 +225,7 @@ public readonly record struct MachineLocation public static HostInfo Create(string host, int? port, bool? encrypted = null) { port ??= GrpcConstants.DefaultGrpcPort; - encrypted ??= port == GrpcConstants.DefaultEncryptedGrpcPort || port == GrpcConstants.DefaultEphemeralEncryptedGrpcPort || port == GrpcConstants.DefaultEphemeralLeaderEncryptedGrpcPort; + encrypted ??= HasEncryptedPort(port); return new HostInfo(host, port.Value, encrypted!.Value); } @@ -294,4 +305,6 @@ public readonly record struct MachineLocation var info = ToGrpcHost(); return (info.Host, info.Port); } + + private static bool HasEncryptedPort(int? port) => port == GrpcConstants.DefaultEncryptedGrpcPort || port == GrpcConstants.DefaultEphemeralEncryptedGrpcPort || port == GrpcConstants.DefaultEphemeralLeaderEncryptedGrpcPort; } diff --git a/Public/Src/Utilities/Utilities.Core/ExceptionUtilities.cs b/Public/Src/Utilities/Utilities.Core/ExceptionUtilities.cs index 6a92a0638..ce38749f7 100644 --- a/Public/Src/Utilities/Utilities.Core/ExceptionUtilities.cs +++ b/Public/Src/Utilities/Utilities.Core/ExceptionUtilities.cs @@ -69,7 +69,7 @@ namespace BuildXL.Utilities.Core // Protocol errors from gRPC operations look differently in net8 "An HTTP/2 connection could not be established because the server did not complete the HTTP/2 handshake.", // Unobserved timeouts during cache operations (see work item #2162565) - "BuildXL.Cache.ContentStore.Service.Grpc.GrpcConnectionTimeoutException: Failed to connect to grpc://" + "BuildXL.Cache.ContentStore.Service.Grpc.GrpcConnectionTimeoutException: Failed to connect to grpcs://" }; private static bool ContainsAllowedException(Exception exception)