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.

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

WX-1174 Adjust NIO Copy functionality #7207

Merged
merged 10 commits into from
Aug 22, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public final class AzureFileSystem extends FileSystem {
private final Map<String, FileStore> fileStores;
private FileStore defaultFileStore;
private boolean closed;
private AzureSasCredential currentActiveSasCredential;

AzureFileSystem(AzureFileSystemProvider parentFileSystemProvider, String endpoint, Map<String, ?> config)
throws IOException {
Expand All @@ -179,6 +180,7 @@ public final class AzureFileSystem extends FileSystem {
this.putBlobThreshold = (Long) config.get(AZURE_STORAGE_PUT_BLOB_THRESHOLD);
this.maxConcurrencyPerRequest = (Integer) config.get(AZURE_STORAGE_MAX_CONCURRENCY_PER_REQUEST);
this.downloadResumeRetries = (Integer) config.get(AZURE_STORAGE_DOWNLOAD_RESUME_RETRIES);
this.currentActiveSasCredential = (AzureSasCredential) config.get(AZURE_STORAGE_SAS_TOKEN_CREDENTIAL);

// Initialize and ensure access to FileStores.
this.fileStores = this.initializeFileStores(config);
Expand Down Expand Up @@ -489,4 +491,11 @@ Long getPutBlobThreshold() {
Integer getMaxConcurrencyPerRequest() {
return this.maxConcurrencyPerRequest;
}

public String createSASAppendedURL(String url) throws IllegalStateException {
if(Objects.isNull(currentActiveSasCredential)) {
throw new IllegalStateException("No current active SAS credential present");
}
return url + "?" + currentActiveSasCredential.getSignature();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to look like

https://example.com?credential

or

https://example.com?key=credential

Copy link
Contributor Author

@JVThomas JVThomas Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, you just simply append the credential to the end of a Azure Storage URI
https://learn.microsoft.com/en-us/azure/storage/common/storage-sas-overview#how-a-shared-access-signature-works

The format's new to me too, I had the same exact reaction when I first saw that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, misread your comment. It's the first option so it'l look like...
https://example.com?sv=...

...where the sv=... portion is the Azure SAS token

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, appreciate the link!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.concurrent.ConcurrentMap;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import com.azure.core.util.CoreUtils;
import com.azure.core.util.logging.ClientLogger;
Expand Down Expand Up @@ -695,16 +696,23 @@ public void copy(Path source, Path destination, CopyOption... copyOptions) throw
// Remove accepted options as we find them. Anything left we don't support.
boolean replaceExisting = false;
List<CopyOption> optionsList = new ArrayList<>(Arrays.asList(copyOptions));
if (!optionsList.contains(StandardCopyOption.COPY_ATTRIBUTES)) {
throw LoggingUtility.logError(ClientLoggerHolder.LOGGER, new UnsupportedOperationException(
"StandardCopyOption.COPY_ATTRIBUTES must be specified as the service will always copy "
+ "file attributes."));
// NOTE: We're going to assume COPY_ATTRIBUTES as a default copy option (but can still be provided and handled safely)
// REPLACE_EXISTING must still be provided if you want to replace existing file

// if (!optionsList.contains(StandardCopyOption.COPY_ATTRIBUTES)) {
// throw LoggingUtility.logError(ClientLoggerHolder.LOGGER, new UnsupportedOperationException(
// "StandardCopyOption.COPY_ATTRIBUTES must be specified as the service will always copy "
// + "file attributes."));
// }
if(optionsList.contains(StandardCopyOption.COPY_ATTRIBUTES)) {
optionsList.remove(StandardCopyOption.COPY_ATTRIBUTES);
}
optionsList.remove(StandardCopyOption.COPY_ATTRIBUTES);

if (optionsList.contains(StandardCopyOption.REPLACE_EXISTING)) {
replaceExisting = true;
optionsList.remove(StandardCopyOption.REPLACE_EXISTING);
}

if (!optionsList.isEmpty()) {
throw LoggingUtility.logError(ClientLoggerHolder.LOGGER,
new UnsupportedOperationException("Unsupported copy option found. Only "
Expand Down Expand Up @@ -760,9 +768,16 @@ public void copy(Path source, Path destination, CopyOption... copyOptions) throw
customer scenarios and how many virtual directories they copy, it could be better to check the directory status
first and then do a copy or createDir, which would always be two requests for all resource types.
*/

try {
/*
Format the url by appending the SAS token as a param, otherwise the copy request will fail.
AzureFileSystem has been updated to handle url transformation via createSASAuthorizedURL()
*/
AzureFileSystem afs = (AzureFileSystem) sourceRes.getPath().getFileSystem();
String sasAppendedSourceUrl = afs.createSASAppendedURL(sourceRes.getBlobClient().getBlobUrl());
SyncPoller<BlobCopyInfo, Void> pollResponse =
destinationRes.getBlobClient().beginCopy(sourceRes.getBlobClient().getBlobUrl(), null, null, null,
destinationRes.getBlobClient().beginCopy(sasAppendedSourceUrl, null, null, null,
null, requestConditions, null);
pollResponse.waitForCompletion(Duration.ofSeconds(COPY_TIMEOUT_SECONDS));
} catch (BlobStorageException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<component name="ProjectRunConfigurationManager">
<configuration default="false" name="Repo template: Cromwell server TES" type="Application" factoryName="Application">
<option name="ALTERNATIVE_JRE_PATH" value="$USER_HOME$/.sdkman/candidates/java/current" />
<option name="ALTERNATIVE_JRE_PATH" value="$PROJECT_DIR$/../.sdkman/candidates/java/current" />
<envs>
<env name="CROMWELL_BUILD_CENTAUR_SLICK_PROFILE" value="slick.jdbc.MySQLProfile$" />
<env name="CROMWELL_BUILD_CENTAUR_JDBC_DRIVER" value="com.mysql.cj.jdbc.Driver" />
Expand All @@ -16,6 +16,7 @@
<option name="VM_PARAMETERS" value="-Dconfig.file=target/ci/resources/tes_application.conf" />
<method v="2">
<option name="Make" enabled="true" />
<option name="RunConfigurationTask" enabled="true" run_configuration_name="renderCiResources" run_configuration_type="SbtRunConfiguration" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 👏

</method>
</configuration>
</component>