From 503a1ecbbc2e4f91f961f8ba69c6c1464be357e6 Mon Sep 17 00:00:00 2001 From: Panos Chatzopoulos Date: Thu, 17 Oct 2024 00:55:10 +0200 Subject: [PATCH 1/6] config can be provided before command --- download/download.go | 9 ++++++--- htsget/htsget.go | 7 +++++-- list/list.go | 7 +++++-- main.go | 30 +++++++++++++++++++++--------- upload/upload.go | 7 +++++-- 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/download/download.go b/download/download.go index 5c62977..14e1819 100644 --- a/download/download.go +++ b/download/download.go @@ -96,14 +96,17 @@ type File struct { // Download function downloads files from the SDA by using the // download's service APIs -func Download(args []string) error { +func Download(args []string, configPathF string) error { // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } + if configPathF == "" { + configPathF = *configPath + } - if *datasetID == "" || *URL == "" || *configPath == "" { + if *datasetID == "" || *URL == "" || configPathF == "" { return fmt.Errorf("missing required arguments, dataset, config and url are required") } @@ -136,7 +139,7 @@ func Download(args []string) error { } // Get the configuration file or the .sda-cli-session - config, err := helpers.GetAuth(*configPath) + config, err := helpers.GetAuth(configPathF) if err != nil { return err } diff --git a/htsget/htsget.go b/htsget/htsget.go index 7fbf6ad..60b1a62 100644 --- a/htsget/htsget.go +++ b/htsget/htsget.go @@ -73,19 +73,22 @@ type htsgetResponse struct { // Htsget function downloads the files included in the urls_list.txt file. // The argument can be a local file or a url to an S3 folder -func Htsget(args []string) error { +func Htsget(args []string, configPathF string) error { // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } + if configPathF == "" { + configPathF = *configPath + } if *datasetID == "" || *fileName == "" || *htsgetHost == "" || *publicKeyFile == "" { return fmt.Errorf("missing required arguments, dataset, filename, host and key are required") } - config, err := helpers.GetAuth(*configPath) + config, err := helpers.GetAuth(configPathF) if err != nil { return err } diff --git a/list/list.go b/list/list.go index feecdbe..f227595 100644 --- a/list/list.go +++ b/list/list.go @@ -52,12 +52,15 @@ var bytesFormat = Args.Bool("bytes", false, "Print file sizes in bytes (not huma var dataset = Args.String("dataset", "", "List all files in the specified dataset.") // List function lists the contents of an s3 -func List(args []string) error { +func List(args []string, configPathF string) error { // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } + if configPathF == "" { + configPathF = *configPath + } prefix := "" if len(Args.Args()) == 1 { @@ -65,7 +68,7 @@ func List(args []string) error { } // // Get the configuration file or the .sda-cli-session - config, err := helpers.GetAuth(*configPath) + config, err := helpers.GetAuth(configPathF) if err != nil { return fmt.Errorf("failed to load config file, reason: %v", err) } diff --git a/main.go b/main.go index dff4c4a..0a23e62 100644 --- a/main.go +++ b/main.go @@ -49,7 +49,7 @@ var Commands = map[string]commandInfo{ func main() { log.SetLevel(log.WarnLevel) - command, args := ParseArgs() + command, args, configPath := ParseArgs() var err error @@ -61,15 +61,15 @@ func main() { case "decrypt": err = decrypt.Decrypt(args) case "upload": - err = upload.Upload(args) + err = upload.Upload(args, configPath) case "list": - err = list.List(args) + err = list.List(args, configPath) case "htsget": - err = htsget.Htsget(args) + err = htsget.Htsget(args, configPath) case "login": err = login.NewLogin(args) case "download": - err = download.Download(args) + err = download.Download(args, configPath) case "version": err = version.Version(Version) default: @@ -84,7 +84,8 @@ func main() { // Parses the command line arguments into a command, and keep the rest // of the arguments for the subcommand. -func ParseArgs() (string, []string) { +func ParseArgs() (string, []string, string) { + var configPath string // Print usage if no arguments are provided. if len(os.Args) < 2 { _ = Help("help") @@ -98,7 +99,18 @@ func ParseArgs() (string, []string) { os.Exit(0) } - return "version", os.Args + return "version", os.Args, "" + } + + // If config comes before the command, we remove it from the + // arguments and set the global config path. + if len(os.Args) > 1 && (os.Args[1] == "--config" || os.Args[1] == "-config") { + if len(os.Args) < 3 { + fmt.Fprintf(os.Stderr, "Error: --config requires an argument\n") + os.Exit(1) + } + configPath = os.Args[2] + os.Args = append(os.Args[:1], os.Args[3:]...) } // Extract the command from the 1st argument, then remove it @@ -130,7 +142,7 @@ func ParseArgs() (string, []string) { // The "list" command can have no arguments since it can use the // config from login so we immediately return in that case. if command == "list" { - return command, os.Args + return command, os.Args, configPath } // If no arguments are provided to the subcommand, it's not @@ -141,7 +153,7 @@ func ParseArgs() (string, []string) { os.Exit(1) } - return command, os.Args + return command, os.Args, configPath } // Prints the main usage string, and the global help or command help diff --git a/upload/upload.go b/upload/upload.go index 5f465d1..b4c5f80 100644 --- a/upload/upload.go +++ b/upload/upload.go @@ -256,7 +256,7 @@ func createFilePaths(dirPath string) ([]string, []string, error) { // Upload function uploads files to the s3 bucket. Input can be files or // directories to be uploaded recursively -func Upload(args []string) error { +func Upload(args []string, configPathF string) error { var files []string var outFiles []string *pubKeyPath = "" @@ -267,6 +267,9 @@ func Upload(args []string) error { if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } + if configPathF == "" { + configPathF = *configPath + } // Dereference the pointer to a string var targetDirString string @@ -288,7 +291,7 @@ func Upload(args []string) error { } // Get the configuration file or the .sda-cli-session - config, err := helpers.GetAuth(*configPath) + config, err := helpers.GetAuth(configPathF) if err != nil { return err } From b601045d3dbb3fe18ba2360061f1ab293348eb49 Mon Sep 17 00:00:00 2001 From: Panos Chatzopoulos Date: Mon, 21 Oct 2024 16:01:34 +0200 Subject: [PATCH 2/6] fix tests --- .github/integration/tests/tests.sh | 4 ++-- download/download_test.go | 2 +- htsget/htsget_test.go | 10 ++++---- list/list_test.go | 6 ++--- upload/upload_test.go | 38 +++++++++++++++--------------- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/.github/integration/tests/tests.sh b/.github/integration/tests/tests.sh index dc72340..e33596d 100755 --- a/.github/integration/tests/tests.sh +++ b/.github/integration/tests/tests.sh @@ -150,7 +150,7 @@ else fi # Download file by using the sda download service -./sda-cli download -config testing/s3cmd-download.conf -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir test-download main/subfolder/dummy_data.c4gh +./sda-cli -config testing/s3cmd-download.conf download -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir test-download main/subfolder/dummy_data.c4gh # Check if file exists in the path if [ ! -f "test-download/main/subfolder/dummy_data" ]; then @@ -168,7 +168,7 @@ fi rm -r test-download # Check listing files in a dataset -output=$(./sda-cli list -config testing/s3cmd-download.conf -dataset https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080) +output=$(./sda-cli -config testing/s3cmd-download.conf list -dataset https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080) expected="FileIDSizePathurn:neic:001-0011.0MB5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8_elixir-europe.org/main/subfolder/dummy_data.c4ghurn:neic:001-0021.0MB5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8_elixir-europe.org/main/subfolder2/dummy_data2.c4ghurn:neic:001-0031.0MB5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8_elixir-europe.org/main/subfolder2/random/dummy_data3.c4ghDatasetsize:3.1MB" if [[ "${output//[$' \t\n\r']/}" == "${expected//[$' \t\n\r']/}" ]]; then echo "Successfully listed files in dataset" diff --git a/download/download_test.go b/download/download_test.go index 348c1ca..5b1e478 100644 --- a/download/download_test.go +++ b/download/download_test.go @@ -76,7 +76,7 @@ func (suite *TestSuite) TestInvalidUrl() { "file2", } - err := Download(os.Args) + err := Download(os.Args, "") assert.Contains( suite.T(), err.Error(), diff --git a/htsget/htsget_test.go b/htsget/htsget_test.go index 25beed3..e8c60b6 100644 --- a/htsget/htsget_test.go +++ b/htsget/htsget_test.go @@ -22,7 +22,7 @@ func TestConfigTestSuite(t *testing.T) { func (suite *TestSuite) MissingArgument() { os.Args = []string{"htsget"} - err := Htsget(os.Args) + err := Htsget(os.Args, "") assert.EqualError(suite.T(), err, "missing required arguments, dataset, filename, host and key are required") } @@ -30,7 +30,7 @@ func (suite *TestSuite) MissingArgument() { func (suite *TestSuite) TestHtsgetMissingConfig() { os.Args = []string{"htsget", "-config", "nonexistent.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} - err := Htsget(os.Args) + err := Htsget(os.Args, "") msg := "no such file or directory" if runtime.GOOS == "windows" { msg = "open nonexistent.conf: The system cannot find the file specified." @@ -59,7 +59,7 @@ access_token = eyJ0eXAiOiJKV1QiLCJqa3UiOiJodHRwczovL29pZGM6ODA4MC9qd2siLCJhbGciO panic(err) } os.Args = []string{"htsget", "-config", tmpDir + "s3cmd_test.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} - err = Htsget(os.Args) + err = Htsget(os.Args, "") assert.ErrorContains(suite.T(), err, "failed to read public key") } @@ -91,7 +91,7 @@ KKj6NUcJGZ2/HeqkYbxm57ZaFLP5cIHsdK+0nQubFVs= panic(err) } os.Args = []string{"htsget", "-config", tmpDir + "s3cmd_test.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "missingserver", "-pubkey", tmpDir + "c4gh.pub.pem"} - err = Htsget(os.Args) + err = Htsget(os.Args, "") assert.ErrorContains(suite.T(), err, "failed to do the request") } @@ -185,6 +185,6 @@ KKj6NUcJGZ2/HeqkYbxm57ZaFLP5cIHsdK+0nQubFVs= defer server.Close() os.Args = []string{"htsget", "-config", tmpDir + "s3cmd_test.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", server.URL, "-pubkey", tmpDir + "c4gh.pub.pem"} - err = Htsget(os.Args) + err = Htsget(os.Args, "") assert.ErrorContains(suite.T(), err, "error downloading the files") } diff --git a/list/list_test.go b/list/list_test.go index e17dcbf..d8a0600 100644 --- a/list/list_test.go +++ b/list/list_test.go @@ -39,7 +39,7 @@ func (suite *TestSuite) TestNoConfig() { os.Args = []string{"list", "-config", ""} - err := List(os.Args) + err := List(os.Args, "") assert.EqualError(suite.T(), err, "failed to load config file, reason: failed to read the configuration file") } @@ -127,7 +127,7 @@ func (suite *TestSuite) TestFunctionality() { // Upload a file os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", dir} - err = upload.Upload(os.Args) + err = upload.Upload(os.Args, "") assert.NoError(suite.T(), err) // Check logs that file was uploaded @@ -142,7 +142,7 @@ func (suite *TestSuite) TestFunctionality() { os.Stdout = w os.Args = []string{"list", "-config", configPath.Name()} - err = List(os.Args) + err = List(os.Args, "") assert.NoError(suite.T(), err) w.Close() diff --git a/upload/upload_test.go b/upload/upload_test.go index f5b6f17..926739e 100644 --- a/upload/upload_test.go +++ b/upload/upload_test.go @@ -72,15 +72,15 @@ func (suite *TestSuite) TestSampleNoFiles() { // Test Upload function os.Args = []string{"upload", "-config", configPath.Name()} - assert.EqualError(suite.T(), Upload(os.Args), "no files to upload") + assert.EqualError(suite.T(), Upload(os.Args, ""), "no files to upload") // Test handling of mistakenly passing a filename as an upload folder os.Args = []string{"upload", "-config", configPath.Name(), "-targetDir", configPath.Name()} - assert.EqualError(suite.T(), Upload(os.Args), configPath.Name()+" is not a valid target directory") + assert.EqualError(suite.T(), Upload(os.Args, ""), configPath.Name()+" is not a valid target directory") // Test handling of mistakenly passing a flag as an upload folder os.Args = []string{"upload", "-config", configPath.Name(), "-targetDir", "-r"} - assert.EqualError(suite.T(), Upload(os.Args), "-r"+" is not a valid target directory") + assert.EqualError(suite.T(), Upload(os.Args, ""), "-r"+" is not a valid target directory") // Test passing flags at the end as well @@ -89,10 +89,10 @@ func (suite *TestSuite) TestSampleNoFiles() { msg = "CreateFile somefileOrfolder: The system cannot find the file specified." } os.Args = []string{"upload", "-config", configPath.Name(), "-r", "somefileOrfolder", "-targetDir", "somedir"} - assert.EqualError(suite.T(), Upload(os.Args), msg) + assert.EqualError(suite.T(), Upload(os.Args, ""), msg) os.Args = []string{"upload", "-config", configPath.Name(), "somefiles", "-targetDir"} - assert.EqualError(suite.T(), Upload(os.Args), "no files to upload") + assert.EqualError(suite.T(), Upload(os.Args, ""), "no files to upload") // Test uploadFiles function config, _ := helpers.LoadConfigFile(configPath.Name()) @@ -220,7 +220,7 @@ func (suite *TestSuite) TestFunctionality() { // Test recursive upload os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", dir} - assert.NoError(suite.T(), Upload(os.Args)) + assert.NoError(suite.T(), Upload(os.Args, "")) // Check logs that file was uploaded logMsg := strings.ReplaceAll(fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")), "\\\\", "\\") @@ -243,7 +243,7 @@ func (suite *TestSuite) TestFunctionality() { // Test upload to a different folder targetPath := filepath.Join("a", "b", "c") os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), testfile.Name(), "-targetDir", targetPath} - assert.NoError(suite.T(), Upload(os.Args)) + assert.NoError(suite.T(), Upload(os.Args, "")) // Check logs that file was uploaded logMsg = fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")) msg = fmt.Sprintf("file uploaded to %s/dummy/%s/%s", ts.URL, filepath.ToSlash(targetPath), filepath.Base(testfile.Name())) @@ -280,7 +280,7 @@ func (suite *TestSuite) TestFunctionality() { // Empty buffer logs str.Reset() newArgs := []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "--encrypt-with-key", publicKey.Name(), testfile.Name(), "-targetDir", "someDir"} - assert.NoError(suite.T(), Upload(newArgs)) + assert.NoError(suite.T(), Upload(newArgs, "")) // Check logs that encrypted file was uploaded logMsg = fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")) @@ -302,17 +302,17 @@ func (suite *TestSuite) TestFunctionality() { // Check that trying to encrypt already encrypted files returns error and aborts newArgs = []string{"upload", "-config", configPath.Name(), "--encrypt-with-key", publicKey.Name(), dir, "-r"} - assert.EqualError(suite.T(), Upload(newArgs), "aborting") + assert.EqualError(suite.T(), Upload(newArgs, ""), "aborting") // Check handling of passing source files as pub key // (code checks first for errors related with file args) newArgs = []string{"upload", "-config", configPath.Name(), "--encrypt-with-key", testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs), "no files to upload") + assert.EqualError(suite.T(), Upload(newArgs, ""), "no files to upload") // If both a bad key and already encrypted file args are given, // file arg errors are captured first newArgs = []string{"upload", "-config", configPath.Name(), "--encrypt-with-key", "somekey", testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs), "aborting") + assert.EqualError(suite.T(), Upload(newArgs, ""), "aborting") // config file without an access_token var confFileNoToken = fmt.Sprintf(` @@ -338,24 +338,24 @@ func (suite *TestSuite) TestFunctionality() { // Check that an access token is supplied newArgs = []string{"upload", "-config", configPath.Name(), testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs), "no access token supplied") + assert.EqualError(suite.T(), Upload(newArgs, ""), "no access token supplied") os.Setenv("ACCESSTOKEN", "BadToken") // Supplying an accesstoken as a ENV overrules the one in the config file newArgs = []string{"upload", "-config", configPath.Name(), testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs), "could not parse token, reason: token contains an invalid number of segments") + assert.EqualError(suite.T(), Upload(newArgs, ""), "could not parse token, reason: token contains an invalid number of segments") suite.SetupTest() os.Setenv("ACCESSTOKEN", suite.accessToken) newArgs = []string{"upload", "-config", configPath.Name(), testfile.Name()} - assert.NoError(suite.T(), Upload(newArgs)) + assert.NoError(suite.T(), Upload(newArgs, "")) // Supplying an accesstoken as a parameter overrules the one in the config file newArgs = []string{"upload", "-accessToken", "BadToken", "-config", configPath.Name(), testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs), "could not parse token, reason: token contains an invalid number of segments") + assert.EqualError(suite.T(), Upload(newArgs, ""), "could not parse token, reason: token contains an invalid number of segments") newArgs = []string{"upload", "-accessToken", suite.accessToken, "-config", configPath.Name(), testfile.Name()} - assert.NoError(suite.T(), Upload(newArgs)) + assert.NoError(suite.T(), Upload(newArgs, "")) // Remove hash files created by Encrypt if err := os.Remove("checksum_encrypted.md5"); err != nil { @@ -453,7 +453,7 @@ func (suite *TestSuite) TestRecursiveToDifferentTarget() { // Test recursive upload to a different folder targetPath := filepath.Join("a", "b", "c") os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", dir, "-targetDir", targetPath} - assert.NoError(suite.T(), Upload(os.Args)) + assert.NoError(suite.T(), Upload(os.Args, "")) // Check logs that file was uploaded logMsg := fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")) msg := fmt.Sprintf("file uploaded to %s/dummy/%s", ts.URL, filepath.ToSlash(filepath.Join(targetPath, filepath.Base(dir), filepath.Base(testfile.Name())))) @@ -538,7 +538,7 @@ func (suite *TestSuite) TestUploadInvalidCharacters() { badchar := string(badc) targetDir := "test" + badchar + "dir" os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-targetDir", targetDir, "-r", testfile.Name()} - err = Upload(os.Args) + err = Upload(os.Args, "") assert.Error(suite.T(), err) assert.Equal(suite.T(), targetDir+" is not a valid target directory", err.Error()) } @@ -564,7 +564,7 @@ func (suite *TestSuite) TestUploadInvalidCharacters() { defer os.Remove(testfile.Name()) os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", testfile.Name()} - err = Upload(os.Args) + err = Upload(os.Args, "") assert.Error(suite.T(), err) assert.Equal(suite.T(), fmt.Sprintf("filepath %v contains disallowed characters: %+v", testfilepath, badchar), err.Error()) } From 072234b5bff0bf266dd23abe09d67b6eee3abd2d Mon Sep 17 00:00:00 2001 From: Panos Chatzopoulos Date: Tue, 22 Oct 2024 13:18:02 +0200 Subject: [PATCH 3/6] config comes only before command --- .github/integration/tests/tests.sh | 30 ++++++------ README.md | 38 +++++++-------- download/download.go | 13 ++--- download/download_test.go | 4 +- htsget/htsget.go | 10 ++-- htsget/htsget_test.go | 16 +++--- list/list.go | 12 ++--- list/list_test.go | 10 ++-- main.go | 2 +- upload/upload.go | 20 +++----- upload/upload_test.go | 78 +++++++++++++++--------------- 11 files changed, 105 insertions(+), 128 deletions(-) diff --git a/.github/integration/tests/tests.sh b/.github/integration/tests/tests.sh index e33596d..618116f 100755 --- a/.github/integration/tests/tests.sh +++ b/.github/integration/tests/tests.sh @@ -49,11 +49,11 @@ files="data_file.c4gh" check_encypted_file $files # Upload a specific file and check it -./sda-cli upload -config testing/s3cmd.conf data_file.c4gh +./sda-cli -config testing/s3cmd.conf upload data_file.c4gh check_uploaded_file "test/$user/data_file.c4gh" data_file.c4gh -if ./sda-cli list -config testing/s3cmd.conf | grep -q 'data_file.c4gh' +if ./sda-cli -config testing/s3cmd.conf list | grep -q 'data_file.c4gh' then echo "Listed file from s3 backend" else @@ -62,7 +62,7 @@ else fi # Try to upload a file twice with the --force-overwrite flag -output=$(./sda-cli upload -config testing/s3cmd.conf --force-overwrite data_file.c4gh) +output=$(./sda-cli -config testing/s3cmd.conf upload --force-overwrite data_file.c4gh) # Create and encrypt multiple files in a folder @@ -76,7 +76,7 @@ check_encypted_file "data_files_enc/data_file.c4gh data_files_enc/data_file1.c4g for k in data_file.c4gh data_file1.c4gh do # Upload and check file - ./sda-cli upload -config testing/s3cmd.conf --force-overwrite "data_files_enc/$k" + ./sda-cli -config testing/s3cmd.conf upload --force-overwrite "data_files_enc/$k" check_uploaded_file "test/$user/$k" "$k" done @@ -90,7 +90,7 @@ cp data_files_enc/data_file.c4gh data_files_enc/dir1/dir2/data_file.c4gh cp data_files_enc/data_file.c4gh data_files_enc/dir1/dir2/data_file2.c4gh # Upload a folder recursively and a single file -./sda-cli upload -config testing/s3cmd.conf -r data_files_enc/dir1 data_files_enc/data_file3.c4gh +./sda-cli -config testing/s3cmd.conf upload -r data_files_enc/dir1 data_files_enc/data_file3.c4gh # Check that files were uploaded with the local path prefix `data_files_enc` stripped from the target path for k in dir1/data_file.c4gh dir1/dir2/data_file.c4gh dir1/dir2/data_file2.c4gh data_file3.c4gh @@ -102,10 +102,10 @@ done # Upload a folder recursively and a single file in a specified upload folder uploadDir="testfolder" -./sda-cli upload -config testing/s3cmd.conf -targetDir "$uploadDir" -r data_files_enc/dir1 data_files_enc/data_file3.c4gh +./sda-cli -config testing/s3cmd.conf upload -targetDir "$uploadDir" -r data_files_enc/dir1 data_files_enc/data_file3.c4gh # Do it again to test that we can pass -targetDir at the end -./sda-cli upload --force-overwrite -config testing/s3cmd.conf -r data_files_enc/dir1 data_files_enc/data_file3.c4gh -targetDir "$uploadDir" +./sda-cli -config testing/s3cmd.conf upload --force-overwrite -r data_files_enc/dir1 data_files_enc/data_file3.c4gh -targetDir "$uploadDir" # Check that files were uploaded with the local path prefix `data_files_enc` stripped from the # target path and into the specified upload folder @@ -117,7 +117,7 @@ done # Upload all contents of a folder recursively to a specified upload folder uploadDir="testfolder2" -./sda-cli upload -config testing/s3cmd.conf -targetDir "$uploadDir" -r data_files_enc/dir1/. +./sda-cli -config testing/s3cmd.conf upload -targetDir "$uploadDir" -r data_files_enc/dir1/. # Check that files were uploaded with the local path prefix `data_files_enc/dir1` stripped from the # target path and into the specified upload folder @@ -132,7 +132,7 @@ mkdir data_files_unenc && mkdir data_files_unenc/dir1 cp data_file data_files_unenc/. && cp data_file data_files_unenc/dir1/data_file1 uploadDir="testEncryptUpload" -./sda-cli upload -config testing/s3cmd.conf -encrypt-with-key sda_key.pub.pem -r data_files_unenc -targetDir "$uploadDir" +./sda-cli -config testing/s3cmd.conf upload -encrypt-with-key sda_key.pub.pem -r data_files_unenc -targetDir "$uploadDir" check_encypted_file "data_files_unenc/data_file.c4gh" "data_files_unenc/dir1/data_file1.c4gh" @@ -178,7 +178,7 @@ else fi # Check listing datasets -output=$(./sda-cli list -config testing/s3cmd-download.conf --datasets -url http://localhost:8080) +output=$(./sda-cli -config testing/s3cmd-download.conf list --datasets -url http://localhost:8080) expected="https://doi.example/ty009.sfrrss/600.45asasga" if [[ $output == *"$expected"* ]]; then echo "Successfully listed datasets" @@ -188,7 +188,7 @@ else fi # Download whole dataset by using the sda-download feature -./sda-cli download -config testing/s3cmd-download.conf -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-dataset --dataset +./sda-cli -config testing/s3cmd-download.conf download -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-dataset --dataset filepaths="download-dataset/main/subfolder/dummy_data download-dataset/main/subfolder2/dummy_data2 download-dataset/main/subfolder2/random/dummy_data3" @@ -210,7 +210,7 @@ else echo "Failed to create a user key pair for downloading encrypted files" exit 1 fi -./sda-cli download -pubkey user_key.pub.pem -config testing/s3cmd-download.conf -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir test-download main/subfolder/dummy_data.c4gh +./sda-cli -config testing/s3cmd-download.conf download -pubkey user_key.pub.pem -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir test-download main/subfolder/dummy_data.c4gh # check if file exists in the path if [ ! -f "test-download/main/subfolder/dummy_data.c4gh" ]; then @@ -315,7 +315,7 @@ rm -r test-download # Download recursively a folder echo "Downloading content of folder" -./sda-cli download -config testing/s3cmd-download.conf -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-folder --recursive main/subfolder2 +./sda-cli -config testing/s3cmd-download.conf download -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-folder --recursive main/subfolder2 folderpaths="download-folder/main/subfolder2/dummy_data2 download-folder/main/subfolder2/random/dummy_data3" @@ -330,7 +330,7 @@ done rm -r download-folder # Download file by providing the file id -./sda-cli download -config testing/s3cmd-download.conf -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-fileid urn:neic:001-001 +./sda-cli -config testing/s3cmd-download.conf download -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-fileid urn:neic:001-001 # Check if file exists in the path if [ ! -f "download-fileid/main/subfolder/dummy_data" ]; then @@ -349,7 +349,7 @@ rm -r download-fileid # Download the file paths content of a text file echo "Downloading content of a text file" -./sda-cli download -config testing/s3cmd-download.conf -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-from-file --from-file testing/file-list.txt +./sda-cli -config testing/s3cmd-download.conf download -dataset-id https://doi.example/ty009.sfrrss/600.45asasga -url http://localhost:8080 -outdir download-from-file --from-file testing/file-list.txt # Check if the content of the text file has been downloaded content_paths="download-from-file/main/subfolder/dummy_data download-from-file/main/subfolder2/dummy_data2" diff --git a/README.md b/README.md index 709f42a..9b78c4e 100644 --- a/README.md +++ b/README.md @@ -107,13 +107,13 @@ The Priority order for the access token is: Now that the configuration file is downloaded, the file(s) can be uploaded to the archive using the binary file created in the first step of this guide. To upload a specific file, use the following command: ```bash -./sda-cli upload -config +./sda-cli -config upload ``` where `` the file downloaded in the previous step and `` a file encrypted in the earlier steps. The tool also allows for uploading multiple files at once, by listing them separated with space like: ```bash -./sda-cli upload -config +./sda-cli -config upload ``` Note that the files will be uploaded in the base folder of the user. @@ -123,7 +123,7 @@ Note that the files will be uploaded in the base folder of the user. One can also upload entire directories recursively, i.e. including all contained files and folders while keeping the local folder structure. This can be achieved with the `-r` flag, e.g. running: ```bash -./sda-cli upload -config -r +./sda-cli -config upload -r ``` will upload `` as is, i.e. with the same inner folder and file structure as the local one, to the archive. @@ -131,7 +131,7 @@ will upload `` as is, i.e. with the same inner folder and file It is also possible to specify multiple directories and files for upload with the same command. For example, ```bash -./sda-cli upload -config -r +./sda-cli -config upload -r ``` However, if `-r` is omitted in the above, any folders will be skipped during upload. @@ -141,7 +141,7 @@ However, if `-r` is omitted in the above, any folders will be skipped during upl The user can specify a different path for uploading files/folders with the `-targetDir` flag followed by the name of the folder. For example, the command: ```bash -./sda-cli upload -config -r -targetDir +./sda-cli -config upload -r -targetDir ``` will create `` under the user's base folder with contents `/` and `/`. Note that the given `` may well be a folder path, e.g. ``, and in this case `` will be uploaded to `folder1/folder2/`. @@ -149,7 +149,7 @@ will create `` under the user's base folder with contents ` -r /. -targetDir +./sda-cli -config upload -r /. -targetDir ``` will upload all contents of `` to `` recursively, effectively renaming `` upon upload to the archive. @@ -159,7 +159,7 @@ will upload all contents of `` to `` recursiv It is possible to combine the encryption and upload steps with the use of the flag `--encrypt-with-key` followed by the path of the crypt4gh public key to be used for encryption. In this case, the input list of file arguments can only contain _unencrypted_ source files. For example the following, ```bash -./sda-cli upload -config --encrypt-with-key +./sda-cli -config upload --encrypt-with-key ``` will encrypt `` using `` as public key and upload the created `` in the base folder of the user. @@ -167,7 +167,7 @@ will encrypt `` using `` as public key a Encrypt on upload can be combined with any of the flags above. For example, ```bash -./sda-cli upload -config --encrypt-with-key -r -targetDir +./sda-cli -config upload --encrypt-with-key -r -targetDir ``` will first encrypt all files in `` and then upload the folder recursively (selecting only the created `c4gh` files) under ``. @@ -190,13 +190,13 @@ All the following cases require a [configuration file to be downloaded](https:// This feature returns all the files in the user's inbox recursively and can be executed using: ```bash -./sda-cli list [-config ] +./sda-cli [-config ] list ``` It also allows for requesting files/filepaths with a specified prefix using: ```bash -./sda-cli list [-config ] +./sda-cli [-config ] list ``` This command will return any file/path starting with the defined ``. @@ -207,7 +207,7 @@ If no config is given by the user, the tool will look for a previous login from To list datasets or the files within a dataset that the user has access to, the `--datasets` flag should be used and the login URL must be provided: ```bash -./sda-cli list -config --datasets --url (--bytes) +./sda-cli -config list --datasets --url (--bytes) ``` where `` is the URL where the user goes for authentication. This command returns a list of accessible datasets, including the number @@ -216,7 +216,7 @@ of files in each dataset and the total size of it. The `--bytes` flag is optiona To list the files within a particular dataset, the user must provide the dataset ID (which can be obtained by executing the previous command): ```bash -./sda-cli list -config -dataset -url (--bytes) +./sda-cli -config list -dataset -url (--bytes) ``` This command returns a list of files within the dataset, including their file IDs, file sizes and file paths. The `--bytes` flag is optional and it will display the size of the files in bytes. @@ -241,14 +241,14 @@ For downloading files the user also needs to know the download service URL and t For downloading one specific file the user needs to provide the path or the id (the id should **NOT** have "/") of this file by running the command below: ```bash -./sda-cli download -config -dataset-id -url [ or ] +./sda-cli -config download -dataset-id -url [ or ] ``` where `` the file downloaded in the [previous step](#download-the-configuration-file), `` the ID of the dataset and `` the path of the file (or `` the id of the file) in the dataset. The tool also allows for downloading multiple files at once, by listing their filepaths (or file ids) separated with space and it also allows for selecting a folder where the files will be downloaded, using the `outdir` argument: ```bash -./sda-cli download -config -dataset-id -url -outdir ... (or ...) +./sda-cli -config download -dataset-id -url -outdir ... (or ...) ``` #### Download files recursively @@ -256,7 +256,7 @@ The tool also allows for downloading multiple files at once, by listing their fi For downloading the content of a folder (including subfolders) the user need to add the `--recursive` flag followed by the path(s) of the folder(s): ```bash -./sda-cli download -config -dataset-id -url -outdir --recursive path/to/folder1 path/to/folder2 ... +./sda-cli -config download -dataset-id -url -outdir --recursive path/to/folder1 path/to/folder2 ... ``` #### Download from file @@ -265,7 +265,7 @@ For downloading multiple files the user can provide a text file with the paths o In this case user needs to use the `--from-file` flag and at the end user needs to provide the path of the text file with the paths of the files to download: ```bash -./sda-cli download -config -dataset-id -url -outdir --from-file +./sda-cli -config download -dataset-id -url -outdir --from-file ``` #### Download all the files of the dataset @@ -273,7 +273,7 @@ In this case user needs to use the `--from-file` flag and at the end user needs For downloading the whole dataset the user needs add the `--dataset` flag and NOT providing any filepaths: ```bash -./sda-cli download -config -dataset-id -url -outdir --dataset +./sda-cli -config download -dataset-id -url -outdir --dataset ``` where the dataset will be downloaded in the `` directory be keeping the original folder structure of the dataset. @@ -283,7 +283,7 @@ where the dataset will be downloaded in the `` directory be keeping the When a [public key](#create-keys) is provided, you can download files that are encrypted on the server-side with that public key. The command is similar to downloading the unencrypted files except that a public key is provided through the `-pubkey` flag. For example: ```bash -./sda-cli download -pubkey -config -dataset-id -url -outdir ... +./sda-cli -config download -pubkey -dataset-id -url -outdir ... ``` After a successful download, the encrypted files can be [decrypted](#decrypt-file) using the private key corresponding to the provided public key. @@ -330,7 +330,7 @@ htsget -dataset -filename -reference - for example: ``` -./sda-cli htsget -config testing/s3cmd.conf -dataset DATASET0001 -filename htsnexus_test_NA12878 -reference 11 -host http://localhost:8088 -pubkey testing/c4gh.pub.pem -output ~/tmp/result.c4gh --force-overwrite +./sda-cli -config testing/s3cmd.conf htsget -dataset DATASET0001 -filename htsnexus_test_NA12878 -reference 11 -host http://localhost:8088 -pubkey testing/c4gh.pub.pem -output ~/tmp/result.c4gh --force-overwrite ``` # Developers' section diff --git a/download/download.go b/download/download.go index 14e1819..cf7c76f 100644 --- a/download/download.go +++ b/download/download.go @@ -26,7 +26,7 @@ import ( // Usage text that will be displayed as command line help text when using the // `help download` command var Usage = ` -USAGE: %s download -config -dataset-id -url (--pubkey ) (-outdir ) ([filepath(s) or fileid(s)] or --dataset or --recursive ) or --from-file +USAGE: %s -config download -dataset-id -url (--pubkey ) (-outdir ) ([filepath(s) or fileid(s)] or --dataset or --recursive ) or --from-file download: Downloads files from the Sensitive Data Archive (SDA) by using APIs from the given url. The user @@ -58,8 +58,6 @@ var ArgHelp = ` // main program help var Args = flag.NewFlagSet("download", flag.ExitOnError) -var configPath = Args.String("config", "", "S3 config file to use for downloading.") - var datasetID = Args.String("dataset-id", "", "Dataset ID for the file to download.") var URL = Args.String("url", "", "The url of the download server.") @@ -96,17 +94,14 @@ type File struct { // Download function downloads files from the SDA by using the // download's service APIs -func Download(args []string, configPathF string) error { +func Download(args []string, configPath string) error { // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } - if configPathF == "" { - configPathF = *configPath - } - if *datasetID == "" || *URL == "" || configPathF == "" { + if *datasetID == "" || *URL == "" || configPath == "" { return fmt.Errorf("missing required arguments, dataset, config and url are required") } @@ -139,7 +134,7 @@ func Download(args []string, configPathF string) error { } // Get the configuration file or the .sda-cli-session - config, err := helpers.GetAuth(configPathF) + config, err := helpers.GetAuth(configPath) if err != nil { return err } diff --git a/download/download_test.go b/download/download_test.go index 5b1e478..4cad790 100644 --- a/download/download_test.go +++ b/download/download_test.go @@ -68,15 +68,13 @@ func (suite *TestSuite) TestInvalidUrl() { "download", "-dataset-id", "TES01", - "-config", - confPath.Name(), "-url", "https://some/url", "file1", "file2", } - err := Download(os.Args, "") + err := Download(os.Args, confPath.Name()) assert.Contains( suite.T(), err.Error(), diff --git a/htsget/htsget.go b/htsget/htsget.go index 60b1a62..8191c41 100644 --- a/htsget/htsget.go +++ b/htsget/htsget.go @@ -20,7 +20,7 @@ import ( // Usage text that will be displayed as command line help text when using the // `help htsget` command var Usage = ` -USAGE: %s htsget [-dataset ] [-filename ] (-reference ) [-htsgethost ] [-pubkey ] (-output ) (--force-overwrite) +USAGE: %s -config testing/s3cmd.conf htsget [-dataset ] [-filename ] (-reference ) [-htsgethost ] [-pubkey ] (-output ) (--force-overwrite) htsget: Htsget downloads files from the Sensitive Data Archive (SDA), using the @@ -51,7 +51,6 @@ var fileName = Args.String("filename", "", "The name of the file to download") var referenceName = Args.String("reference", "", "The reference number of the file to download") var htsgetHost = Args.String("host", "", "The host to download from") var publicKeyFile = Args.String("pubkey", "", "Public key file") -var configPath = Args.String("config", "", "config file.") var outPut = Args.String("output", "", "Name for the downloaded file.") var forceOverwrite = Args.Bool("force-overwrite", false, "Force overwrite existing files.") @@ -73,22 +72,19 @@ type htsgetResponse struct { // Htsget function downloads the files included in the urls_list.txt file. // The argument can be a local file or a url to an S3 folder -func Htsget(args []string, configPathF string) error { +func Htsget(args []string, configPath string) error { // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } - if configPathF == "" { - configPathF = *configPath - } if *datasetID == "" || *fileName == "" || *htsgetHost == "" || *publicKeyFile == "" { return fmt.Errorf("missing required arguments, dataset, filename, host and key are required") } - config, err := helpers.GetAuth(configPathF) + config, err := helpers.GetAuth(configPath) if err != nil { return err } diff --git a/htsget/htsget_test.go b/htsget/htsget_test.go index e8c60b6..298ad40 100644 --- a/htsget/htsget_test.go +++ b/htsget/htsget_test.go @@ -29,8 +29,8 @@ func (suite *TestSuite) MissingArgument() { // test Htsget with mocked http request func (suite *TestSuite) TestHtsgetMissingConfig() { - os.Args = []string{"htsget", "-config", "nonexistent.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} - err := Htsget(os.Args, "") + os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} + err := Htsget(os.Args, "nonexistent.conf") msg := "no such file or directory" if runtime.GOOS == "windows" { msg = "open nonexistent.conf: The system cannot find the file specified." @@ -58,8 +58,8 @@ access_token = eyJ0eXAiOiJKV1QiLCJqa3UiOiJodHRwczovL29pZGM6ODA4MC9qd2siLCJhbGciO if err != nil { panic(err) } - os.Args = []string{"htsget", "-config", tmpDir + "s3cmd_test.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} - err = Htsget(os.Args, "") + os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} + err = Htsget(os.Args, tmpDir+"s3cmd_test.conf") assert.ErrorContains(suite.T(), err, "failed to read public key") } @@ -90,8 +90,8 @@ KKj6NUcJGZ2/HeqkYbxm57ZaFLP5cIHsdK+0nQubFVs= if err != nil { panic(err) } - os.Args = []string{"htsget", "-config", tmpDir + "s3cmd_test.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "missingserver", "-pubkey", tmpDir + "c4gh.pub.pem"} - err = Htsget(os.Args, "") + os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "missingserver", "-pubkey", tmpDir + "c4gh.pub.pem"} + err = Htsget(os.Args, tmpDir+"s3cmd_test.conf") assert.ErrorContains(suite.T(), err, "failed to do the request") } @@ -184,7 +184,7 @@ KKj6NUcJGZ2/HeqkYbxm57ZaFLP5cIHsdK+0nQubFVs= server := httptest.NewServer(mux) defer server.Close() - os.Args = []string{"htsget", "-config", tmpDir + "s3cmd_test.conf", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", server.URL, "-pubkey", tmpDir + "c4gh.pub.pem"} - err = Htsget(os.Args, "") + os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", server.URL, "-pubkey", tmpDir + "c4gh.pub.pem"} + err = Htsget(os.Args, tmpDir+"s3cmd_test.conf") assert.ErrorContains(suite.T(), err, "error downloading the files") } diff --git a/list/list.go b/list/list.go index f227595..3ae0fd2 100644 --- a/list/list.go +++ b/list/list.go @@ -18,7 +18,7 @@ import ( // Usage text that will be displayed as command line help text when using the // `help list` command var Usage = ` -USAGE: %s list [-config ] [prefix] (-url --datasets) (-url --dataset ) [-bytes] +USAGE: %s [-config ] list [prefix] (-url --datasets) (-url --dataset ) [-bytes] list: Lists recursively all files under the user's folder in the Sensitive @@ -40,9 +40,6 @@ var ArgHelp = ` // main program help var Args = flag.NewFlagSet("list", flag.ExitOnError) -var configPath = Args.String("config", "", - "S3 config file to use for listing.") - var URL = Args.String("url", "", "The url of the sda-download server") var datasets = Args.Bool("datasets", false, "List all datasets in the user's folder.") @@ -52,15 +49,12 @@ var bytesFormat = Args.Bool("bytes", false, "Print file sizes in bytes (not huma var dataset = Args.String("dataset", "", "List all files in the specified dataset.") // List function lists the contents of an s3 -func List(args []string, configPathF string) error { +func List(args []string, configPath string) error { // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } - if configPathF == "" { - configPathF = *configPath - } prefix := "" if len(Args.Args()) == 1 { @@ -68,7 +62,7 @@ func List(args []string, configPathF string) error { } // // Get the configuration file or the .sda-cli-session - config, err := helpers.GetAuth(configPathF) + config, err := helpers.GetAuth(configPath) if err != nil { return fmt.Errorf("failed to load config file, reason: %v", err) } diff --git a/list/list_test.go b/list/list_test.go index d8a0600..50833ae 100644 --- a/list/list_test.go +++ b/list/list_test.go @@ -37,7 +37,7 @@ func (suite *TestSuite) SetupTest() { func (suite *TestSuite) TestNoConfig() { - os.Args = []string{"list", "-config", ""} + os.Args = []string{"list"} err := List(os.Args, "") assert.EqualError(suite.T(), err, "failed to load config file, reason: failed to read the configuration file") @@ -126,8 +126,8 @@ func (suite *TestSuite) TestFunctionality() { log.SetOutput(&uploadOutput) // Upload a file - os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", dir} - err = upload.Upload(os.Args, "") + os.Args = []string{"upload", "--force-unencrypted", "-r", dir} + err = upload.Upload(os.Args, configPath.Name()) assert.NoError(suite.T(), err) // Check logs that file was uploaded @@ -141,8 +141,8 @@ func (suite *TestSuite) TestFunctionality() { r, w, _ := os.Pipe() os.Stdout = w - os.Args = []string{"list", "-config", configPath.Name()} - err = List(os.Args, "") + os.Args = []string{"list"} + err = List(os.Args, configPath.Name()) assert.NoError(suite.T(), err) w.Close() diff --git a/main.go b/main.go index 0a23e62..6a15261 100644 --- a/main.go +++ b/main.go @@ -20,7 +20,7 @@ import ( var Version = "development" -var Usage = `USAGE: %s [command-args] +var Usage = `USAGE: %s -config [command-args] This is a helper tool that can help with common tasks when interacting with the Sensitive Data Archive (SDA). diff --git a/upload/upload.go b/upload/upload.go index b4c5f80..ee24716 100644 --- a/upload/upload.go +++ b/upload/upload.go @@ -25,7 +25,7 @@ import ( // Usage text that will be displayed as command line help text when using the // `help upload` command var Usage = ` -USAGE: %s upload -config (-accessToken ) (--encrypt-with-key ) (--force-overwrite) (--force-unencrypted) (-r) [file(s) | folder(s)] (-targetDir ) +USAGE: %s -config upload (-accessToken ) (--encrypt-with-key ) (--force-overwrite) (--force-unencrypted) (-r) [file(s) | folder(s)] (-targetDir ) upload: Uploads files to the Sensitive Data Archive (SDA). @@ -44,9 +44,6 @@ var ArgHelp = ` // main program help var Args = flag.NewFlagSet("upload", flag.ExitOnError) -var configPath = Args.String("config", "", - "S3 config file to use for uploading.") - var forceUnencrypted = Args.Bool("force-unencrypted", false, "Force uploading unencrypted files.") var dirUpload = Args.Bool("r", false, "Upload directories recursively.") @@ -66,7 +63,7 @@ var pubKeyPath = Args.String("encrypt-with-key", "", var accessToken = Args.String("accessToken", "", "Access token to the inbox service.\n(optional, if it is set in the config file or exported as the ENV `ACCESSTOKEN`)") // Function uploadFiles uploads the files in the input list to the s3 bucket -func uploadFiles(files, outFiles []string, targetDir string, config *helpers.Config) error { +func uploadFiles(files, outFiles []string, targetDir string, config *helpers.Config, configPath string) error { // check also here in case sth went wrong with input files if len(files) == 0 { return errors.New("no files to upload") @@ -126,8 +123,8 @@ func uploadFiles(files, outFiles []string, targetDir string, config *helpers.Con // create progress bar instance p := mpb.New() - log.Infof("Uploading %s with config %s\n", filename, *configPath) - fmt.Printf("Uploading %s with config %s\n", filename, *configPath) + log.Infof("Uploading %s with config %s\n", filename, configPath) + fmt.Printf("Uploading %s with config %s\n", filename, configPath) f, err := os.Open(path.Clean(filename)) if err != nil { @@ -256,7 +253,7 @@ func createFilePaths(dirPath string) ([]string, []string, error) { // Upload function uploads files to the s3 bucket. Input can be files or // directories to be uploaded recursively -func Upload(args []string, configPathF string) error { +func Upload(args []string, configPath string) error { var files []string var outFiles []string *pubKeyPath = "" @@ -267,9 +264,6 @@ func Upload(args []string, configPathF string) error { if err != nil { return fmt.Errorf("failed parsing arguments, reason: %v", err) } - if configPathF == "" { - configPathF = *configPath - } // Dereference the pointer to a string var targetDirString string @@ -291,7 +285,7 @@ func Upload(args []string, configPathF string) error { } // Get the configuration file or the .sda-cli-session - config, err := helpers.GetAuth(configPathF) + config, err := helpers.GetAuth(configPath) if err != nil { return err } @@ -368,5 +362,5 @@ func Upload(args []string, configPathF string) error { } } - return uploadFiles(files, outFiles, filepath.ToSlash(*targetDir), config) + return uploadFiles(files, outFiles, filepath.ToSlash(*targetDir), config, configPath) } diff --git a/upload/upload_test.go b/upload/upload_test.go index 926739e..3ca24fb 100644 --- a/upload/upload_test.go +++ b/upload/upload_test.go @@ -70,17 +70,17 @@ func (suite *TestSuite) TestSampleNoFiles() { } // Test Upload function - os.Args = []string{"upload", "-config", configPath.Name()} + os.Args = []string{"upload"} - assert.EqualError(suite.T(), Upload(os.Args, ""), "no files to upload") + assert.EqualError(suite.T(), Upload(os.Args, configPath.Name()), "no files to upload") // Test handling of mistakenly passing a filename as an upload folder - os.Args = []string{"upload", "-config", configPath.Name(), "-targetDir", configPath.Name()} - assert.EqualError(suite.T(), Upload(os.Args, ""), configPath.Name()+" is not a valid target directory") + os.Args = []string{"upload", "-targetDir", configPath.Name()} + assert.EqualError(suite.T(), Upload(os.Args, configPath.Name()), configPath.Name()+" is not a valid target directory") // Test handling of mistakenly passing a flag as an upload folder - os.Args = []string{"upload", "-config", configPath.Name(), "-targetDir", "-r"} - assert.EqualError(suite.T(), Upload(os.Args, ""), "-r"+" is not a valid target directory") + os.Args = []string{"upload", "-targetDir", "-r"} + assert.EqualError(suite.T(), Upload(os.Args, configPath.Name()), "-r"+" is not a valid target directory") // Test passing flags at the end as well @@ -88,17 +88,17 @@ func (suite *TestSuite) TestSampleNoFiles() { if runtime.GOOS == "windows" { msg = "CreateFile somefileOrfolder: The system cannot find the file specified." } - os.Args = []string{"upload", "-config", configPath.Name(), "-r", "somefileOrfolder", "-targetDir", "somedir"} - assert.EqualError(suite.T(), Upload(os.Args, ""), msg) + os.Args = []string{"upload", "-r", "somefileOrfolder", "-targetDir", "somedir"} + assert.EqualError(suite.T(), Upload(os.Args, configPath.Name()), msg) - os.Args = []string{"upload", "-config", configPath.Name(), "somefiles", "-targetDir"} - assert.EqualError(suite.T(), Upload(os.Args, ""), "no files to upload") + os.Args = []string{"upload", "somefiles", "-targetDir"} + assert.EqualError(suite.T(), Upload(os.Args, configPath.Name()), "no files to upload") // Test uploadFiles function config, _ := helpers.LoadConfigFile(configPath.Name()) var files []string - err = uploadFiles(files, files, "", config) + err = uploadFiles(files, files, "", config, configPath.Name()) assert.EqualError(suite.T(), err, "no files to upload") } @@ -219,8 +219,8 @@ func (suite *TestSuite) TestFunctionality() { defer log.SetOutput(os.Stdout) // Test recursive upload - os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", dir} - assert.NoError(suite.T(), Upload(os.Args, "")) + os.Args = []string{"upload", "--force-unencrypted", "-r", dir} + assert.NoError(suite.T(), Upload(os.Args, configPath.Name())) // Check logs that file was uploaded logMsg := strings.ReplaceAll(fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")), "\\\\", "\\") @@ -242,8 +242,8 @@ func (suite *TestSuite) TestFunctionality() { // Test upload to a different folder targetPath := filepath.Join("a", "b", "c") - os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), testfile.Name(), "-targetDir", targetPath} - assert.NoError(suite.T(), Upload(os.Args, "")) + os.Args = []string{"upload", "--force-unencrypted", testfile.Name(), "-targetDir", targetPath} + assert.NoError(suite.T(), Upload(os.Args, configPath.Name())) // Check logs that file was uploaded logMsg = fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")) msg = fmt.Sprintf("file uploaded to %s/dummy/%s/%s", ts.URL, filepath.ToSlash(targetPath), filepath.Base(testfile.Name())) @@ -279,8 +279,8 @@ func (suite *TestSuite) TestFunctionality() { // Empty buffer logs str.Reset() - newArgs := []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "--encrypt-with-key", publicKey.Name(), testfile.Name(), "-targetDir", "someDir"} - assert.NoError(suite.T(), Upload(newArgs, "")) + newArgs := []string{"upload", "--force-unencrypted", "--encrypt-with-key", publicKey.Name(), testfile.Name(), "-targetDir", "someDir"} + assert.NoError(suite.T(), Upload(newArgs, configPath.Name())) // Check logs that encrypted file was uploaded logMsg = fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")) @@ -301,18 +301,18 @@ func (suite *TestSuite) TestFunctionality() { assert.NotContains(suite.T(), logMsg, msg) // Check that trying to encrypt already encrypted files returns error and aborts - newArgs = []string{"upload", "-config", configPath.Name(), "--encrypt-with-key", publicKey.Name(), dir, "-r"} - assert.EqualError(suite.T(), Upload(newArgs, ""), "aborting") + newArgs = []string{"upload", "--encrypt-with-key", publicKey.Name(), dir, "-r"} + assert.EqualError(suite.T(), Upload(newArgs, configPath.Name()), "aborting") // Check handling of passing source files as pub key // (code checks first for errors related with file args) - newArgs = []string{"upload", "-config", configPath.Name(), "--encrypt-with-key", testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs, ""), "no files to upload") + newArgs = []string{"upload", "--encrypt-with-key", testfile.Name()} + assert.EqualError(suite.T(), Upload(newArgs, configPath.Name()), "no files to upload") // If both a bad key and already encrypted file args are given, // file arg errors are captured first - newArgs = []string{"upload", "-config", configPath.Name(), "--encrypt-with-key", "somekey", testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs, ""), "aborting") + newArgs = []string{"upload", "--encrypt-with-key", "somekey", testfile.Name()} + assert.EqualError(suite.T(), Upload(newArgs, configPath.Name()), "aborting") // config file without an access_token var confFileNoToken = fmt.Sprintf(` @@ -337,25 +337,25 @@ func (suite *TestSuite) TestFunctionality() { } // Check that an access token is supplied - newArgs = []string{"upload", "-config", configPath.Name(), testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs, ""), "no access token supplied") + newArgs = []string{"upload", testfile.Name()} + assert.EqualError(suite.T(), Upload(newArgs, configPath.Name()), "no access token supplied") os.Setenv("ACCESSTOKEN", "BadToken") // Supplying an accesstoken as a ENV overrules the one in the config file - newArgs = []string{"upload", "-config", configPath.Name(), testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs, ""), "could not parse token, reason: token contains an invalid number of segments") + newArgs = []string{"upload", testfile.Name()} + assert.EqualError(suite.T(), Upload(newArgs, configPath.Name()), "could not parse token, reason: token contains an invalid number of segments") suite.SetupTest() os.Setenv("ACCESSTOKEN", suite.accessToken) - newArgs = []string{"upload", "-config", configPath.Name(), testfile.Name()} - assert.NoError(suite.T(), Upload(newArgs, "")) + newArgs = []string{"upload", testfile.Name()} + assert.NoError(suite.T(), Upload(newArgs, configPath.Name())) // Supplying an accesstoken as a parameter overrules the one in the config file - newArgs = []string{"upload", "-accessToken", "BadToken", "-config", configPath.Name(), testfile.Name()} - assert.EqualError(suite.T(), Upload(newArgs, ""), "could not parse token, reason: token contains an invalid number of segments") + newArgs = []string{"upload", "-accessToken", "BadToken", testfile.Name()} + assert.EqualError(suite.T(), Upload(newArgs, configPath.Name()), "could not parse token, reason: token contains an invalid number of segments") - newArgs = []string{"upload", "-accessToken", suite.accessToken, "-config", configPath.Name(), testfile.Name()} - assert.NoError(suite.T(), Upload(newArgs, "")) + newArgs = []string{"upload", "-accessToken", suite.accessToken, testfile.Name()} + assert.NoError(suite.T(), Upload(newArgs, configPath.Name())) // Remove hash files created by Encrypt if err := os.Remove("checksum_encrypted.md5"); err != nil { @@ -452,8 +452,8 @@ func (suite *TestSuite) TestRecursiveToDifferentTarget() { log.SetOutput(&str) // Test recursive upload to a different folder targetPath := filepath.Join("a", "b", "c") - os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", dir, "-targetDir", targetPath} - assert.NoError(suite.T(), Upload(os.Args, "")) + os.Args = []string{"upload", "--force-unencrypted", "-r", dir, "-targetDir", targetPath} + assert.NoError(suite.T(), Upload(os.Args, configPath.Name())) // Check logs that file was uploaded logMsg := fmt.Sprintf("%v", strings.TrimSuffix(str.String(), "\n")) msg := fmt.Sprintf("file uploaded to %s/dummy/%s", ts.URL, filepath.ToSlash(filepath.Join(targetPath, filepath.Base(dir), filepath.Base(testfile.Name())))) @@ -537,8 +537,8 @@ func (suite *TestSuite) TestUploadInvalidCharacters() { for _, badc := range badchars { badchar := string(badc) targetDir := "test" + badchar + "dir" - os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-targetDir", targetDir, "-r", testfile.Name()} - err = Upload(os.Args, "") + os.Args = []string{"upload", "--force-unencrypted", "-targetDir", targetDir, "-r", testfile.Name()} + err = Upload(os.Args, configPath.Name()) assert.Error(suite.T(), err) assert.Equal(suite.T(), targetDir+" is not a valid target directory", err.Error()) } @@ -563,8 +563,8 @@ func (suite *TestSuite) TestUploadInvalidCharacters() { } defer os.Remove(testfile.Name()) - os.Args = []string{"upload", "--force-unencrypted", "-config", configPath.Name(), "-r", testfile.Name()} - err = Upload(os.Args, "") + os.Args = []string{"upload", "--force-unencrypted", "-r", testfile.Name()} + err = Upload(os.Args, configPath.Name()) assert.Error(suite.T(), err) assert.Equal(suite.T(), fmt.Sprintf("filepath %v contains disallowed characters: %+v", testfilepath, badchar), err.Error()) } From 4b57b91887d9d083afba4d19fa16dc4c684cbb34 Mon Sep 17 00:00:00 2001 From: Panos Chatzopoulos Date: Wed, 23 Oct 2024 09:58:31 +0200 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Joakim Bygdell --- upload/upload.go | 6 ++---- upload/upload_test.go | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/upload/upload.go b/upload/upload.go index ee24716..dcd346d 100644 --- a/upload/upload.go +++ b/upload/upload.go @@ -63,7 +63,7 @@ var pubKeyPath = Args.String("encrypt-with-key", "", var accessToken = Args.String("accessToken", "", "Access token to the inbox service.\n(optional, if it is set in the config file or exported as the ENV `ACCESSTOKEN`)") // Function uploadFiles uploads the files in the input list to the s3 bucket -func uploadFiles(files, outFiles []string, targetDir string, config *helpers.Config, configPath string) error { +func uploadFiles(files, outFiles []string, targetDir string, config *helpers.Config) error { // check also here in case sth went wrong with input files if len(files) == 0 { return errors.New("no files to upload") @@ -123,8 +123,6 @@ func uploadFiles(files, outFiles []string, targetDir string, config *helpers.Con // create progress bar instance p := mpb.New() - log.Infof("Uploading %s with config %s\n", filename, configPath) - fmt.Printf("Uploading %s with config %s\n", filename, configPath) f, err := os.Open(path.Clean(filename)) if err != nil { @@ -362,5 +360,5 @@ func Upload(args []string, configPath string) error { } } - return uploadFiles(files, outFiles, filepath.ToSlash(*targetDir), config, configPath) + return uploadFiles(files, outFiles, filepath.ToSlash(*targetDir), config) } diff --git a/upload/upload_test.go b/upload/upload_test.go index 3ca24fb..3a6fc18 100644 --- a/upload/upload_test.go +++ b/upload/upload_test.go @@ -98,7 +98,7 @@ func (suite *TestSuite) TestSampleNoFiles() { config, _ := helpers.LoadConfigFile(configPath.Name()) var files []string - err = uploadFiles(files, files, "", config, configPath.Name()) + err = uploadFiles(files, files, "", config) assert.EqualError(suite.T(), err, "no files to upload") } From 13a028fcc70f86b8bd1a1c92b5460e0cf103f068 Mon Sep 17 00:00:00 2001 From: Panos Chatzopoulos Date: Fri, 25 Oct 2024 14:43:31 +0200 Subject: [PATCH 5/6] concat strings differently --- htsget/htsget_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/htsget/htsget_test.go b/htsget/htsget_test.go index 298ad40..32bb3bb 100644 --- a/htsget/htsget_test.go +++ b/htsget/htsget_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "runtime" "testing" @@ -54,12 +55,12 @@ multipart_chunk_size_mb = 50 use_https = False socket_timeout = 30 access_token = eyJ0eXAiOiJKV1QiLCJqa3UiOiJodHRwczovL29pZGM6ODA4MC9qd2siLCJhbGciOiJFUzI1NiIsImtpZCI6IlYxcXN1VlVINUEyaTR5TWlmSFZTQWJDTTlxMnVldkF0MUktNzZfdlNTVjQifQ.eyJzdWIiOiJyZXF1ZXN0ZXJAZGVtby5vcmciLCJhdWQiOlsiYXVkMSIsImF1ZDIiXSwiYXpwIjoiYXpwIiwic2NvcGUiOiJvcGVuaWQgZ2E0Z2hfcGFzc3BvcnRfdjEiLCJpc3MiOiJodHRwczovL29pZGM6ODA4MC8iLCJleHAiOjk5OTk5OTk5OTksImlhdCI6MTU2MTYyMTkxMywianRpIjoiNmFkN2FhNDItM2U5Yy00ODMzLWJkMTYtNzY1Y2I4MGMyMTAyIn0.shY1Af3m6cPbRQd5Rn_tjvzBiJ8zx0cbsPkV8nebGrqAekpp42kUuoTYc2GCekowZMx-7J_qt3pz5Tk6nRyIHw` - err := os.WriteFile(tmpDir+"s3cmd_test.conf", []byte(s3cmdConf), 0600) + err := os.WriteFile(filepath.Join(tmpDir, "s3cmd_test.conf"), []byte(s3cmdConf), 0600) if err != nil { panic(err) } os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "somehost", "-pubkey", "somekey"} - err = Htsget(os.Args, tmpDir+"s3cmd_test.conf") + err = Htsget(os.Args, filepath.Join(tmpDir, "s3cmd_test.conf")) assert.ErrorContains(suite.T(), err, "failed to read public key") } @@ -79,7 +80,7 @@ multipart_chunk_size_mb = 50 use_https = False socket_timeout = 30 access_token = eyJ0eXAiOiJKV1QiLCJqa3UiOiJodHRwczovL29pZGM6ODA4MC9qd2siLCJhbGciOiJFUzI1NiIsImtpZCI6IlYxcXN1VlVINUEyaTR5TWlmSFZTQWJDTTlxMnVldkF0MUktNzZfdlNTVjQifQ.eyJzdWIiOiJyZXF1ZXN0ZXJAZGVtby5vcmciLCJhdWQiOlsiYXVkMSIsImF1ZDIiXSwiYXpwIjoiYXpwIiwic2NvcGUiOiJvcGVuaWQgZ2E0Z2hfcGFzc3BvcnRfdjEiLCJpc3MiOiJodHRwczovL29pZGM6ODA4MC8iLCJleHAiOjk5OTk5OTk5OTksImlhdCI6MTU2MTYyMTkxMywianRpIjoiNmFkN2FhNDItM2U5Yy00ODMzLWJkMTYtNzY1Y2I4MGMyMTAyIn0.shY1Af3m6cPbRQd5Rn_tjvzBiJ8zx0cbsPkV8nebGrqAekpp42kUuoTYc2GCekowZMx-7J_qt3pz5Tk6nRyIHw` - err := os.WriteFile(tmpDir+"s3cmd_test.conf", []byte(s3cmdConf), 0600) + err := os.WriteFile(filepath.Join(tmpDir, "s3cmd_test.conf"), []byte(s3cmdConf), 0600) if err != nil { panic(err) } @@ -91,7 +92,7 @@ KKj6NUcJGZ2/HeqkYbxm57ZaFLP5cIHsdK+0nQubFVs= panic(err) } os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", "missingserver", "-pubkey", tmpDir + "c4gh.pub.pem"} - err = Htsget(os.Args, tmpDir+"s3cmd_test.conf") + err = Htsget(os.Args, filepath.Join(tmpDir, "s3cmd_test.conf")) assert.ErrorContains(suite.T(), err, "failed to do the request") } @@ -112,7 +113,7 @@ multipart_chunk_size_mb = 50 use_https = False socket_timeout = 30 access_token = eyJ0eXAiOiJKV1QiLCJqa3UiOiJodHRwczovL29pZGM6ODA4MC9qd2siLCJhbGciOiJFUzI1NiIsImtpZCI6IlYxcXN1VlVINUEyaTR5TWlmSFZTQWJDTTlxMnVldkF0MUktNzZfdlNTVjQifQ.eyJzdWIiOiJyZXF1ZXN0ZXJAZGVtby5vcmciLCJhdWQiOlsiYXVkMSIsImF1ZDIiXSwiYXpwIjoiYXpwIiwic2NvcGUiOiJvcGVuaWQgZ2E0Z2hfcGFzc3BvcnRfdjEiLCJpc3MiOiJodHRwczovL29pZGM6ODA4MC8iLCJleHAiOjk5OTk5OTk5OTksImlhdCI6MTU2MTYyMTkxMywianRpIjoiNmFkN2FhNDItM2U5Yy00ODMzLWJkMTYtNzY1Y2I4MGMyMTAyIn0.shY1Af3m6cPbRQd5Rn_tjvzBiJ8zx0cbsPkV8nebGrqAekpp42kUuoTYc2GCekowZMx-7J_qt3pz5Tk6nRyIHw` - err := os.WriteFile(tmpDir+"s3cmd_test.conf", []byte(s3cmdConf), 0600) + err := os.WriteFile(filepath.Join(tmpDir, "s3cmd_test.conf"), []byte(s3cmdConf), 0600) if err != nil { panic(err) } @@ -185,6 +186,6 @@ KKj6NUcJGZ2/HeqkYbxm57ZaFLP5cIHsdK+0nQubFVs= defer server.Close() os.Args = []string{"htsget", "-dataset", "DATASET0001", "-filename", "htsnexus_test_NA12878", "-host", server.URL, "-pubkey", tmpDir + "c4gh.pub.pem"} - err = Htsget(os.Args, tmpDir+"s3cmd_test.conf") + err = Htsget(os.Args, filepath.Join(tmpDir, "s3cmd_test.conf")) assert.ErrorContains(suite.T(), err, "error downloading the files") } From 3c7e86a0a130d61842b1b106f0237a2af3fafeba Mon Sep 17 00:00:00 2001 From: Panos Chatzopoulos Date: Fri, 1 Nov 2024 13:04:11 +0100 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Joakim Bygdell --- main.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/main.go b/main.go index 6a15261..9e4d8f4 100644 --- a/main.go +++ b/main.go @@ -100,11 +100,7 @@ func ParseArgs() (string, []string, string) { } return "version", os.Args, "" - } - - // If config comes before the command, we remove it from the - // arguments and set the global config path. - if len(os.Args) > 1 && (os.Args[1] == "--config" || os.Args[1] == "-config") { + case "--config", "-config": if len(os.Args) < 3 { fmt.Fprintf(os.Stderr, "Error: --config requires an argument\n") os.Exit(1)