From 554d98746a96d5a93e3449d62c81c9b7a7c84e9d Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 25 Jul 2019 17:36:51 -0400 Subject: [PATCH] Add repo-s3-port option for setting a non-standard S3 service port. If this option is set then ports appended to repo-s3-endpoint or repo-s3-host will be ignored. Setting this option explicitly may be the only way to use a bare ipv6 address with S3 (since multiple colons confuse the parser) but we plan to improve this in the future. --- build/lib/pgBackRestBuild/Config/Data.pm | 14 ++++++ doc/xml/reference.xml | 9 ++++ doc/xml/release.xml | 6 +++ lib/pgBackRest/LibCAuto.pm | 1 + src/config/config.auto.c | 9 ++++ src/config/config.auto.h | 5 ++- src/config/define.auto.c | 54 ++++++++++++++++++++++++ src/config/define.auto.h | 1 + src/config/parse.auto.c | 13 ++++++ src/perl/embed.auto.c | 1 + src/storage/helper.c | 6 ++- src/storage/s3/storage.h | 1 - test/src/module/command/helpTest.c | 1 + test/src/module/storage/s3Test.c | 33 +++++++++++++++ 14 files changed, 151 insertions(+), 3 deletions(-) diff --git a/build/lib/pgBackRestBuild/Config/Data.pm b/build/lib/pgBackRestBuild/Config/Data.pm index 49f77d850..bbdadcc82 100644 --- a/build/lib/pgBackRestBuild/Config/Data.pm +++ b/build/lib/pgBackRestBuild/Config/Data.pm @@ -287,6 +287,8 @@ use constant CFGOPT_REPO_S3_ENDPOINT => CFGDEF_RE push @EXPORT, qw(CFGOPT_REPO_S3_ENDPOINT); use constant CFGOPT_REPO_S3_HOST => CFGDEF_REPO_S3 . '-host'; push @EXPORT, qw(CFGOPT_REPO_S3_HOST); +use constant CFGOPT_REPO_S3_PORT => CFGDEF_REPO_S3 . '-port'; + push @EXPORT, qw(CFGOPT_REPO_S3_PORT); use constant CFGOPT_REPO_S3_REGION => CFGDEF_REPO_S3 . '-region'; push @EXPORT, qw(CFGOPT_REPO_S3_REGION); use constant CFGOPT_REPO_S3_TOKEN => CFGDEF_REPO_S3 . '-token'; @@ -1862,6 +1864,18 @@ my %hConfigDefine = &CFGDEF_COMMAND => CFGOPT_REPO_TYPE, }, + &CFGOPT_REPO_S3_PORT => + { + &CFGDEF_SECTION => CFGDEF_SECTION_GLOBAL, + &CFGDEF_TYPE => CFGDEF_TYPE_INTEGER, + &CFGDEF_PREFIX => CFGDEF_PREFIX_REPO, + &CFGDEF_INDEX_TOTAL => CFGDEF_INDEX_REPO, + &CFGDEF_DEFAULT => 443, + &CFGDEF_ALLOW_RANGE => [1, 65535], + &CFGDEF_DEPEND => CFGOPT_REPO_S3_BUCKET, + &CFGDEF_COMMAND => CFGOPT_REPO_TYPE, + }, + &CFGOPT_REPO_S3_REGION, { &CFGDEF_INHERIT => CFGOPT_REPO_S3_BUCKET, diff --git a/doc/xml/reference.xml b/doc/xml/reference.xml index 4f0d2714a..3e089f039 100644 --- a/doc/xml/reference.xml +++ b/doc/xml/reference.xml @@ -437,6 +437,15 @@ 127.0.0.1 + + + S3 repository port. + + Port to use when connecting to the endpoint (or host if specified). + + 9000 + + S3 repository region. diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 71eb15b47..a5b30f411 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -33,6 +33,12 @@ + + +

Add repo-s3-port option for setting a non-standard S3 service port.

+ + + diff --git a/lib/pgBackRest/LibCAuto.pm b/lib/pgBackRest/LibCAuto.pm index ca089ffda..18813f586 100644 --- a/lib/pgBackRest/LibCAuto.pm +++ b/lib/pgBackRest/LibCAuto.pm @@ -271,6 +271,7 @@ sub libcAutoExportTag 'CFGOPT_REPO_S3_HOST', 'CFGOPT_REPO_S3_KEY', 'CFGOPT_REPO_S3_KEY_SECRET', + 'CFGOPT_REPO_S3_PORT', 'CFGOPT_REPO_S3_REGION', 'CFGOPT_REPO_S3_TOKEN', 'CFGOPT_REPO_S3_VERIFY_TLS', diff --git a/src/config/config.auto.c b/src/config/config.auto.c index 0f54935f8..490f58a03 100644 --- a/src/config/config.auto.c +++ b/src/config/config.auto.c @@ -447,6 +447,7 @@ STRING_EXTERN(CFGOPT_REPO1_S3_ENDPOINT_STR, CFGOPT_REPO1 STRING_EXTERN(CFGOPT_REPO1_S3_HOST_STR, CFGOPT_REPO1_S3_HOST); STRING_EXTERN(CFGOPT_REPO1_S3_KEY_STR, CFGOPT_REPO1_S3_KEY); STRING_EXTERN(CFGOPT_REPO1_S3_KEY_SECRET_STR, CFGOPT_REPO1_S3_KEY_SECRET); +STRING_EXTERN(CFGOPT_REPO1_S3_PORT_STR, CFGOPT_REPO1_S3_PORT); STRING_EXTERN(CFGOPT_REPO1_S3_REGION_STR, CFGOPT_REPO1_S3_REGION); STRING_EXTERN(CFGOPT_REPO1_S3_TOKEN_STR, CFGOPT_REPO1_S3_TOKEN); STRING_EXTERN(CFGOPT_REPO1_S3_VERIFY_TLS_STR, CFGOPT_REPO1_S3_VERIFY_TLS); @@ -1634,6 +1635,14 @@ static ConfigOptionData configOptionData[CFG_OPTION_TOTAL] = CONFIG_OPTION_LIST CONFIG_OPTION_DEFINE_ID(cfgDefOptRepoS3KeySecret) ) + //------------------------------------------------------------------------------------------------------------------------------ + CONFIG_OPTION + ( + CONFIG_OPTION_NAME(CFGOPT_REPO1_S3_PORT) + CONFIG_OPTION_INDEX(0) + CONFIG_OPTION_DEFINE_ID(cfgDefOptRepoS3Port) + ) + //------------------------------------------------------------------------------------------------------------------------------ CONFIG_OPTION ( diff --git a/src/config/config.auto.h b/src/config/config.auto.h index 4a313151b..e975d1a32 100644 --- a/src/config/config.auto.h +++ b/src/config/config.auto.h @@ -343,6 +343,8 @@ Option constants STRING_DECLARE(CFGOPT_REPO1_S3_KEY_STR); #define CFGOPT_REPO1_S3_KEY_SECRET "repo1-s3-key-secret" STRING_DECLARE(CFGOPT_REPO1_S3_KEY_SECRET_STR); +#define CFGOPT_REPO1_S3_PORT "repo1-s3-port" + STRING_DECLARE(CFGOPT_REPO1_S3_PORT_STR); #define CFGOPT_REPO1_S3_REGION "repo1-s3-region" STRING_DECLARE(CFGOPT_REPO1_S3_REGION_STR); #define CFGOPT_REPO1_S3_TOKEN "repo1-s3-token" @@ -386,7 +388,7 @@ Option constants #define CFGOPT_TYPE "type" STRING_DECLARE(CFGOPT_TYPE_STR); -#define CFG_OPTION_TOTAL 166 +#define CFG_OPTION_TOTAL 167 /*********************************************************************************************************************************** Command enum @@ -565,6 +567,7 @@ typedef enum cfgOptRepoS3Host, cfgOptRepoS3Key, cfgOptRepoS3KeySecret, + cfgOptRepoS3Port, cfgOptRepoS3Region, cfgOptRepoS3Token, cfgOptRepoS3VerifyTls, diff --git a/src/config/define.auto.c b/src/config/define.auto.c index 8b208cdc4..43deb17d5 100644 --- a/src/config/define.auto.c +++ b/src/config/define.auto.c @@ -3720,6 +3720,60 @@ static ConfigDefineOptionData configDefineOptionData[] = CFGDEFDATA_OPTION_LIST ) ) + // ----------------------------------------------------------------------------------------------------------------------------- + CFGDEFDATA_OPTION + ( + CFGDEFDATA_OPTION_NAME("repo-s3-port") + CFGDEFDATA_OPTION_REQUIRED(true) + CFGDEFDATA_OPTION_SECTION(cfgDefSectionGlobal) + CFGDEFDATA_OPTION_TYPE(cfgDefOptTypeInteger) + CFGDEFDATA_OPTION_INTERNAL(false) + + CFGDEFDATA_OPTION_INDEX_TOTAL(1) + CFGDEFDATA_OPTION_SECURE(false) + + CFGDEFDATA_OPTION_HELP_SECTION("repository") + CFGDEFDATA_OPTION_HELP_SUMMARY("S3 repository port.") + CFGDEFDATA_OPTION_HELP_DESCRIPTION + ( + "Port to use when connecting to the endpoint (or host if specified)." + ) + + CFGDEFDATA_OPTION_COMMAND_LIST + ( + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchiveGet) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchiveGetAsync) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchivePush) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchivePushAsync) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdBackup) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdCheck) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdExpire) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdInfo) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdLocal) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdLs) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdRemote) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdRestore) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdStanzaCreate) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdStanzaDelete) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdStanzaUpgrade) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdStart) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdStop) + ) + + CFGDEFDATA_OPTION_OPTIONAL_LIST + ( + CFGDEFDATA_OPTION_OPTIONAL_ALLOW_RANGE(1, 65535) + CFGDEFDATA_OPTION_OPTIONAL_DEPEND_LIST + ( + cfgDefOptRepoType, + "s3" + ) + + CFGDEFDATA_OPTION_OPTIONAL_DEFAULT("443") + CFGDEFDATA_OPTION_OPTIONAL_PREFIX("repo") + ) + ) + // ----------------------------------------------------------------------------------------------------------------------------- CFGDEFDATA_OPTION ( diff --git a/src/config/define.auto.h b/src/config/define.auto.h index e6d99abb5..24ede6967 100644 --- a/src/config/define.auto.h +++ b/src/config/define.auto.h @@ -127,6 +127,7 @@ typedef enum cfgDefOptRepoS3Host, cfgDefOptRepoS3Key, cfgDefOptRepoS3KeySecret, + cfgDefOptRepoS3Port, cfgDefOptRepoS3Region, cfgDefOptRepoS3Token, cfgDefOptRepoS3VerifyTls, diff --git a/src/config/parse.auto.c b/src/config/parse.auto.c index e6dee740d..185c69c93 100644 --- a/src/config/parse.auto.c +++ b/src/config/parse.auto.c @@ -2008,6 +2008,18 @@ static const struct option optionList[] = .val = PARSE_OPTION_FLAG | PARSE_DEPRECATE_FLAG | cfgOptRepoS3KeySecret, }, + // repo-s3-port option + // ----------------------------------------------------------------------------------------------------------------------------- + { + .name = CFGOPT_REPO1_S3_PORT, + .has_arg = required_argument, + .val = PARSE_OPTION_FLAG | cfgOptRepoS3Port, + }, + { + .name = "reset-" CFGOPT_REPO1_S3_PORT, + .val = PARSE_OPTION_FLAG | PARSE_RESET_FLAG | cfgOptRepoS3Port, + }, + // repo-s3-region option and deprecations // ----------------------------------------------------------------------------------------------------------------------------- { @@ -2421,6 +2433,7 @@ static const ConfigOption optionResolveOrder[] = cfgOptRepoS3Host, cfgOptRepoS3Key, cfgOptRepoS3KeySecret, + cfgOptRepoS3Port, cfgOptRepoS3Region, cfgOptRepoS3Token, cfgOptRepoS3VerifyTls, diff --git a/src/perl/embed.auto.c b/src/perl/embed.auto.c index 69b19610e..7cde4dbc7 100644 --- a/src/perl/embed.auto.c +++ b/src/perl/embed.auto.c @@ -8226,6 +8226,7 @@ static const EmbeddedModule embeddedModule[] = "'CFGOPT_REPO_S3_HOST',\n" "'CFGOPT_REPO_S3_KEY',\n" "'CFGOPT_REPO_S3_KEY_SECRET',\n" + "'CFGOPT_REPO_S3_PORT',\n" "'CFGOPT_REPO_S3_REGION',\n" "'CFGOPT_REPO_S3_TOKEN',\n" "'CFGOPT_REPO_S3_VERIFY_TLS',\n" diff --git a/src/storage/helper.c b/src/storage/helper.c index a4173e2df..308e653a2 100644 --- a/src/storage/helper.c +++ b/src/storage/helper.c @@ -322,12 +322,16 @@ storageRepoGet(const String *type, bool write) else if (strEqZ(type, STORAGE_TYPE_S3)) { // Set the default port - unsigned int port = STORAGE_S3_PORT_DEFAULT; + unsigned int port = cfgOptionUInt(cfgOptRepoS3Port); // Extract port from the endpoint and host if it is present const String *endPoint = cfgOptionHostPort(cfgOptRepoS3Endpoint, &port); const String *host = cfgOptionHostPort(cfgOptRepoS3Host, &port); + // If the port option was set explicitly then use it in preference to appended ports + if (cfgOptionSource(cfgOptRepoS3Port) != cfgSourceDefault) + port = cfgOptionUInt(cfgOptRepoS3Port); + result = storageS3New( cfgOptionStr(cfgOptRepoPath), write, storageRepoPathExpression, cfgOptionStr(cfgOptRepoS3Bucket), endPoint, cfgOptionStr(cfgOptRepoS3Region), cfgOptionStr(cfgOptRepoS3Key), cfgOptionStr(cfgOptRepoS3KeySecret), diff --git a/src/storage/s3/storage.h b/src/storage/s3/storage.h index 419dcb1a3..f7f2b570c 100644 --- a/src/storage/s3/storage.h +++ b/src/storage/s3/storage.h @@ -15,7 +15,6 @@ Storage type /*********************************************************************************************************************************** Defaults ***********************************************************************************************************************************/ -#define STORAGE_S3_PORT_DEFAULT 443 #define STORAGE_S3_TIMEOUT_DEFAULT 60000 #define STORAGE_S3_PARTSIZE_MIN ((size_t)5 * 1024 * 1024) #define STORAGE_S3_DELETE_MAX 1000 diff --git a/test/src/module/command/helpTest.c b/test/src/module/command/helpTest.c index c2690036f..cdfa7d721 100644 --- a/test/src/module/command/helpTest.c +++ b/test/src/module/command/helpTest.c @@ -208,6 +208,7 @@ testRun(void) " --repo-s3-host s3 repository host\n" " --repo-s3-key s3 repository access key\n" " --repo-s3-key-secret s3 repository secret access key\n" + " --repo-s3-port s3 repository port [default=443]\n" " --repo-s3-region s3 repository region\n" " --repo-s3-token s3 repository security token\n" " --repo-s3-verify-tls verify S3 server certificate [default=y]\n" diff --git a/test/src/module/storage/s3Test.c b/test/src/module/storage/s3Test.c index 103e5e7be..57388b08d 100644 --- a/test/src/module/storage/s3Test.c +++ b/test/src/module/storage/s3Test.c @@ -599,6 +599,39 @@ testRun(void) strPtr(((StorageS3 *)storage->driver)->secretAccessKey), strPtr(secretAccessKey), " check secret access key"); TEST_RESULT_STR( strPtr(((StorageS3 *)storage->driver)->securityToken), strPtr(securityToken), " check security token"); + + // Use the port option to override both + // ------------------------------------------------------------------------------------------------------------------------- + argList = strLstNew(); + strLstAddZ(argList, "pgbackrest"); + strLstAddZ(argList, "--stanza=db"); + strLstAddZ(argList, "--repo1-type=s3"); + strLstAdd(argList, strNewFmt("--repo1-path=%s", strPtr(path))); + strLstAdd(argList, strNewFmt("--repo1-s3-bucket=%s", strPtr(bucket))); + strLstAdd(argList, strNewFmt("--repo1-s3-region=%s", strPtr(region))); + strLstAdd(argList, strNewFmt("--repo1-s3-endpoint=%s:999", strPtr(endPoint))); + strLstAdd(argList, strNewFmt("--repo1-s3-host=%s:7777", strPtr(host))); + strLstAddZ(argList, "--repo1-s3-port=9001"); + strLstAddZ(argList, "--repo1-s3-ca-path=" TLS_CERT_FAKE_PATH); + strLstAddZ(argList, "--repo1-s3-ca-file=" TLS_CERT_FAKE_PATH "/pgbackrest-test.crt"); + setenv("PGBACKREST_REPO1_S3_KEY", strPtr(accessKey), true); + setenv("PGBACKREST_REPO1_S3_KEY_SECRET", strPtr(secretAccessKey), true); + setenv("PGBACKREST_REPO1_S3_TOKEN", strPtr(securityToken), true); + strLstAddZ(argList, "archive-get"); + harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); + + TEST_ASSIGN(storage, storageRepoGet(strNew(STORAGE_TYPE_S3), false), "get S3 repo storage with options"); + TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->bucket), strPtr(bucket), " check bucket"); + TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->region), strPtr(region), " check region"); + TEST_RESULT_STR( + strPtr(((StorageS3 *)storage->driver)->bucketEndpoint), strPtr(strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint))), + " check host"); + TEST_RESULT_UINT(((StorageS3 *)storage->driver)->port, 9001, " check port"); + TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->accessKey), strPtr(accessKey), " check access key"); + TEST_RESULT_STR( + strPtr(((StorageS3 *)storage->driver)->secretAccessKey), strPtr(secretAccessKey), " check secret access key"); + TEST_RESULT_STR( + strPtr(((StorageS3 *)storage->driver)->securityToken), strPtr(securityToken), " check security token"); } // *****************************************************************************************************************************