Skip to content

Commit

Permalink
[SNOW-1313929] Add https to endpoints without it (#914)
Browse files Browse the repository at this point in the history
### Description
- Add https to endpoints without it
- Add unit test for SetCommonClientConfig

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
  • Loading branch information
sfc-gh-erojaslizano authored Apr 17, 2024
1 parent 46e496a commit 47235fb
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 14 deletions.
31 changes: 31 additions & 0 deletions Snowflake.Data.Tests/UnitTests/SFS3ClientTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/

using System;
using Amazon.S3.Encryption;

namespace Snowflake.Data.Tests.UnitTests
{
Expand Down Expand Up @@ -219,6 +220,36 @@ public void TestUploadFile(string requestKey, ResultStatus expectedResultStatus)
AssertForUploadFileTests(expectedResultStatus);
}

[Test]
public void TestAppendHttpsToEndpoint()
{
// Arrange
var amazonS3Client = new AmazonS3Config();
var endpoint = "endpointWithNoHttps.com";
var expectedEndpoint = "https://endpointWithNoHttps.com";

// ACT
SFS3Client.SetCommonClientConfig(amazonS3Client, string.Empty, endpoint, 1, 0);

// Assert
Assert.That(amazonS3Client.ServiceURL, Is.EqualTo(expectedEndpoint));
}

[Test]
public void TestAppendHttpsToEndpointWithBrackets()
{
// Arrange
var amazonS3Client = new AmazonS3Config();
var endpoint = "[endpointWithNoHttps.com]";
var expectedEndpoint = "https://endpointWithNoHttps.com";

// ACT
SFS3Client.SetCommonClientConfig(amazonS3Client, string.Empty, endpoint, 1, 0);

// Assert
Assert.That(amazonS3Client.ServiceURL, Is.EqualTo(expectedEndpoint));
}

[Test]
[TestCase(MockS3Client.AwsStatusOk, ResultStatus.UPLOADED)]
[TestCase(SFS3Client.EXPIRED_TOKEN, ResultStatus.RENEW_TOKEN)]
Expand Down
29 changes: 15 additions & 14 deletions Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public SFS3Client(
stageInfo.endPoint,
maxRetry,
parallel);

// Get the AWS token value and create the S3 client
if (stageInfo.stageCredentials.TryGetValue(AWS_TOKEN, out string awsSessionToken))
{
Expand Down Expand Up @@ -164,7 +164,7 @@ public RemoteLocation ExtractBucketNameAndPath(string stageLocation)
{
bucketName = stageLocation.Substring(0, stageLocation.IndexOf('/'));

s3path = stageLocation.Substring(stageLocation.IndexOf('/') + 1,
s3path = stageLocation.Substring(stageLocation.IndexOf('/') + 1,
stageLocation.Length - stageLocation.IndexOf('/') - 1);
if (s3path != null && !s3path.EndsWith("/"))
{
Expand Down Expand Up @@ -287,13 +287,13 @@ private FileHeader HandleFileHeaderResponse(ref SFFileMetadata fileMetadata, Get
}

/// <summary>
/// Set the client configuration common to both client with and without client-side
/// Set the client configuration common to both client with and without client-side
/// encryption.
/// </summary>
/// <param name="clientConfig">The client config to update.</param>
/// <param name="region">The region if any.</param>
/// <param name="endpoint">The endpoint if any.</param>
private static void SetCommonClientConfig(
internal static void SetCommonClientConfig(
AmazonS3Config clientConfig,
string region,
string endpoint,
Expand All @@ -309,23 +309,25 @@ private static void SetCommonClientConfig(
}

// If a specific endpoint is specified use this
if ((null != endpoint) && (0 != endpoint.Length))
if (!string.IsNullOrEmpty(endpoint))
{
var start = endpoint.IndexOf('[');
var end = endpoint.IndexOf(']');
if(start > -1 && end > -1 && end > start)
if (start > -1 && end > -1 && end > start)
{
endpoint = endpoint.Substring(start + 1, end - start - 1);
if(!endpoint.Contains("https"))
{
endpoint = "https://" + endpoint;
}
}

if (!endpoint.StartsWith("https://", StringComparison.OrdinalIgnoreCase))
{
endpoint = "https://" + endpoint;
}

clientConfig.ServiceURL = endpoint;
}

// The region information used to determine the endpoint for the service.
// RegionEndpoint and ServiceURL are mutually exclusive properties.
// RegionEndpoint and ServiceURL are mutually exclusive properties.
// If both stageInfo.endPoint and stageInfo.region have a value, stageInfo.region takes
// precedence and ServiceUrl will be reset to null.
if ((null != region) && (0 != region.Length))
Expand All @@ -337,7 +339,6 @@ private static void SetCommonClientConfig(
// Unavailable for .net framework 4.6
//clientConfig.MaxConnectionsPerServer = parallel;
clientConfig.MaxErrorRetry = maxRetry;

}

/// <summary>
Expand Down Expand Up @@ -410,7 +411,7 @@ private PutObjectRequest GetPutObjectRequest(ref AmazonS3Client client, SFFileMe
{
PutGetStageInfo stageInfo = fileMetadata.stageInfo;
RemoteLocation location = ExtractBucketNameAndPath(stageInfo.location);

// Create S3 PUT request
fileBytesStream.Position = 0;
PutObjectRequest putObjectRequest = new PutObjectRequest
Expand Down Expand Up @@ -585,4 +586,4 @@ private SFFileMetadata HandleDownloadFileErr(Exception ex, SFFileMetadata fileMe
return fileMetadata;
}
}
}
}

0 comments on commit 47235fb

Please sign in to comment.