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

Fix requester pays access harder. #7716 #7730

Merged
merged 7 commits into from
Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ final guavaVersion = System.getProperty('guava.version', '31.0.1-jre')
final log4j2Version = System.getProperty('log4j2Version', '2.17.1')
final testNGVersion = '7.0.0'

final googleCloudNioDependency = 'com.google.cloud:google-cloud-nio:0.123.23'

final googleCloudNioDependency = 'com.google.cloud:google-cloud-nio:0.123.25'

final baseJarName = 'gatk'
final secondaryBaseJarName = 'hellbender'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package org.broadinstitute.hellbender.engine;

import com.google.cloud.storage.StorageException;
import org.broadinstitute.hellbender.CommandLineProgramTest;
import org.broadinstitute.hellbender.cmdline.StandardArgumentDefinitions;
import org.broadinstitute.hellbender.exceptions.UserException;
import org.broadinstitute.hellbender.testutils.ArgumentsBuilder;
import org.broadinstitute.hellbender.testutils.IntegrationTestSpec;
import org.broadinstitute.hellbender.tools.examples.ExampleReadWalkerWithVariants;
import org.broadinstitute.hellbender.utils.io.IOUtils;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;

public class RequesterPaysIntegrationTest extends CommandLineProgramTest {

@Override
public String getTestedToolName() {
return ExampleReadWalkerWithVariants.class.getSimpleName();
}

@DataProvider
public Object[][] getRequesterPaysPaths() {

//Files here are paid for by the bucket owners
final String OWNER = getGCPTestInputPath() + "org/broadinstitute/hellbender/engine/RequesterPaysIntegrationTest/";

//Files here are requester pays
final String REQUESTER = getGCPRequesterPaysBucket() + "test/resources/nio/";

return new Object[][]{
{OWNER, OWNER, OWNER, OWNER, false},
{OWNER, OWNER, OWNER, REQUESTER, true},
{OWNER, OWNER, REQUESTER, OWNER, true},
{OWNER, OWNER, REQUESTER, REQUESTER, true},
{OWNER, REQUESTER, OWNER, OWNER, true},
{OWNER, REQUESTER, OWNER, REQUESTER, true},
{OWNER, REQUESTER, REQUESTER, OWNER, true},
{OWNER, REQUESTER, REQUESTER, REQUESTER, true},
{REQUESTER, OWNER, OWNER, OWNER, true},
{REQUESTER, OWNER, OWNER, REQUESTER, true},
{REQUESTER, OWNER, REQUESTER, OWNER, true},
{REQUESTER, OWNER, REQUESTER, REQUESTER, true},
{REQUESTER, REQUESTER, OWNER, OWNER, true},
{REQUESTER, REQUESTER, OWNER, REQUESTER, true},
{REQUESTER, REQUESTER, REQUESTER, OWNER, true},
{REQUESTER, REQUESTER, REQUESTER, REQUESTER, true},
};
}

// Disabled due to https://github.com/broadinstitute/gatk/issues/7763
@Test(dataProvider = "getRequesterPaysPaths", groups="cloud", enabled = false)
public void testMixedNormalAndRequesterPays(String referenceBase, String bamBase, String vcfBase,
String intervalBase, boolean requiresRequesterPays) throws IOException {
final ArgumentsBuilder args = new ArgumentsBuilder();
final File output = IOUtils.createTempFile("out", ".txt");
args.addReference(referenceBase + "hg19mini.fasta")
.addInput(bamBase + "reads_data_source_test1.bam" )
.addVCF(vcfBase + "example_variants_withSequenceDict.vcf")
.addInterval(intervalBase + "hg19mini.all.interval_list")
.addOutput(output)
.add(StandardArgumentDefinitions.NIO_PROJECT_FOR_REQUESTER_PAYS_LONG_NAME, getGCPTestProject());
runCommandLine(args);
IntegrationTestSpec.assertEqualTextFiles(output, new File(packageRootTestDir+"engine/RequesterPaysIntegrationTest/expected_ExampleReadWalkerWithVariantsIntegrationTest_output.txt"));
}

// Disabled due to https://github.com/broadinstitute/gatk/issues/7763
@Test(dataProvider = "getRequesterPaysPaths", groups="cloud", enabled = false)
public void testWithoutRequesterPaysArgument(String referenceBase, String bamBase, String vcfBase,
String intervalBase, boolean requiresRequesterPays) {
final ArgumentsBuilder args = new ArgumentsBuilder();
args.addReference(referenceBase + "hg19mini.fasta")
.addInput(bamBase + "reads_data_source_test1.bam" )
.addVCF(vcfBase + "example_variants_withSequenceDict.vcf")
.addInterval(intervalBase + "hg19mini.all.interval_list")
.addOutput(IOUtils.createTempFile("out", ".txt"));
try{
runCommandLine(args);
Assert.assertFalse(requiresRequesterPays, "This shouldn't have reached here because it should if thrown.");
} catch (final UserException.CouldNotReadInputFile | StorageException ex){
if( !requiresRequesterPays){
Assert.fail("This should have thrown an exception.");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.google.cloud.storage.StorageException;
import com.google.cloud.storage.contrib.nio.CloudStorageConfiguration;
import com.google.cloud.storage.contrib.nio.CloudStorageFileSystemProvider;
import com.google.cloud.storage.contrib.nio.SeekableByteChannelPrefetcher;
import htsjdk.samtools.util.IOUtil;
import org.broadinstitute.hellbender.GATKBaseTest;
Expand All @@ -17,6 +18,8 @@
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.channels.SeekableByteChannel;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -34,7 +37,7 @@ public final class BucketUtilsUnitTest extends GATKBaseTest {
* a different project than the service account doing the testing or the test may fail because it can access the
* file directly through alternative permissions.
*/
public static final String FILE_IN_REQUESTER_PAYS_BUCKET = "gs://hellbender-requester-pays-test/test/resources/nio/big.txt";
public static final String FILE_IN_REQUESTER_PAYS_BUCKET = getGCPRequesterPaysBucket() + "test/resources/nio/big.txt";

static {
setDefaultNioOptions();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
Read at 1:200-275:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC
Overlapping variant at 1:200-200: Ref: G* Alt(s): [A]
Overlapping variant at 1:203-206: Ref: GGGG* Alt(s): [G]

Read at 1:205-280:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC
Overlapping variant at 1:203-206: Ref: GGGG* Alt(s): [G]
Overlapping variant at 1:280-280: Ref: G* Alt(s): [A]

Read at 1:210-285:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC
Overlapping variant at 1:280-280: Ref: G* Alt(s): [A]
Overlapping variant at 1:284-286: Ref: GGG* Alt(s): [G]
Overlapping variant at 1:285-285: Ref: G* Alt(s): [A]

Read at 1:1000-1075:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC
Overlapping variant at 1:1000-1000: Ref: G* Alt(s): [A]

Read at 1:1100-1175:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC

Read at 2:500-575:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC

Read at 2:550-625:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC

Read at 2:650-725:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC

Read at 3:300-375:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC

Read at 3:400-475:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC

Read at 4:700-775:
ACCCTAACCCTAACCCTAACCCTAACCATAACCCTAAGACTAACCCTAAACCTAACCCTCATAATCGAAATACAAC
Overlapping variant at 4:775-775: Ref: G* Alt(s): [A]

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.broadinstitute.hellbender.testutils;

import com.google.common.base.Strings;
import htsjdk.samtools.util.Log;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -107,6 +108,23 @@ public void setTestVerbosity(){

public static final Logger logger = LogManager.getLogger("org.broadinstitute.gatk");

/**
* This is a public requester pays bucket owned by the broad-gatk-test project.
* It must be owned by a different project than the service account doing the testing or the test may fail because it can access the
* file directly through alternative permissions.
*/
public static final String REQUESTER_PAYS_BUCKET = "gs://hellbender-requester-pays-test/";

/**
* A publicly readable GCS bucket set as requester pays, this should not be owned by the same project that is set
* as {@link BaseTest#getGCPTestProject()} or the tests for requester pays access may be invalid.
* @return HELLBENDER_REQUESTER_PAYS_BUCKET env. var if defined, {@value BaseTest#REQUESTER_PAYS_BUCKET}.
*/
public static final String getGCPRequesterPaysBucket(){
final String valueFromEnvironment = System.getenv("HELLBENDER_REQUESTER_PAYS_BUCKET");
return Strings.isNullOrEmpty(valueFromEnvironment) ? REQUESTER_PAYS_BUCKET : valueFromEnvironment;
}

/**
* name of the google cloud project that stores the data and will run the code
* @return HELLBENDER_TEST_PROJECT env. var if defined, throws otherwise.
Expand Down