Skip to content

[cloud-sql-proxy] Remove env var assignment of db passwords #1314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[cloud-sql-proxy] Remove env var assignment of db passwords #1314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f660fc
[cloud-sql-proxy] Remove env var assignment of mysql passwords
cjac Apr 3, 2025
91d4f7d
sanitize postgres password code
cjac Apr 4, 2025
25616d3
wrap args in double quotes ; remove set +x nonsense
cjac Apr 5, 2025
938c15c
no need for such a large instance for this test
cjac Apr 5, 2025
504ed58
using argument to specify output file instead of redirect ; clean up …
cjac Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 145 additions & 78 deletions cloud-sql-proxy/cloud-sql-proxy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
# Do not use "set -x" to avoid printing passwords in clear in the logs
set -euo pipefail

function os_id() ( set +x ; grep '^ID=' /etc/os-release | cut -d= -f2 | xargs ; )
function os_version() ( set +x ; grep '^VERSION_ID=' /etc/os-release | cut -d= -f2 | xargs ; )
function os_codename() ( set +x ; grep '^VERSION_CODENAME=' /etc/os-release | cut -d= -f2 | xargs ; )
function os_id() { grep '^ID=' /etc/os-release | cut -d= -f2 | xargs ; }
function os_version() { grep '^VERSION_ID=' /etc/os-release | cut -d= -f2 | xargs ; }
function os_codename(){ grep '^VERSION_CODENAME=' /etc/os-release | cut -d= -f2 | xargs ; }

function version_ge() ( set +x ; [ "$1" = "$(echo -e "$1\n$2" | sort -V | tail -n1)" ] ; )
function version_gt() ( set +x ; [ "$1" = "$2" ] && return 1 || version_ge $1 $2 ; )
function version_le() ( set +x ; [ "$1" = "$(echo -e "$1\n$2" | sort -V | head -n1)" ] ; )
function version_lt() ( set +x ; [ "$1" = "$2" ] && return 1 || version_le $1 $2 ; )
function version_ge(){ [[ "$1" = "$(echo -e "$1\n$2"|sort -V|tail -n1)" ]]; }
function version_gt(){ [[ "$1" = "$2" ]]&& return 1 || version_ge "$1" "$2";}
function version_le(){ [[ "$1" = "$(echo -e "$1\n$2"|sort -V|head -n1)" ]]; }
function version_lt(){ [[ "$1" = "$2" ]]&& return 1 || version_le "$1" "$2";}

