PG-1443 Make pg_tde_change_key_provider CLI follow conventions

The pg_tde_change_key_provider tool should act more like PostgreSQL's
own CLI tools, which includes changing the usage slightly (but not
entirely) to match, making the error messages more similar to
PostgreSQL's and making the code a bit more PG-like.
pull/238/head
Andreas Karlsson 2 months ago committed by Andreas Karlsson
parent 9ea8dd6ddb
commit 2805fa898b
  1. 169
      contrib/pg_tde/src/pg_tde_change_key_provider.c
  2. 10
      contrib/pg_tde/t/pg_tde_change_key_provider.pl

@ -5,37 +5,29 @@
#include "common/controldata_utils.h"
#include "common/logging.h"
#include "pg_getopt.h"
#include "catalog/tde_global_space.h"
#include "catalog/tde_keyring.h"
#include "common/pg_tde_utils.h"
#include "pg_tde.h"
/* version string we expect back from pg_tde_change_key_provider */
#define PROGNAME "pg_tde_change_key_provider (PostgreSQL) " PG_VERSION "\n"
static const char *progname;
static void
help(void)
usage(void)
{
puts("pg_tde_change_key_provider changes the configuration of a pg_tde key provider");
puts("");
puts("Usage:");
puts("");
puts("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> <new_provider_type> <provider_parameters...>");
puts("");
puts(" Where <new_provider_type> can be file, vault-v2 or kmip");
puts("");
puts("Depending on the provider type, the complete parameter list is:");
puts("");
puts("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> file <filename>");
puts("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> vault-v2 <url> <mount_path> <token_path> [<ca_path>]");
puts("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> kmip <host> <port> <cert_path> <key_path> [<ca_path>]");
puts("");
printf("Use dbOid %d for global key providers.\n", GLOBAL_DATA_TDE_OID);
puts("");
puts("WARNING:");
puts("");
puts("This tool only changes the values, without properly XLogging the changes, or validating that keys can be fetched using them. Only use it in case the database is inaccessible and can't be started.\n");
printf(_("%s changes the configuration of a pg_tde key provider\n\n"), progname);
printf(_("Usage:\n"));
printf(_(" %s [-D <datadir>] <dbOid> <provider_name> <new_provider_type> <provider_parameters...>\n\n"), progname);
printf(_(" Where <new_provider_type> can be file, vault-v2 or kmip\n\n"));
printf(_("Depending on the provider type, the complete parameter list is:\n\n"));
printf(_("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> file <filename>\n"));
printf(_("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> vault-v2 <url> <mount_path> <token_path> [<ca_path>]\n"));
printf(_("pg_tde_change_key_provider [-D <datadir>] <dbOid> <provider_name> kmip <host> <port> <cert_path> <key_path> [<ca_path>]\n"));
printf(_("\nUse dbOid %d for global key providers.\n\n"), GLOBAL_DATA_TDE_OID);
printf(_("WARNING:\n"));
printf(_(" This tool only changes the values, without properly XLogging the changes, or validating that keys can be fetched using them. Only use it in case the database is inaccessible and can't be started.\n"));
}
#define BUFFER_SIZE 1024
@ -84,7 +76,7 @@ build_json(char *buffer, int count,...)
}
if (ptr - buffer > BUFFER_SIZE)
{
fprintf(stderr, "Error: Configuration too long.\n");
pg_log_error("configuration too long");
return false;
}
}
@ -94,7 +86,7 @@ build_json(char *buffer, int count,...)
if (ptr - buffer > BUFFER_SIZE)
{
fprintf(stderr, "Error: Configuration too long.\n");
pg_log_error("configuration too long");
return false;
}
@ -104,31 +96,33 @@ build_json(char *buffer, int count,...)
int
main(int argc, char *argv[])
{
static struct option long_options[] = {
{"pgdata", required_argument, NULL, 'D'},
{NULL, 0, NULL, 0}
};
int c;
int option_index;
char *datadir = NULL;
Oid db_oid;
char *provider_name;
char *new_provider_type;
char *datadir = getenv("PGDATA");
int argstart = 0;
char json[BUFFER_SIZE * 2] = {0,};
char json[BUFFER_SIZE * 2] = {0};
ControlFileData *controlfile;
bool crc_ok;
char tdedir[MAXPGPATH] = {0,};
char tdedir[MAXPGPATH] = {0};
char *cptr = tdedir;
bool provider_found = false;
KeyringProviderRecordInFile record;
Oid db_oid;
pg_logging_init(argv[0]);
pg_logging_set_level(PG_LOG_WARNING);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_tde_change_key_provider"));
progname = get_progname(argv[0]);
if (argc > 1)
{
if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
{
help();
usage();
exit(0);
}
if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
@ -138,101 +132,99 @@ main(int argc, char *argv[])
}
}
if (argc > 3)
while ((c = getopt_long(argc, argv, "D:", long_options, &option_index)) != -1)
{
if (strcmp(argv[1], "-D") == 0)
switch (c)
{
datadir = argv[2];
case 'D':
datadir = optarg;
break;
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
argstart += 2;
}
if (datadir == NULL || strlen(datadir) == 0)
if (datadir == NULL)
{
help();
puts("\n");
fprintf(stderr, "Error: Data directory missing.\n");
exit(1);
datadir = getenv("PGDATA");
/* If no datadir was specified, and none could be found, error out */
if (datadir == NULL)
{
pg_log_error("no data directory specified");
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
}
if (argc - argstart <= 3)
if (argc - optind < 3)
{
help();
pg_log_error("too few arguments");
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
db_oid = atoi(argv[1 + argstart]);
provider_name = argv[2 + argstart];
new_provider_type = argv[3 + argstart];
db_oid = atoi(argv[optind++]);
provider_name = argv[optind++];
new_provider_type = argv[optind++];
if (strcmp("file", new_provider_type) == 0)
{
provider_found = true;
if (argc - argstart != 5)
if (argc - optind != 1)
{
help();
puts("\n");
fprintf(stderr, "Error: wrong number of arguments.\n");
pg_log_error("wrong number of arguments for \"%s\"", new_provider_type);
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
if (!build_json(json, 1, "path", argv[4 + argstart]))
if (!build_json(json, 1, "path", argv[optind]))
{
exit(1);
}
}
if (strcmp("vault-v2", new_provider_type) == 0)
else if (strcmp("vault-v2", new_provider_type) == 0)
{
provider_found = true;
if (argc - argstart != 7 && argc - argstart != 8)
if (argc - optind != 3 && argc - optind != 4)
{
help();
puts("\n");
fprintf(stderr, "Error: wrong number of arguments.\n");
pg_log_error("wrong number of arguments for \"%s\"", new_provider_type);
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
if (!build_json(json, 4,
"url", argv[4 + argstart],
"mountPath", argv[5 + argstart],
"tokenPath", argv[6 + argstart],
"caPath", (argc - argstart > 7 ? argv[7 + argstart] : "")))
"url", argv[optind],
"mountPath", argv[optind + 1],
"tokenPath", argv[optind + 2],
"caPath", (argc - optind > 3 ? argv[optind + 3] : "")))
{
exit(1);
}
}
if (strcmp("kmip", new_provider_type) == 0)
else if (strcmp("kmip", new_provider_type) == 0)
{
provider_found = true;
if (argc - argstart != 8 && argc - argstart != 9)
if (argc - optind != 4 && argc - optind != 5)
{
help();
puts("\n");
fprintf(stderr, "Error: wrong number of arguments.\n");
pg_log_error("wrong number of arguments for \"%s\"", new_provider_type);
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
if (!build_json(json, 5,
"host", argv[4 + argstart],
"port", argv[5 + argstart],
"certPath", argv[6 + argstart],
"keyPath", argv[7 + argstart],
"caPath", argc - argstart > 8 ? argv[8 + argstart] : ""))
"host", argv[optind],
"port", argv[optind + 1],
"certPath", argv[optind + 2],
"keyPath", argv[optind + 3],
"caPath", argc - optind > 4 ? argv[optind + 4] : ""))
{
exit(1);
}
}
if (!provider_found)
else
{
help();
puts("\n");
fprintf(stderr, "Error: Unknown provider type: %s\n.", new_provider_type);
pg_log_error("unknown provider type \"%s\"", new_provider_type);
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}
@ -256,10 +248,7 @@ main(int argc, char *argv[])
pg_tde_set_data_dir(tdedir);
if (get_keyring_info_file_record_by_name(provider_name, db_oid, &record) == false)
{
fprintf(stderr, "Error: provider not found\n.");
exit(1);
}
pg_fatal("provder \"%s\" not found for database %u", provider_name, db_oid);
record.provider.provider_type = get_keyring_provider_from_typename(new_provider_type);
memset(record.provider.options, 0, sizeof(record.provider.options));

@ -263,7 +263,7 @@ command_fails_like(
'file',
'/tmp/file',
],
qr/Error: provider not found/,
qr/error: provder "incorrect-global-provider" not found for database 1664/,
'gives error on unknown key provider');
command_fails_like(
@ -274,7 +274,7 @@ command_fails_like(
'global-provider',
'incorrect-provider-type',
],
qr/Error: Unknown provider type: incorrect-provider-type/,
qr/error: unknown provider type "incorrect-provider-type"/,
'gives error on unknown provider type');
command_fails_like(
@ -285,7 +285,7 @@ command_fails_like(
'global-provider',
'file',
],
qr/Error: wrong number of arguments./,
qr/error: wrong number of arguments for "file"/,
'gives error on missing arguments for file provider');
command_fails_like(
@ -296,7 +296,7 @@ command_fails_like(
'global-provider',
'kmip',
],
qr/Error: wrong number of arguments./,
qr/error: wrong number of arguments for "kmip"/,
'gives error on missing arguments for kmip provider');
command_fails_like(
@ -307,7 +307,7 @@ command_fails_like(
'global-provider',
'vault-v2',
],
qr/Error: wrong number of arguments./,
qr/error: wrong number of arguments for "vault-v2"/,
'gives error on missing arguments for vault-v2 provider');
done_testing();

Loading…
Cancel
Save