From b3fa178a9d740c0c8310e757b94640a04e471237 Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Tue, 24 Nov 2020 20:47:25 +0800 Subject: [PATCH 1/5] handle special character in URL --- pkg/storage/parse.go | 1 + tests/br_s3/run.sh | 118 +++++++++++++++++++++++-------------------- 2 files changed, 64 insertions(+), 55 deletions(-) diff --git a/pkg/storage/parse.go b/pkg/storage/parse.go index 5a7a09601..1687c8475 100644 --- a/pkg/storage/parse.go +++ b/pkg/storage/parse.go @@ -29,6 +29,7 @@ func ParseBackend(rawURL string, options *BackendOptions) (*backup.StorageBacken return nil, errors.Annotate(berrors.ErrStorageInvalidConfig, "empty store is not allowed") } + rawURL = strings.ReplaceAll(rawURL, "+", "%2B") u, err := url.Parse(rawURL) if err != nil { return nil, errors.Trace(err) diff --git a/tests/br_s3/run.sh b/tests/br_s3/run.sh index 8b2cdace0..9af4993d3 100755 --- a/tests/br_s3/run.sh +++ b/tests/br_s3/run.sh @@ -19,8 +19,8 @@ TABLE="usertable" DB_COUNT=3 # start the s3 server -export MINIO_ACCESS_KEY=brs3accesskey -export MINIO_SECRET_KEY=brs3secretkey +export MINIO_ACCESS_KEY='KEXI7MANNASOPDLAOIEF' +export MINIO_SECRET_KEY='MaKYxEGDInMPtEYECXRJLU+FPNKb/wAX/MElir7E' export MINIO_BROWSER=off export AWS_ACCESS_KEY_ID=$MINIO_ACCESS_KEY export AWS_SECRET_ACCESS_KEY=$MINIO_SECRET_KEY @@ -51,66 +51,74 @@ for i in $(seq $DB_COUNT); do run_sql "CREATE DATABASE $DB${i};" go-ycsb load mysql -P tests/$TEST_NAME/workload -p mysql.host=$TIDB_IP -p mysql.port=$TIDB_PORT -p mysql.user=root -p mysql.db=$DB${i} done +S3_KEY="" +for p in $(seq 2); do -for i in $(seq $DB_COUNT); do - row_count_ori[${i}]=$(run_sql "SELECT COUNT(*) FROM $DB${i}.$TABLE;" | awk '/COUNT/{print $2}') -done + for i in $(seq $DB_COUNT); do + row_count_ori[${i}]=$(run_sql "SELECT COUNT(*) FROM $DB${i}.$TABLE;" | awk '/COUNT/{print $2}') + done -# backup full -echo "backup start..." -BACKUP_LOG="backup.log" -rm -f $BACKUP_LOG -unset BR_LOG_TO_TERM -run_br --pd $PD_ADDR backup full -s "s3://mybucket/$DB?endpoint=http://$S3_ENDPOINT" \ - --log-file $BACKUP_LOG || \ - ( cat $BACKUP_LOG && BR_LOG_TO_TERM=1 && exit 1 ) -cat $BACKUP_LOG -BR_LOG_TO_TERM=1 - -if grep -i $MINIO_SECRET_KEY $BACKUP_LOG; then - echo "Secret key logged in log. Please remove them." - exit 1 -fi + # backup full + echo "backup start..." + BACKUP_LOG="backup.log" + rm -f $BACKUP_LOG + unset BR_LOG_TO_TERM + run_br --pd $PD_ADDR backup full -s "s3://mybucket/$DB?endpoint=http://$S3_ENDPOINT$S3_KEY" \ + --log-file $BACKUP_LOG || \ + ( cat $BACKUP_LOG && BR_LOG_TO_TERM=1 && exit 1 ) + cat $BACKUP_LOG + BR_LOG_TO_TERM=1 -for i in $(seq $DB_COUNT); do - run_sql "DROP DATABASE $DB${i};" -done + if grep -i $MINIO_SECRET_KEY $BACKUP_LOG; then + echo "Secret key logged in log. Please remove them." + exit 1 + fi -# restore full -echo "restore start..." -RESTORE_LOG="restore.log" -rm -f $RESTORE_LOG -unset BR_LOG_TO_TERM -run_br restore full -s "s3://mybucket/$DB" --pd $PD_ADDR --s3.endpoint="http://$S3_ENDPOINT" \ - --log-file $RESTORE_LOG || \ - ( cat $RESTORE_LOG && BR_LOG_TO_TERM=1 && exit 1 ) -cat $RESTORE_LOG -BR_LOG_TO_TERM=1 - -if grep -i $MINIO_SECRET_KEY $RESTORE_LOG; then - echo "Secret key logged in log. Please remove them." - exit 1 -fi + for i in $(seq $DB_COUNT); do + run_sql "DROP DATABASE $DB${i};" + done -for i in $(seq $DB_COUNT); do - row_count_new[${i}]=$(run_sql "SELECT COUNT(*) FROM $DB${i}.$TABLE;" | awk '/COUNT/{print $2}') -done + # restore full + echo "restore start..." + RESTORE_LOG="restore.log" + rm -f $RESTORE_LOG + unset BR_LOG_TO_TERM + run_br restore full -s "s3://mybucket/$DB$S3_KEY" --pd $PD_ADDR --s3.endpoint="http://$S3_ENDPOINT" \ + --log-file $RESTORE_LOG || \ + ( cat $RESTORE_LOG && BR_LOG_TO_TERM=1 && exit 1 ) + cat $RESTORE_LOG + BR_LOG_TO_TERM=1 -fail=false -for i in $(seq $DB_COUNT); do - if [ "${row_count_ori[i]}" != "${row_count_new[i]}" ];then - fail=true - echo "TEST: [$TEST_NAME] fail on database $DB${i}" - fi - echo "database $DB${i} [original] row count: ${row_count_ori[i]}, [after br] row count: ${row_count_new[i]}" -done + if grep -i $MINIO_SECRET_KEY $RESTORE_LOG; then + echo "Secret key logged in log. Please remove them." + exit 1 + fi + + for i in $(seq $DB_COUNT); do + row_count_new[${i}]=$(run_sql "SELECT COUNT(*) FROM $DB${i}.$TABLE;" | awk '/COUNT/{print $2}') + done -if $fail; then - echo "TEST: [$TEST_NAME] failed!" - exit 1 -else - echo "TEST: [$TEST_NAME] successed!" -fi + fail=false + for i in $(seq $DB_COUNT); do + if [ "${row_count_ori[i]}" != "${row_count_new[i]}" ];then + fail=true + echo "TEST: [$TEST_NAME] fail on database $DB${i}" + fi + echo "database $DB${i} [original] row count: ${row_count_ori[i]}, [after br] row count: ${row_count_new[i]}" + done + + if $fail; then + echo "TEST: [$TEST_NAME] failed!" + exit 1 + else + echo "TEST: [$TEST_NAME] successed!" + fi + + # prepare for next test + S3_KEY="&access-key=$MINIO_ACCESS_KEY&secret-access-key=$MINIO_SECRET_KEY" + export AWS_ACCESS_KEY_ID="" + export AWS_SECRET_ACCESS_KEY="" +done for i in $(seq $DB_COUNT); do run_sql "DROP DATABASE $DB${i};" From ef66053743100edd02277f1e3f9da00df6532d54 Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Tue, 24 Nov 2020 09:11:11 -0600 Subject: [PATCH 2/5] Update tests/br_s3/run.sh Co-authored-by: kennytm --- tests/br_s3/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/br_s3/run.sh b/tests/br_s3/run.sh index 9af4993d3..74b8a2d8c 100755 --- a/tests/br_s3/run.sh +++ b/tests/br_s3/run.sh @@ -83,7 +83,7 @@ for p in $(seq 2); do RESTORE_LOG="restore.log" rm -f $RESTORE_LOG unset BR_LOG_TO_TERM - run_br restore full -s "s3://mybucket/$DB$S3_KEY" --pd $PD_ADDR --s3.endpoint="http://$S3_ENDPOINT" \ + run_br restore full -s "s3://mybucket/$DB?$S3_KEY" --pd $PD_ADDR --s3.endpoint="http://$S3_ENDPOINT" \ --log-file $RESTORE_LOG || \ ( cat $RESTORE_LOG && BR_LOG_TO_TERM=1 && exit 1 ) cat $RESTORE_LOG From 91eb39d41d0829d582f6fb5f73e0de14b4c64c0a Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Wed, 25 Nov 2020 11:37:03 +0800 Subject: [PATCH 3/5] clear data --- tests/br_s3/run.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/br_s3/run.sh b/tests/br_s3/run.sh index 74b8a2d8c..02cc032c1 100755 --- a/tests/br_s3/run.sh +++ b/tests/br_s3/run.sh @@ -118,6 +118,8 @@ for p in $(seq 2); do S3_KEY="&access-key=$MINIO_ACCESS_KEY&secret-access-key=$MINIO_SECRET_KEY" export AWS_ACCESS_KEY_ID="" export AWS_SECRET_ACCESS_KEY="" + rm -rf "$TEST_DIR/$DB" + mkdir -p "$TEST_DIR/$DB" done for i in $(seq $DB_COUNT); do From 84ddaac1331a14f7772f6ae778bba8083b886502 Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Wed, 25 Nov 2020 11:47:28 +0800 Subject: [PATCH 4/5] try to fix --- tests/br_s3/run.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/br_s3/run.sh b/tests/br_s3/run.sh index 02cc032c1..e2ed29cc6 100755 --- a/tests/br_s3/run.sh +++ b/tests/br_s3/run.sh @@ -44,8 +44,6 @@ stop_minio() { } trap stop_minio EXIT -s3cmd --access_key=$MINIO_ACCESS_KEY --secret_key=$MINIO_SECRET_KEY --host=$S3_ENDPOINT --host-bucket=$S3_ENDPOINT --no-ssl mb s3://mybucket - # Fill in the database for i in $(seq $DB_COUNT); do run_sql "CREATE DATABASE $DB${i};" @@ -53,6 +51,7 @@ for i in $(seq $DB_COUNT); do done S3_KEY="" for p in $(seq 2); do + s3cmd --access_key=$MINIO_ACCESS_KEY --secret_key=$MINIO_SECRET_KEY --host=$S3_ENDPOINT --host-bucket=$S3_ENDPOINT --no-ssl mb s3://mybucket for i in $(seq $DB_COUNT); do row_count_ori[${i}]=$(run_sql "SELECT COUNT(*) FROM $DB${i}.$TABLE;" | awk '/COUNT/{print $2}') From 0b102e590b318ef59a13ddfc292cf65e7cb2aaae Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Wed, 25 Nov 2020 14:37:45 +0800 Subject: [PATCH 5/5] add comment and a unit test --- pkg/storage/parse.go | 3 +++ pkg/storage/parse_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/pkg/storage/parse.go b/pkg/storage/parse.go index 1687c8475..06fa6713c 100644 --- a/pkg/storage/parse.go +++ b/pkg/storage/parse.go @@ -29,6 +29,9 @@ func ParseBackend(rawURL string, options *BackendOptions) (*backup.StorageBacken return nil, errors.Annotate(berrors.ErrStorageInvalidConfig, "empty store is not allowed") } + // https://github.com/pingcap/br/issues/603 + // In aws the secret key may contain '/+=' and '+' has a special meaning in URL. + // Replace "+" by "%2B" here to avoid this problem. rawURL = strings.ReplaceAll(rawURL, "+", "%2B") u, err := url.Parse(rawURL) if err != nil { diff --git a/pkg/storage/parse_test.go b/pkg/storage/parse_test.go index 3e13d4920..0b08656c0 100644 --- a/pkg/storage/parse_test.go +++ b/pkg/storage/parse_test.go @@ -66,6 +66,16 @@ func (r *testStorageSuite) TestCreateStorage(c *C) { c.Assert(s3.Sse, Equals, "aws:kms") c.Assert(s3.SseKmsKeyId, Equals, "TestKey") + // special character in access keys + s, err = ParseBackend(`s3://bucket4/prefix/path?access-key=NXN7IPIOSAAKDEEOLMAF&secret-access-key=nREY/7Dt+PaIbYKrKlEEMMF/ExCiJEX=XMLPUANw`, nil) + c.Assert(err, IsNil) + s3 = s.GetS3() + c.Assert(s3, NotNil) + c.Assert(s3.Bucket, Equals, "bucket4") + c.Assert(s3.Prefix, Equals, "prefix/path") + c.Assert(s3.AccessKey, Equals, "NXN7IPIOSAAKDEEOLMAF") + c.Assert(s3.SecretAccessKey, Equals, "nREY/7Dt+PaIbYKrKlEEMMF/ExCiJEX=XMLPUANw") + gcsOpt := &BackendOptions{ GCS: GCSBackendOptions{ Endpoint: "https://gcs.example.com/",