Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SNOW-1313929] Add https to endpoints without it #914

Merged
merged 3 commits into from
Apr 17, 2024
Merged

[SNOW-1313929] Add https to endpoints without it #914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69363bb
[SNOW-1313929] Add https to endpoints without it
sfc-gh-erojaslizano Apr 11, 2024
9bb006b
Change endpoint verification to startWith, change test names
sfc-gh-erojaslizano Apr 16, 2024
e0f2247
remove SetCommonClientConfig to SFS3ClientTest
sfc-gh-erojaslizano Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions Snowflake.Data.Tests/UnitTests/SFS3ClientTest.cs
View file
Open in desktop
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
View file
Open in desktop
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;
}
}
}
}
Loading