readonly -A supported_os=(
['debian']="10 11 12"
Expand All @@ -41,16 +41,16 @@ if [[ "$(os_id)" == "rocky" ]];
then _os_version=$(os_version | sed -e 's/[^0-9].*$//g')
else _os_version="$(os_version)"; fi
for os_id_val in 'rocky' 'ubuntu' 'debian' ; do
eval "function is_${os_id_val}() ( set +x ; [[ \"$(os_id)\" == '${os_id_val}' ]] ; )"
eval "function is_${os_id_val}() { [[ \"$(os_id)\" == '${os_id_val}' ]] ; }"

for osver in $(echo "${supported_os["${os_id_val}"]}") ; do
eval "function is_${os_id_val}${osver%%.*}() ( set +x ; is_${os_id_val} && [[ \"${_os_version}\" == \"${osver}\" ]] ; )"
eval "function ge_${os_id_val}${osver%%.*}() ( set +x ; is_${os_id_val} && version_ge \"${_os_version}\" \"${osver}\" ; )"
eval "function le_${os_id_val}${osver%%.*}() ( set +x ; is_${os_id_val} && version_le \"${_os_version}\" \"${osver}\" ; )"
eval "function is_${os_id_val}${osver%%.*}() { is_${os_id_val} && [[ \"${_os_version}\" == \"${osver}\" ]] ; }"
eval "function ge_${os_id_val}${osver%%.*}() { is_${os_id_val} && version_ge \"${_os_version}\" \"${osver}\" ; }"
eval "function le_${os_id_val}${osver%%.*}() { is_${os_id_val} && version_le \"${_os_version}\" \"${osver}\" ; }"
done
done

function is_debuntu() ( set +x ; is_debian || is_ubuntu ; )
function is_debuntu() { is_debian || is_ubuntu ; }

function print_metadata_value() {
local readonly tmpfile=$(mktemp)
Expand All @@ -73,8 +73,7 @@ function print_metadata_value_if_exists() {
return ${return_code}
}

function get_metadata_value() (
set +x
function get_metadata_value() {
local readonly varname=$1
local -r MDS_PREFIX=http://metadata.google.internal/computeMetadata/v1
# Print the instance metadata value.
Expand All @@ -87,14 +86,13 @@ function get_metadata_value() (
fi

return ${return_code}
)
}

function get_metadata_attribute() (
set +x
function get_metadata_attribute() {
local -r attribute_name="$1"
local -r default_value="${2:-}"
get_metadata_value "attributes/${attribute_name}" || echo -n "${default_value}"
)
}

# Detect dataproc image version from its various names
if (! test -v DATAPROC_IMAGE_VERSION) && test -v DATAPROC_VERSION; then
Expand Down Expand Up @@ -209,13 +207,19 @@ fi
readonly CLOUDSQL_INSTANCE_TYPE

METASTORE_PROXY_PORT="$(/usr/share/google/get_metadata_value attributes/metastore-proxy-port || echo '')"
if [[ "${METASTORE_INSTANCE}" =~ =tcp:[0-9]+$ ]]; then
METASTORE_PROXY_PORT="${METASTORE_INSTANCE##*:}"
else
METASTORE_PROXY_PORT=${DEFAULT_DB_PORT["${CLOUDSQL_INSTANCE_TYPE}"]}
if [[ -z "${METASTORE_PROXY_PORT}" ]] ; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't overwrite the value that was collected from the metadata attribute ; only attempt to set this value if the attrbiute was not set.

if [[ "${METASTORE_INSTANCE}" =~ =tcp:[0-9]+$ ]]; then
METASTORE_PROXY_PORT="${METASTORE_INSTANCE##*:}"
else
METASTORE_PROXY_PORT=${DEFAULT_DB_PORT["${CLOUDSQL_INSTANCE_TYPE}"]}
fi
fi
readonly METASTORE_PROXY_PORT

# Allow users to specify hive password using secret manager
DB_HIVE_SECRET="$(/usr/share/google/get_metadata_value attributes/db-hive-secret || echo '')"
DB_ADMIN_SECRET="$(/usr/share/google/get_metadata_value attributes/db-admin-secret || echo '')"

# Database user to use to access metastore.
DB_HIVE_USER="$(/usr/share/google/get_metadata_value attributes/db-hive-user || echo 'hive')"
readonly DB_HIVE_USER
Expand All @@ -233,39 +237,50 @@ readonly KMS_KEY_URI
DB_ADMIN_PASSWORD_URI="$(/usr/share/google/get_metadata_value attributes/db-admin-password-uri || echo '')"
readonly DB_ADMIN_PASSWORD_URI

DB_ADMIN_PASSWORD=''
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove DB_ADMIN_PASSWORD environment variable entirely. Do not store sensitive data in environment variables.

if [[ -n "${DB_ADMIN_PASSWORD_URI}" ]]; then
if [[ -n "${DB_ADMIN_SECRET}" ]] ; then
gcloud secrets versions access "${DB_ADMIN_SECRET#*:}" \
--project="${METASTORE_INSTANCE%%:*}" \
--secret="${DB_ADMIN_SECRET%:*}" \
--out-file=/dev/shm/db-pw
elif [[ -n "${DB_ADMIN_PASSWORD_URI}" ]]; then
# Decrypt password
DB_ADMIN_PASSWORD="$(gsutil cat "${DB_ADMIN_PASSWORD_URI}" |
gsutil cat "${DB_ADMIN_PASSWORD_URI}" |
gcloud kms decrypt \
--ciphertext-file - \
--plaintext-file - \
--key "${KMS_KEY_URI}")"
--plaintext-file /dev/shm/db-pw \
--key "${KMS_KEY_URI}"
else
touch /dev/shm/db-pw
fi
if [[ "${CLOUDSQL_INSTANCE_TYPE}" == "POSTGRES" && -z "${DB_ADMIN_PASSWORD}" ]]; then
if [[ "${CLOUDSQL_INSTANCE_TYPE}" == "POSTGRES" ]] &&
[[ "$(perl -e 'chomp($l=<STDIN>); print length $l;' < /dev/shm/db-pw)" == "0" ]]
then
log 'POSTGRES DB admin password is not set'
fi
readonly DB_ADMIN_PASSWORD

# Database password used to access metastore.
DB_HIVE_PASSWORD_URI="$(/usr/share/google/get_metadata_value attributes/db-hive-password-uri || echo '')"
readonly DB_HIVE_PASSWORD_URI
if [[ -n "${DB_HIVE_PASSWORD_URI}" ]]; then
if [[ -n "${DB_HIVE_SECRET}" ]] ; then
gcloud secrets versions access "${DB_HIVE_SECRET#*:}" \
--project="${METASTORE_INSTANCE%%:*}" \
--secret="${DB_HIVE_SECRET%:*}" \
--out-file=/dev/shm/hive-pw
elif [[ -n "${DB_HIVE_PASSWORD_URI}" ]]; then
# Decrypt password
DB_HIVE_PASSWORD="$(gsutil cat "${DB_HIVE_PASSWORD_URI}" |
gsutil cat "${DB_HIVE_PASSWORD_URI}" |
gcloud kms decrypt \
--ciphertext-file - \
--plaintext-file - \
--key "${KMS_KEY_URI}")"
readonly DB_HIVE_PASSWORD
--plaintext-file /dev/shm/hive-pw \
--key "${KMS_KEY_URI}"
else
db_hive_pwd=$(bdconfig get_property_value \
/usr/local/bin/bdconfig get_property_value \
--configuration_file "/etc/hive/conf/hive-site.xml" \
--name "javax.jdo.option.ConnectionPassword" 2>/dev/null)
if [[ "${db_hive_pwd}" == "None" ]]; then
db_hive_pwd="hive-password"
fi
readonly DB_HIVE_PASSWORD=${db_hive_pwd}
--name "javax.jdo.option.ConnectionPassword" 2>/dev/null > /dev/shm/hive-pw
fi

if perl -e 'chomp($l=<STDIN>); exit( $l eq "None" ? 0 : 1 )' < /dev/shm/hive-pw; then
echo "hive-password" > /dev/shm/hive-pw
fi

# Name of MySQL database to use for the metastore.
Expand Down Expand Up @@ -394,13 +409,6 @@ function install_cloud_sql_proxy() {
local proxy_flags
proxy_flags="$(get_proxy_flags)"

# Validate db_hive_password and escape invalid xml characters if found.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these lines are duplicated in the single perl line below

local db_hive_password_xml_escaped
db_hive_password_xml_escaped=${DB_HIVE_PASSWORD//&/&amp;}
db_hive_password_xml_escaped=${db_hive_password_xml_escaped//</&lt;}
db_hive_password_xml_escaped=${db_hive_password_xml_escaped//>/&gt;}
db_hive_password_xml_escaped=${db_hive_password_xml_escaped//'"'/&quot;}

# Install proxy as systemd service for reboot tolerance.
cat <<EOF >${INIT_SCRIPT}
[Unit]
Expand Down Expand Up @@ -443,12 +451,12 @@ EOF
</property>
<property>
<name>javax.jdo.option.ConnectionPassword</name>
<value>${db_hive_password_xml_escaped}</value>
<value>$(perl -pe 'chomp ; s:<:&lt;:g; s:>:&gt;:g ; s:":&quot;:g' < /dev/shm/hive-pw)</value>
</property>
</configuration>
EOF

bdconfig merge_configurations \
/usr/local/bin/bdconfig merge_configurations \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rocky doesn't have /usr/local/bin in its PATH

--configuration_file /etc/hive/conf/hive-site.xml \
--source_configuration_file hive-template.xml \
--clobber
Expand All @@ -457,54 +465,113 @@ EOF
log 'Cloud SQL Proxy installation succeeded'
}

function admin_mysql() {
local admin_defaults_file="/dev/shm/admin-db.cnf"
local db_password_param="--defaults-file=${admin_defaults_file}"
mysql "${db_password_param}" "$*"
}

function hive_mysql() {
local hive_defaults_file="/dev/shm/hive-db.cnf"
local db_hive_password_param="--defaults-file=${hive_defaults_file}"
mysql "${db_hive_password_param}" "$*"
}

function initialize_mysql_metastore_db() {
log 'Initialzing MYSQL DB for Hive metastore ...'
local db_password_param='--password='
if [[ -n ${DB_ADMIN_PASSWORD} ]]; then
db_password_param+=${DB_ADMIN_PASSWORD}
fi
local db_hive_password_param=''
if [[ -n ${DB_HIVE_PASSWORD} ]]; then
db_hive_password_param+="-p${DB_HIVE_PASSWORD}"
fi
log 'Initialzing MySQL DB for Hive metastore ...'
local admin_defaults_file="/dev/shm/admin-db.cnf"
local hive_defaults_file="/dev/shm/hive-db.cnf"
local db_password_param="--defaults-file=${admin_defaults_file}"
local db_hive_password_param="--defaults-file=${hive_defaults_file}"

(
printf "[client]\nhost=127.0.0.1\nport=${METASTORE_PROXY_PORT}\nuser=${DB_ADMIN_USER}\npassword=\""
perl -pe 'chomp' < /dev/shm/db-pw
echo '"'
) > "${admin_defaults_file}"
(
printf "[client]\nhost=127.0.0.1\nport=${METASTORE_PROXY_PORT}\nuser=${DB_HIVE_USER}\npassword=\""
perl -pe 'chomp' < /dev/shm/hive-pw
echo '"'
) > "${hive_defaults_file}"
(
echo -n "CREATE USER IF NOT EXISTS '${DB_HIVE_USER}'@'cloudsqlproxy~%' IDENTIFIED BY '"
perl -pe 'chomp' < /dev/shm/hive-pw
echo -n "';"
) > /dev/shm/create_hive_user.sql

# create hive user if it does not exist
hive_mysql -e '' || \
admin_mysql < /dev/shm/create_hive_user.sql

# Check if metastore is initialized.
if ! mysql -h 127.0.0.1 -P "${METASTORE_PROXY_PORT}" -u "${DB_HIVE_USER}" "${db_hive_password_param}" -e ''; then
mysql -h 127.0.0.1 -P "${METASTORE_PROXY_PORT}" -u "${DB_ADMIN_USER}" "${db_password_param}" -e \
"CREATE USER '${DB_HIVE_USER}' IDENTIFIED BY '${DB_HIVE_PASSWORD}';"
fi
if ! mysql -h 127.0.0.1 -P "${METASTORE_PROXY_PORT}" -u "${DB_HIVE_USER}" "${db_hive_password_param}" -e "use ${METASTORE_DB}"; then
if ! hive_mysql -e "use ${METASTORE_DB}"; then
# Initialize a Hive metastore DB
mysql -h 127.0.0.1 -P "${METASTORE_PROXY_PORT}" -u "${DB_ADMIN_USER}" "${db_password_param}" -e \
"CREATE DATABASE ${METASTORE_DB};
GRANT ALL PRIVILEGES ON ${METASTORE_DB}.* TO '${DB_HIVE_USER}';"
/usr/lib/hive/bin/schematool -dbType mysql -initSchema ||
admin_mysql -e \
"CREATE DATABASE IF NOT EXISTS ${METASTORE_DB};
GRANT ALL PRIVILEGES ON ${METASTORE_DB}.* TO '${DB_HIVE_USER}'@'cloudsqlproxy~%';"
/usr/lib/hive/bin/schematool -dbType mysql -initSchema || {
rm -f /dev/shm/*-db.cnf /dev/shm/*.sql
err 'Failed to set mysql schema.'
}
fi
log 'MYSQL DB initialized for Hive metastore'
rm -f /dev/shm/*-db.cnf /dev/shm/*.sql
}

function exit_handler() {
rm -f /dev/shm/*-pw /dev/shm/*-db.cnf /dev/shm/*_passfile /dev/shm/*.sql
}

trap exit_handler EXIT

function admin_psql() {
PGPASSFILE="/dev/shm/admin_passfile" psql "$*"
}

function hive_psql() {
PGPASSFILE="/dev/shm/hive_passfile" psql "$*"
}

function initialize_postgres_metastore_db() {
log 'Initialzing POSTGRES DB for Hive metastore ...'
local admin_connection=postgresql://"${DB_ADMIN_USER}":"${DB_ADMIN_PASSWORD}"@127.0.0.1:"${METASTORE_PROXY_PORT}"/
local hive_connection=postgresql://"${DB_HIVE_USER}":"${DB_HIVE_PASSWORD}"@127.0.0.1:"${METASTORE_PROXY_PORT}"/postgres
log 'Initialzing PostgreSQL DB for Hive metastore ...'
local admin_pgpass=/dev/shm/admin_passfile
local hive_pgpass=/dev/shm/hive_passfile
(
echo -n "postgresql://${DB_ADMIN_USER}:"
perl -pe 'chomp' < /dev/shm/db-pw
echo -n "@127.0.0.1:${METASTORE_PROXY_PORT}/"
) > "${admin_pgpass}"
(
echo -n "postgresql://${DB_HIVE_USER}:"
perl -pe 'chomp' < /dev/shm/hive-pw
echo -n "@127.0.0.1:${METASTORE_PROXY_PORT}/postgres"
) > "${hive_pgpass}"
(
echo -n "CREATE USER ${DB_HIVE_USER} WITH PASSWORD '"
perl -pe 'chomp' < /dev/shm/hive-pw
echo -n "';"
) > /dev/shm/create_hive_user.sql

# Check if metastore is initialized.
if ! psql "${hive_connection}" -c ''; then
if ! hive_psql -c ''; then
log 'Create DB Hive user...'
psql "${admin_connection}" -c "CREATE USER ${DB_HIVE_USER} WITH PASSWORD '${DB_HIVE_PASSWORD}';"
admin_psql < /dev/shm/create_hive_user.sql
fi
if ! psql "${hive_connection}" -c '\c "${METASTORE_DB}" ' ; then
if ! hive_psql -c '\c "${METASTORE_DB}" ' ; then
log 'Create Hive Metastore database...'
psql "${admin_connection}" -c "CREATE DATABASE ${METASTORE_DB};"
psql "${hive_connection}" -c '\c "${METASTORE_DB}" '
psql "${admin_connection}" -c "GRANT ALL PRIVILEGES ON DATABASE ${METASTORE_DB} TO ${DB_HIVE_USER} ;"
admin_psql -c "CREATE DATABASE ${METASTORE_DB};"
hive_psql -c '\c "${METASTORE_DB}" '
admin_psql -c "GRANT ALL PRIVILEGES ON DATABASE ${METASTORE_DB} TO ${DB_HIVE_USER} ;"

log 'Create Hive Metastore schema...'
/usr/lib/hive/bin/schematool -dbType postgres -initSchema ||
/usr/lib/hive/bin/schematool -dbType postgres -initSchema || {
err 'Failed to set postgres schema.'
rm -f /dev/shm/*_passfile /dev/shm/*.sql
}
fi
log 'POSTGRES DB initialized for Hive metastore'
log 'PostgreSQL DB initialized for Hive metastore'
rm -f /dev/shm/*_passfile /dev/shm/*.sql
}

function initialize_metastore_db() {
Expand Down
2 changes: 1 addition & 1 deletion cloud-sql-proxy/test_cloud_sql_proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_cloud_sql_proxy(self, configuration):
self.createCluster(
configuration,
self.INIT_ACTIONS,
machine_type="n1-standard-8",
machine_type="n1-standard-2",
metadata=metadata,
scopes='sql-admin')

Expand Down