From cc6956c00915f4be8591c78ab657368aa6cbb7b9 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Thu, 9 Jul 2026 16:43:23 +0200 Subject: [PATCH] refactor: extract shared utility sources into src/bin/common/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move 16 .c/.h pairs out of src/bin/pg_autoctl/ into the new src/bin/common/ directory and compile them into libpgaf_common.a, a static archive linked by pg_autoctl (and, in a future PR, pgaftest). New files src/bin/common/Makefile — builds libpgaf_common.a standalone src/bin/common/Makefile.common — shared build variables included by both pg_autoctl/Makefile and the upcoming pgaftest/Makefile; auto- detects macOS gettext via brew Moved files (pg_autoctl → common, superset variants kept) debian, env_utils, file_utils, ini_file, ini_implementation, ipaddr, lock_utils, parsing, pgctl, pgsetup, pgsql, pgtuning, pidfile, signals, string_utils, system_utils Notable content changes in the common/ copies: pgsql.c/h — adds pgsql_alter_role_password() used by the upcoming --autoctl-node-password feature pgctl.c — adds --checkpoint=fast for pg_basebackup; fixes intToString dangling-pointer use pgsetup.c — minor style / brace fixes from citus_indent signals.c — idem ipaddr.c — idem ini_file.c — idem src/bin/Makefile updated to build common/ before pg_autoctl. src/bin/pg_autoctl/Makefile updated to include Makefile.common and link $(COMMON_LIB) instead of compiling the moved sources directly. --- src/bin/Makefile | 12 +- src/bin/common/Makefile | 16 +++ src/bin/common/Makefile.common | 129 ++++++++++++++++++ src/bin/{pg_autoctl => common}/debian.c | 0 src/bin/{pg_autoctl => common}/debian.h | 0 src/bin/{pg_autoctl => common}/env_utils.c | 0 src/bin/{pg_autoctl => common}/env_utils.h | 0 src/bin/{pg_autoctl => common}/file_utils.c | 0 src/bin/{pg_autoctl => common}/file_utils.h | 0 src/bin/{pg_autoctl => common}/ini_file.c | 9 +- src/bin/{pg_autoctl => common}/ini_file.h | 0 .../ini_implementation.c | 0 src/bin/{pg_autoctl => common}/ipaddr.c | 2 + src/bin/{pg_autoctl => common}/ipaddr.h | 0 src/bin/{pg_autoctl => common}/lock_utils.c | 0 src/bin/{pg_autoctl => common}/lock_utils.h | 0 src/bin/{pg_autoctl => common}/parsing.c | 0 src/bin/{pg_autoctl => common}/parsing.h | 0 src/bin/{pg_autoctl => common}/pgctl.c | 43 +++--- src/bin/{pg_autoctl => common}/pgctl.h | 2 +- src/bin/{pg_autoctl => common}/pgsetup.c | 10 ++ src/bin/{pg_autoctl => common}/pgsetup.h | 0 src/bin/{pg_autoctl => common}/pgsql.c | 82 ++++++++++- src/bin/{pg_autoctl => common}/pgsql.h | 2 + src/bin/{pg_autoctl => common}/pgtuning.c | 0 src/bin/{pg_autoctl => common}/pgtuning.h | 0 src/bin/{pg_autoctl => common}/pidfile.c | 0 src/bin/{pg_autoctl => common}/pidfile.h | 0 src/bin/{pg_autoctl => common}/signals.c | 2 + src/bin/{pg_autoctl => common}/signals.h | 0 src/bin/{pg_autoctl => common}/string_utils.c | 0 src/bin/{pg_autoctl => common}/string_utils.h | 0 src/bin/{pg_autoctl => common}/system_utils.c | 0 src/bin/{pg_autoctl => common}/system_utils.h | 0 src/bin/pg_autoctl/Makefile | 97 ++----------- 35 files changed, 294 insertions(+), 112 deletions(-) create mode 100644 src/bin/common/Makefile create mode 100644 src/bin/common/Makefile.common rename src/bin/{pg_autoctl => common}/debian.c (100%) rename src/bin/{pg_autoctl => common}/debian.h (100%) rename src/bin/{pg_autoctl => common}/env_utils.c (100%) rename src/bin/{pg_autoctl => common}/env_utils.h (100%) rename src/bin/{pg_autoctl => common}/file_utils.c (100%) rename src/bin/{pg_autoctl => common}/file_utils.h (100%) rename src/bin/{pg_autoctl => common}/ini_file.c (99%) rename src/bin/{pg_autoctl => common}/ini_file.h (100%) rename src/bin/{pg_autoctl => common}/ini_implementation.c (100%) rename src/bin/{pg_autoctl => common}/ipaddr.c (99%) rename src/bin/{pg_autoctl => common}/ipaddr.h (100%) rename src/bin/{pg_autoctl => common}/lock_utils.c (100%) rename src/bin/{pg_autoctl => common}/lock_utils.h (100%) rename src/bin/{pg_autoctl => common}/parsing.c (100%) rename src/bin/{pg_autoctl => common}/parsing.h (100%) rename src/bin/{pg_autoctl => common}/pgctl.c (99%) rename src/bin/{pg_autoctl => common}/pgctl.h (100%) rename src/bin/{pg_autoctl => common}/pgsetup.c (99%) rename src/bin/{pg_autoctl => common}/pgsetup.h (100%) rename src/bin/{pg_autoctl => common}/pgsql.c (97%) rename src/bin/{pg_autoctl => common}/pgsql.h (99%) rename src/bin/{pg_autoctl => common}/pgtuning.c (100%) rename src/bin/{pg_autoctl => common}/pgtuning.h (100%) rename src/bin/{pg_autoctl => common}/pidfile.c (100%) rename src/bin/{pg_autoctl => common}/pidfile.h (100%) rename src/bin/{pg_autoctl => common}/signals.c (99%) rename src/bin/{pg_autoctl => common}/signals.h (100%) rename src/bin/{pg_autoctl => common}/string_utils.c (100%) rename src/bin/{pg_autoctl => common}/string_utils.h (100%) rename src/bin/{pg_autoctl => common}/system_utils.c (100%) rename src/bin/{pg_autoctl => common}/system_utils.h (100%) diff --git a/src/bin/Makefile b/src/bin/Makefile index 7c743833d..4a489b55e 100644 --- a/src/bin/Makefile +++ b/src/bin/Makefile @@ -1,15 +1,19 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the PostgreSQL License. -all: pg_autoctl ; +all: common pg_autoctl ; -pg_autoctl: +common: + $(MAKE) -C common + +pg_autoctl: common $(MAKE) -C pg_autoctl pg_autoctl clean: + $(MAKE) -C common clean $(MAKE) -C pg_autoctl clean -install: $(pg_autoctl) +install: pg_autoctl $(MAKE) -C pg_autoctl install -.PHONY: all pg_autoctl install clean +.PHONY: all common pg_autoctl install clean diff --git a/src/bin/common/Makefile b/src/bin/common/Makefile new file mode 100644 index 000000000..5b106fcc4 --- /dev/null +++ b/src/bin/common/Makefile @@ -0,0 +1,16 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the PostgreSQL License. + +SRC_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) + +.DEFAULT_GOAL := $(SRC_DIR)libpgaf_common.a + +include $(SRC_DIR)Makefile.common + +override CFLAGS += -I$(SRC_DIR)../pg_autoctl + +clean: + rm -f $(COMMON_OBJ) $(COMMON_LIB) + rm -rf $(DEPDIR) + +.PHONY: clean diff --git a/src/bin/common/Makefile.common b/src/bin/common/Makefile.common new file mode 100644 index 000000000..be7a0fe9e --- /dev/null +++ b/src/bin/common/Makefile.common @@ -0,0 +1,129 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the PostgreSQL License. +# +# Makefile.common — shared build variables and common source library. +# Included by both pg_autoctl and pgaftest Makefiles via: +# include ../common/Makefile.common + +PG_CONFIG ?= pg_config + +COMMON_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) +LIB_DIR := $(COMMON_DIR)../lib + +PG_SNPRINTF = $(wildcard $(LIB_DIR)/pg/snprintf.*) +LOG_SRC = $(wildcard $(LIB_DIR)/log/src/log.*) +COMMANDLINE_SRC = $(wildcard $(LIB_DIR)/subcommands.c/commandline.*) +PARSON_SRC = $(wildcard $(LIB_DIR)/parson/parson.*) + +COMMON_INCLUDES = -I$(COMMON_DIR) +COMMON_INCLUDES += -I$(LIB_DIR)/pg +COMMON_INCLUDES += -I$(LIB_DIR)/log/src/ +COMMON_INCLUDES += -I$(LIB_DIR)/subcommands.c/ +COMMON_INCLUDES += -I$(LIB_DIR)/libs/ +COMMON_INCLUDES += -I$(LIB_DIR)/parson/ + +CC = $(shell $(PG_CONFIG) --cc) + +DEFAULT_CFLAGS = -std=c99 -D_GNU_SOURCE -g +DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --includedir) +DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --includedir-server) +DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --pkgincludedir)/internal +DEFAULT_CFLAGS += $(shell $(PG_CONFIG) --cflags) +DEFAULT_CFLAGS += -Wformat +DEFAULT_CFLAGS += -Wall +DEFAULT_CFLAGS += -Werror=implicit-int +DEFAULT_CFLAGS += -Werror=implicit-function-declaration +DEFAULT_CFLAGS += -Werror=return-type +DEFAULT_CFLAGS += -Wno-declaration-after-statement +DEFAULT_CFLAGS += -D_WANT_SEMUN +DEFAULT_CFLAGS += -Wno-missing-braces +DEFAULT_CFLAGS += $(COMMON_INCLUDES) + +# On macOS, gettext is keg-only and not in the default include path. +# postgres_fe.h → c.h → libintl.h requires finding it explicitly. +ifeq ($(shell uname -s),Darwin) + GETTEXT_PREFIX := $(shell brew --prefix gettext 2>/dev/null) + ifneq ($(GETTEXT_PREFIX),) + DEFAULT_CFLAGS += -I$(GETTEXT_PREFIX)/include + endif +endif + +override CFLAGS := $(DEFAULT_CFLAGS) $(CFLAGS) + +BINDIR ?= $(shell $(PG_CONFIG) --bindir) + +# pg_config --ldflags from source builds can emit colon-joined paths such as +# -L/path/a:/path/b as a single token, which the linker rejects. Split each +# -L token on ':' so every directory gets its own -L flag. +PG_LDFLAGS_RAW := $(shell $(PG_CONFIG) --ldflags) +PG_LDFLAGS := $(shell echo "$(PG_LDFLAGS_RAW)" | awk '{ \ + for (i = 1; i <= NF; i++) { \ + if (substr($$i, 1, 2) == "-L") { \ + n = split(substr($$i, 3), p, ":"); \ + for (j = 1; j <= n; j++) if (p[j] != "") printf "-L%s ", p[j]; \ + } else { printf "%s ", $$i; } \ + } \ +}') + +LIBS = -L $(shell $(PG_CONFIG) --pkglibdir) +LIBS += -L $(shell $(PG_CONFIG) --libdir) +LIBS += $(PG_LDFLAGS) +LIBS += $(shell $(PG_CONFIG) --libs) +LIBS += -lpq + +DEPDIR = .deps + +# ----------------------------------------------------------------------- +# Common source library (src/bin/common/*.c → libpgaf_common.a) +# Callers link with: $(COMMON_LIB) and add $(COMMON_LIB) to OBJS. +# ----------------------------------------------------------------------- + +COMMON_SRC = $(wildcard $(COMMON_DIR)*.c) +COMMON_OBJ = $(patsubst $(COMMON_DIR)%.c,$(COMMON_DIR)%.o,$(COMMON_SRC)) +COMMON_LIB = $(COMMON_DIR)libpgaf_common.a + +# Compile rule for common objects (run from each caller's directory) +$(COMMON_DIR)%.o: $(COMMON_DIR)%.c + @if test ! -d $(COMMON_DIR)$(DEPDIR); then mkdir -p $(COMMON_DIR)$(DEPDIR); fi + $(CC) $(CFLAGS) -c -MMD -MP -MF$(COMMON_DIR)$(DEPDIR)/$(*F).Po -o $@ $< + +$(COMMON_LIB): $(COMMON_OBJ) + $(AR) rcs $@ $^ + +Po_common_files := $(wildcard $(COMMON_DIR)$(DEPDIR)/*.Po) +ifneq (,$(Po_common_files)) +include $(Po_common_files) +endif + +# ----------------------------------------------------------------------- +# Compile rule for caller's own .c → .o with auto-dependency tracking +# ----------------------------------------------------------------------- +%.o : %.c + @if test ! -d $(DEPDIR); then mkdir -p $(DEPDIR); fi + $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -o $@ $< + +Po_files := $(wildcard $(DEPDIR)/*.Po) +ifneq (,$(Po_files)) +include $(Po_files) +endif + +# Shared rules for vendored libs +lib-snprintf.o: $(PG_SNPRINTF) + $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-snprintf.Po -MT$@ \ + -o $@ $(LIB_DIR)/pg/snprintf.c + +lib-strerror.o: $(PG_SNPRINTF) + $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-strerror.Po -MT$@ \ + -o $@ $(LIB_DIR)/pg/strerror.c + +lib-log.o: $(LOG_SRC) + $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-log.Po -MT$@ \ + -o $@ $(LIB_DIR)/log/src/log.c + +lib-commandline.o: $(COMMANDLINE_SRC) + $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-commandline.Po -MT$@ \ + -o $@ $(LIB_DIR)/subcommands.c/commandline.c + +lib-parson.o: $(PARSON_SRC) + $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-parson.Po -MT$@ \ + -o $@ $(LIB_DIR)/parson/parson.c diff --git a/src/bin/pg_autoctl/debian.c b/src/bin/common/debian.c similarity index 100% rename from src/bin/pg_autoctl/debian.c rename to src/bin/common/debian.c diff --git a/src/bin/pg_autoctl/debian.h b/src/bin/common/debian.h similarity index 100% rename from src/bin/pg_autoctl/debian.h rename to src/bin/common/debian.h diff --git a/src/bin/pg_autoctl/env_utils.c b/src/bin/common/env_utils.c similarity index 100% rename from src/bin/pg_autoctl/env_utils.c rename to src/bin/common/env_utils.c diff --git a/src/bin/pg_autoctl/env_utils.h b/src/bin/common/env_utils.h similarity index 100% rename from src/bin/pg_autoctl/env_utils.h rename to src/bin/common/env_utils.h diff --git a/src/bin/pg_autoctl/file_utils.c b/src/bin/common/file_utils.c similarity index 100% rename from src/bin/pg_autoctl/file_utils.c rename to src/bin/common/file_utils.c diff --git a/src/bin/pg_autoctl/file_utils.h b/src/bin/common/file_utils.h similarity index 100% rename from src/bin/pg_autoctl/file_utils.h rename to src/bin/common/file_utils.h diff --git a/src/bin/pg_autoctl/ini_file.c b/src/bin/common/ini_file.c similarity index 99% rename from src/bin/pg_autoctl/ini_file.c rename to src/bin/common/ini_file.c index 40c88016e..309f77d57 100644 --- a/src/bin/pg_autoctl/ini_file.c +++ b/src/bin/common/ini_file.c @@ -116,11 +116,12 @@ parse_ini_buffer(const char *filename, } default: - + { /* should never happen, or it's a development bug */ log_fatal("Unknown option type %d", option->type); ini_destroy(ini); return false; + } } } } @@ -219,10 +220,11 @@ ini_validate_options(IniOption *optionList) } default: - + { /* should never happen, or it's a development bug */ log_fatal("Unknown option type %d", option->type); return false; + } } } return true; @@ -660,10 +662,11 @@ ini_merge(IniOption *dstOptionList, IniOption *overrideOptionList) } default: - + { /* should never happen, or it's a development bug */ log_fatal("Unknown option type %d", option->type); return false; + } } } return true; diff --git a/src/bin/pg_autoctl/ini_file.h b/src/bin/common/ini_file.h similarity index 100% rename from src/bin/pg_autoctl/ini_file.h rename to src/bin/common/ini_file.h diff --git a/src/bin/pg_autoctl/ini_implementation.c b/src/bin/common/ini_implementation.c similarity index 100% rename from src/bin/pg_autoctl/ini_implementation.c rename to src/bin/common/ini_implementation.c diff --git a/src/bin/pg_autoctl/ipaddr.c b/src/bin/common/ipaddr.c similarity index 99% rename from src/bin/pg_autoctl/ipaddr.c rename to src/bin/common/ipaddr.c index 8b82e0520..7abd47451 100644 --- a/src/bin/pg_autoctl/ipaddr.c +++ b/src/bin/common/ipaddr.c @@ -303,7 +303,9 @@ fetchLocalCIDR(const char *localIpAddress, char *localCIDR, int size) } default: + { continue; + } } if (strcmp(address, localIpAddress) == 0) diff --git a/src/bin/pg_autoctl/ipaddr.h b/src/bin/common/ipaddr.h similarity index 100% rename from src/bin/pg_autoctl/ipaddr.h rename to src/bin/common/ipaddr.h diff --git a/src/bin/pg_autoctl/lock_utils.c b/src/bin/common/lock_utils.c similarity index 100% rename from src/bin/pg_autoctl/lock_utils.c rename to src/bin/common/lock_utils.c diff --git a/src/bin/pg_autoctl/lock_utils.h b/src/bin/common/lock_utils.h similarity index 100% rename from src/bin/pg_autoctl/lock_utils.h rename to src/bin/common/lock_utils.h diff --git a/src/bin/pg_autoctl/parsing.c b/src/bin/common/parsing.c similarity index 100% rename from src/bin/pg_autoctl/parsing.c rename to src/bin/common/parsing.c diff --git a/src/bin/pg_autoctl/parsing.h b/src/bin/common/parsing.h similarity index 100% rename from src/bin/pg_autoctl/parsing.h rename to src/bin/common/parsing.h diff --git a/src/bin/pg_autoctl/pgctl.c b/src/bin/common/pgctl.c similarity index 99% rename from src/bin/pg_autoctl/pgctl.c rename to src/bin/common/pgctl.c index 3afe1d3b9..8ad756ae3 100644 --- a/src/bin/pg_autoctl/pgctl.c +++ b/src/bin/common/pgctl.c @@ -1264,7 +1264,7 @@ pg_basebackup(const char *pgdata, NodeAddress *primaryNode = &(replicationSource->primaryNode); char primaryConnInfo[MAXCONNINFO] = { 0 }; - char *args[16]; + char *args[18]; /* enough for all pg_basebackup flags incl. --checkpoint=fast */ int argsIndex = 0; char command[BUFSIZE]; @@ -1330,6 +1330,7 @@ pg_basebackup(const char *pgdata, args[argsIndex++] = "--max-rate"; args[argsIndex++] = replicationSource->maximumBackupRate; args[argsIndex++] = "--wal-method=stream"; + args[argsIndex++] = "--checkpoint=fast"; /* we don't use a replication slot e.g. when upstream is a standby */ if (!IS_EMPTY_STRING_BUFFER(replicationSource->slotName)) @@ -1628,13 +1629,12 @@ pg_ctl_postgres(const char *pg_ctl, const char *pgdata, int pgport, /* prepare startup.log file in PGDATA */ join_path_components(logfile, pgdata, "startup.log"); - IntString pgportStr = intToString(pgport); - args[argsIndex++] = (char *) postgres; args[argsIndex++] = "-D"; args[argsIndex++] = (char *) pgdata; args[argsIndex++] = "-p"; - args[argsIndex++] = pgportStr.strValue; + IntString pgportStr = intToString(pgport); + args[argsIndex++] = (char *) pgportStr.strValue; if (listen) { @@ -2028,56 +2028,57 @@ pg_ctl_status(const char *pg_ctl, const char *pgdata, bool log_output) /* - * pg_ctl_reload reloads PostgreSQL configuration by running "pg_ctl reload". + * pg_ctl_promote promotes a standby by running "pg_ctl promote" */ bool -pg_ctl_reload(const char *pg_ctl, const char *pgdata) +pg_ctl_promote(const char *pg_ctl, const char *pgdata) { - Program program = run_program(pg_ctl, "-D", pgdata, "reload", NULL); + Program program = + run_program(pg_ctl, "-D", pgdata, "--no-wait", "promote", NULL); int returnCode = program.returnCode; + log_debug("%s promote -D %s --no-wait", pg_ctl, pgdata); + if (program.stdErr != NULL) { - log_debug("%s", program.stdErr); + log_error("%s", program.stdErr); } - free_program(&program); - if (returnCode != 0) { - log_error("pg_ctl reload -D %s failed (exit %d)", pgdata, returnCode); + /* pg_ctl promote will have logged errors */ + free_program(&program); return false; } + free_program(&program); return true; } /* - * pg_ctl_promote promotes a standby by running "pg_ctl promote" + * pg_ctl_reload reloads Postgres configuration by running "pg_ctl reload". + * Does not require a libpq connection — useful when HBA hasn't been set up yet. */ bool -pg_ctl_promote(const char *pg_ctl, const char *pgdata) +pg_ctl_reload(const char *pg_ctl, const char *pgdata) { - Program program = - run_program(pg_ctl, "-D", pgdata, "--no-wait", "promote", NULL); + Program program = run_program(pg_ctl, "-D", pgdata, "reload", NULL); int returnCode = program.returnCode; - log_debug("%s promote -D %s --no-wait", pg_ctl, pgdata); - if (program.stdErr != NULL) { - log_error("%s", program.stdErr); + log_debug("%s", program.stdErr); } + free_program(&program); + if (returnCode != 0) { - /* pg_ctl promote will have logged errors */ - free_program(&program); + log_error("pg_ctl reload -D %s failed (exit %d)", pgdata, returnCode); return false; } - free_program(&program); return true; } diff --git a/src/bin/pg_autoctl/pgctl.h b/src/bin/common/pgctl.h similarity index 100% rename from src/bin/pg_autoctl/pgctl.h rename to src/bin/common/pgctl.h index 3466c47a6..e030d8217 100644 --- a/src/bin/pg_autoctl/pgctl.h +++ b/src/bin/common/pgctl.h @@ -57,9 +57,9 @@ bool pg_ctl_postgres(const char *pg_ctl, const char *pgdata, int pgport, bool pg_log_startup(const char *pgdata, int logLevel); bool pg_log_recovery_setup(const char *pgdata, int logLevel); bool pg_ctl_stop(const char *pg_ctl, const char *pgdata); -bool pg_ctl_reload(const char *pg_ctl, const char *pgdata); int pg_ctl_status(const char *pg_ctl, const char *pgdata, bool log_output); bool pg_ctl_promote(const char *pg_ctl, const char *pgdata); +bool pg_ctl_reload(const char *pg_ctl, const char *pgdata); bool pg_setup_standby_mode(uint32_t pg_control_version, const char *pgdata, diff --git a/src/bin/pg_autoctl/pgsetup.c b/src/bin/common/pgsetup.c similarity index 99% rename from src/bin/pg_autoctl/pgsetup.c rename to src/bin/common/pgsetup.c index 5554d0344..c9e0b0ecf 100644 --- a/src/bin/pg_autoctl/pgsetup.c +++ b/src/bin/common/pgsetup.c @@ -1491,8 +1491,10 @@ nodeKindToString(PgInstanceKind kind) } default: + { log_fatal("nodeKindToString: unknown node kind %d", kind); return NULL; + } } /* can't happen, keep compiler happy */ @@ -1563,7 +1565,9 @@ pmStatusToString(PostmasterStatus pm_status) } case POSTMASTER_STATUS_STANDBY: + { return "standby"; + } } /* keep compiler happy */ @@ -1842,7 +1846,9 @@ pgsetup_sslmode_to_string(SSLMode sslMode) } case SSL_MODE_VERIFY_FULL: + { return "verify-full"; + } } /* This is a huge bug */ @@ -1986,7 +1992,9 @@ pgsetup_hba_level_to_string(HBAEditLevel hbaLevel) } case HBA_EDIT_UNKNOWN: + { return "unknown"; + } } log_error("BUG: hbaLevel %d is unknown", hbaLevel); @@ -2033,7 +2041,9 @@ dbstateToString(DBState state) } case DB_IN_PRODUCTION: + { return "in production"; + } } return "unrecognized status code"; } diff --git a/src/bin/pg_autoctl/pgsetup.h b/src/bin/common/pgsetup.h similarity index 100% rename from src/bin/pg_autoctl/pgsetup.h rename to src/bin/common/pgsetup.h diff --git a/src/bin/pg_autoctl/pgsql.c b/src/bin/common/pgsql.c similarity index 97% rename from src/bin/pg_autoctl/pgsql.c rename to src/bin/common/pgsql.c index 534fbbebb..4fe32a084 100644 --- a/src/bin/pg_autoctl/pgsql.c +++ b/src/bin/common/pgsql.c @@ -1581,12 +1581,13 @@ BuildNodesArrayValues(NodeAddressArray *nodeArray, { NodeAddress *node = &(nodeArray->nodes[nodeIndex]); IntString nodeIdStr = intToString(node->nodeId); + char *nodeIdString = nodeIdStr.strValue; int idParamIndex = paramIndex; int lsnParamIndex = paramIndex + 1; sqlParams->types[idParamIndex] = INT8OID; - strlcpy(sqlParams->nodeIds[nodeIndex], nodeIdStr.strValue, NODEID_MAX_LENGTH); + strlcpy(sqlParams->nodeIds[nodeIndex], nodeIdString, NODEID_MAX_LENGTH); /* store the (char *) pointer to the data in values */ sqlParams->values[idParamIndex] = sqlParams->nodeIds[nodeIndex]; @@ -2334,6 +2335,85 @@ pgsql_create_user(PGSQL *pgsql, const char *userName, const char *password, } +/* + * pgsql_alter_role_password runs ALTER ROLE PASSWORD '...'. + * + * The password is never logged; we log "ALTER ROLE PASSWORD '*****'" + * instead. Uses PQescapeIdentifier / PQescapeLiteral so the values are safe + * to interpolate directly into the query string. + */ +bool +pgsql_alter_role_password(PGSQL *pgsql, const char *roleName, + const char *password) +{ + /* open a connection upfront since it is needed by PQescape functions */ + PGconn *connection = pgsql_open_connection(pgsql); + if (connection == NULL) + { + /* error message was logged in pgsql_open_connection */ + return false; + } + + char *escapedRole = PQescapeIdentifier(connection, roleName, strlen(roleName)); + if (escapedRole == NULL) + { + log_error("Failed to escape role name \"%s\": %s", + roleName, PQerrorMessage(connection)); + pgsql_finish(pgsql); + return false; + } + + char *escapedPassword = PQescapeLiteral(connection, password, strlen(password)); + if (escapedPassword == NULL) + { + log_error("Failed to escape password for role \"%s\": %s", + roleName, PQerrorMessage(connection)); + PQfreemem(escapedRole); + pgsql_finish(pgsql); + return false; + } + + /* log without the real password */ + log_debug("ALTER ROLE %s PASSWORD '*****';", escapedRole); + + PQExpBuffer query = createPQExpBuffer(); + appendPQExpBuffer(query, "ALTER ROLE %s PASSWORD %s", escapedRole, escapedPassword); + PQfreemem(escapedRole); + PQfreemem(escapedPassword); + + if (PQExpBufferBroken(query)) + { + log_error("Failed to allocate memory"); + destroyPQExpBuffer(query); + pgsql_finish(pgsql); + return false; + } + + PGresult *result = PQexec(connection, query->data); + destroyPQExpBuffer(query); + + if (!is_response_ok(result)) + { + log_error("Failed to alter role \"%s\" password: %s", + roleName, PQerrorMessage(connection)); + PQclear(result); + clear_results(pgsql); + pgsql_finish(pgsql); + return false; + } + + PQclear(result); + clear_results(pgsql); + + if (pgsql->connectionStatementType == PGSQL_CONNECTION_SINGLE_STATEMENT) + { + pgsql_finish(pgsql); + } + + return true; +} + + /* * pgsql_has_replica returns whether a replica with the given username is active. */ diff --git a/src/bin/pg_autoctl/pgsql.h b/src/bin/common/pgsql.h similarity index 99% rename from src/bin/pg_autoctl/pgsql.h rename to src/bin/common/pgsql.h index cd8387172..978f6b172 100644 --- a/src/bin/pg_autoctl/pgsql.h +++ b/src/bin/common/pgsql.h @@ -366,6 +366,8 @@ bool pgsql_create_extension(PGSQL *pgsql, const char *name); bool pgsql_create_user(PGSQL *pgsql, const char *userName, const char *password, bool login, bool superuser, bool replication, int connlimit); +bool pgsql_alter_role_password(PGSQL *pgsql, const char *roleName, + const char *password); bool pgsql_has_replica(PGSQL *pgsql, char *userName, bool *hasReplica); bool hostname_from_uri(const char *pguri, char *hostname, int maxHostLength, int *port); diff --git a/src/bin/pg_autoctl/pgtuning.c b/src/bin/common/pgtuning.c similarity index 100% rename from src/bin/pg_autoctl/pgtuning.c rename to src/bin/common/pgtuning.c diff --git a/src/bin/pg_autoctl/pgtuning.h b/src/bin/common/pgtuning.h similarity index 100% rename from src/bin/pg_autoctl/pgtuning.h rename to src/bin/common/pgtuning.h diff --git a/src/bin/pg_autoctl/pidfile.c b/src/bin/common/pidfile.c similarity index 100% rename from src/bin/pg_autoctl/pidfile.c rename to src/bin/common/pidfile.c diff --git a/src/bin/pg_autoctl/pidfile.h b/src/bin/common/pidfile.h similarity index 100% rename from src/bin/pg_autoctl/pidfile.h rename to src/bin/common/pidfile.h diff --git a/src/bin/pg_autoctl/signals.c b/src/bin/common/signals.c similarity index 99% rename from src/bin/pg_autoctl/signals.c rename to src/bin/common/signals.c index 0114affe0..a870d66f5 100644 --- a/src/bin/pg_autoctl/signals.c +++ b/src/bin/common/signals.c @@ -253,6 +253,8 @@ signal_to_string(int signal) } default: + { return "unknown signal"; + } } } diff --git a/src/bin/pg_autoctl/signals.h b/src/bin/common/signals.h similarity index 100% rename from src/bin/pg_autoctl/signals.h rename to src/bin/common/signals.h diff --git a/src/bin/pg_autoctl/string_utils.c b/src/bin/common/string_utils.c similarity index 100% rename from src/bin/pg_autoctl/string_utils.c rename to src/bin/common/string_utils.c diff --git a/src/bin/pg_autoctl/string_utils.h b/src/bin/common/string_utils.h similarity index 100% rename from src/bin/pg_autoctl/string_utils.h rename to src/bin/common/string_utils.h diff --git a/src/bin/pg_autoctl/system_utils.c b/src/bin/common/system_utils.c similarity index 100% rename from src/bin/pg_autoctl/system_utils.c rename to src/bin/common/system_utils.c diff --git a/src/bin/pg_autoctl/system_utils.h b/src/bin/common/system_utils.h similarity index 100% rename from src/bin/pg_autoctl/system_utils.h rename to src/bin/common/system_utils.h diff --git a/src/bin/pg_autoctl/Makefile b/src/bin/pg_autoctl/Makefile index f1d884db1..b1bc05322 100644 --- a/src/bin/pg_autoctl/Makefile +++ b/src/bin/pg_autoctl/Makefile @@ -5,104 +5,37 @@ PG_AUTOCTL = ./pg_autoctl SRC_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) -DEPDIR = $(SRC_DIR)/.deps +# Must be set before include so that targets in Makefile.common don't +# become the default goal when included before the all: rule below. +.DEFAULT_GOAL := all -INCLUDES = $(patsubst ${SRC_DIR}%.h,%.h,$(wildcard ${SRC_DIR}*.h)) +include $(SRC_DIR)../common/Makefile.common -SRC = $(patsubst ${SRC_DIR}%.c,%.c,$(wildcard ${SRC_DIR}*.c)) -OBJS = $(patsubst %.c,%.o,$(SRC)) -OBJS += lib-log.o lib-commandline.o lib-parson.o lib-snprintf.o lib-strerror.o - -PG_CONFIG ?= pg_config -BINDIR ?= $(shell $(PG_CONFIG) --bindir) - -PG_SNPRINTF = $(wildcard ${SRC_DIR}../lib/pg/snprintf.*) -LOG_SRC = $(wildcard ${SRC_DIR}../lib/log/src/log.*) -COMMANDLINE_SRC = $(wildcard ${SRC_DIR}../lib/subcommands.c/commandline.*) -PARSON_SRC = $(wildcard ${SRC_DIR}../lib/parson/parson.*) - -COMMON_LIBS = -I${SRC_DIR}../lib/pg -COMMON_LIBS += -I${SRC_DIR}../lib/log/src/ -COMMON_LIBS += -I${SRC_DIR}../lib/subcommands.c/ -COMMON_LIBS += -I${SRC_DIR}../lib/libs/ -COMMON_LIBS += -I${SRC_DIR}../lib/parson/ - -CC = $(shell $(PG_CONFIG) --cc) +# pg_autoctl includes its own source directory too +override CFLAGS += -I$(SRC_DIR) -DEFAULT_CFLAGS = -std=c99 -D_GNU_SOURCE -g -DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --includedir) -DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --includedir-server) -DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --pkgincludedir)/internal -DEFAULT_CFLAGS += $(shell $(PG_CONFIG) --cflags) -DEFAULT_CFLAGS += -Wformat -DEFAULT_CFLAGS += -Wall -DEFAULT_CFLAGS += -Werror=implicit-int -DEFAULT_CFLAGS += -Werror=implicit-function-declaration -DEFAULT_CFLAGS += -Werror=return-type -DEFAULT_CFLAGS += -Wno-declaration-after-statement +INCLUDES = $(patsubst $(SRC_DIR)%.h,%.h,$(wildcard $(SRC_DIR)*.h)) -# Needed for FreeBSD -DEFAULT_CFLAGS += -D_WANT_SEMUN - -# Needed for OSX -DEFAULT_CFLAGS += -Wno-missing-braces -DEFAULT_CFLAGS += $(COMMON_LIBS) - -ifdef USE_SECURITY_FLAGS -# Flags taken from: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10203#guide -SECURITY_CFLAGS=-fstack-protector-strong -D_FORTIFY_SOURCE=2 -O2 -z noexecstack -fpie -Wl,-pie -Wl,-z,relro -Wl,-z,now -Wformat -Wformat-security -Werror=format-security -DEFAULT_CFLAGS += $(SECURITY_CFLAGS) -endif - -override CFLAGS := $(DEFAULT_CFLAGS) $(CFLAGS) +SRC = $(patsubst $(SRC_DIR)%.c,%.c,$(wildcard $(SRC_DIR)*.c)) +OBJS = $(patsubst %.c,%.o,$(SRC)) +OBJS += lib-log.o lib-commandline.o lib-parson.o lib-snprintf.o lib-strerror.o +OBJS += $(COMMON_LIB) -LIBS = -L $(shell $(PG_CONFIG) --pkglibdir) -LIBS += -L $(shell $(PG_CONFIG) --libdir) -LIBS += $(shell $(PG_CONFIG) --ldflags) -LIBS += $(shell $(PG_CONFIG) --libs) -LIBS += -lpq +# ncurses needed for pg_autoctl watch LIBS += -lncurses -all: $(PG_AUTOCTL) ; - -# Based on Postgres Makefile for automatic dependency generation -# https://github.com/postgres/postgres/blob/1933ae629e7b706c6c23673a381e778819db307d/src/Makefile.global.in#L890-L924 -%.o : %.c - @if test ! -d $(DEPDIR); then mkdir -p $(DEPDIR); fi - $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -o $@ $< - -Po_files := $(wildcard $(DEPDIR)/*.Po) -ifneq (,$(Po_files)) -include $(Po_files) -endif - +all: $(COMMON_LIB) $(PG_AUTOCTL) ; $(PG_AUTOCTL): $(OBJS) $(INCLUDES) $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LIBS) -o $@ -lib-snprintf.o: $(PG_SNPRINTF) - $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -MT$@ -o $@ ${SRC_DIR}../lib/pg/snprintf.c - -lib-strerror.o: $(PG_SNPRINTF) - $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -MT$@ -o $@ ${SRC_DIR}../lib/pg/strerror.c - -lib-log.o: $(LOG_SRC) - $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -MT$@ -o $@ ${SRC_DIR}../lib/log/src/log.c - -lib-commandline.o: $(COMMANDLINE_SRC) - $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -MT$@ -o $@ ${SRC_DIR}../lib/subcommands.c/commandline.c - -lib-parson.o: $(PARSON_SRC) - $(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -MT$@ -o $@ ${SRC_DIR}../lib/parson/parson.c - clean: rm -f $(OBJS) $(PG_AUTOCTL) + rm -f $(COMMON_LIB) $(COMMON_OBJ) rm -rf $(DEPDIR) install: $(PG_AUTOCTL) install -d $(DESTDIR)$(BINDIR) install -m 0755 $(PG_AUTOCTL) $(DESTDIR)$(BINDIR) - - -.PHONY: all monitor clean +.PHONY: all clean install