[cloud-sql-proxy] Remove env var assignment of db passwords#1314
[cloud-sql-proxy] Remove env var assignment of db passwords#1314cjac wants to merge 5 commits intoGoogleCloudDataproc:mainfrom
Conversation
|
/gcbrun |
cjac
changed the title
cjac
changed the title
cjac
force-pushed
the
hive-secret-manager-20250402
branch
2 times, most recently
from
cda43bb to
78da4b1
Compare
|
/gcbrun |
cjac
changed the title
cjac
left a comment
cjac
left a comment
There was a problem hiding this comment.
Some notes about why the changes are being made, and thoughts about how to improve.
| METASTORE_PROXY_PORT="${METASTORE_INSTANCE##*:}" | ||
| else | ||
| METASTORE_PROXY_PORT=${DEFAULT_DB_PORT["${CLOUDSQL_INSTANCE_TYPE}"]} | ||
| if [[ -z "${METASTORE_PROXY_PORT}" ]] ; then |
There was a problem hiding this comment.
don't overwrite the value that was collected from the metadata attribute ; only attempt to set this value if the attrbiute was not set.
| DB_ADMIN_PASSWORD_URI="$(/usr/share/google/get_metadata_value attributes/db-admin-password-uri || echo '')" | ||
| readonly DB_ADMIN_PASSWORD_URI | ||
|
|
||
| DB_ADMIN_PASSWORD='' |
There was a problem hiding this comment.
remove DB_ADMIN_PASSWORD environment variable entirely. Do not store sensitive data in environment variables.
cloud-sql-proxy/cloud-sql-proxy.sh
Outdated
| if [[ -n "${DB_ADMIN_SECRET}" ]] ; then | ||
| gcloud secrets versions access "${DB_ADMIN_SECRET#*:}" \ | ||
| --project="${METASTORE_INSTANCE%%:*}" \ | ||
| --secret="${DB_ADMIN_SECRET%:*}" > /dev/shm/db-pw |
There was a problem hiding this comment.
Instead of storing classified material in environment variables, store to ephemeral storage, and ensure it is purged on exit of script.
cloud-sql-proxy/cloud-sql-proxy.sh
Outdated
| touch /dev/shm/db-pw | ||
| fi | ||
| if [[ "${CLOUDSQL_INSTANCE_TYPE}" == "POSTGRES" && -z "${DB_ADMIN_PASSWORD}" ]]; then | ||
| if [[ "${CLOUDSQL_INSTANCE_TYPE}" == "POSTGRES" && -z "$(perl -pe 'chomp' < /dev/shm/db-pw)" ]]; then |
There was a problem hiding this comment.
this test could instead be done with wc -c to eliminate the final instance of the variable being stored in bash strings
cloud-sql-proxy/cloud-sql-proxy.sh
Outdated
| gsutil cat "${DB_HIVE_PASSWORD_URI}" | | ||
| gcloud kms decrypt \ | ||
| --ciphertext-file - \ | ||
| --plaintext-file - \ |
There was a problem hiding this comment.
--plaintext-file /dev/shm/hive-pw
| local proxy_flags | ||
| proxy_flags="$(get_proxy_flags)" | ||
|
|
||
| # Validate db_hive_password and escape invalid xml characters if found. |
There was a problem hiding this comment.
these lines are duplicated in the single perl line below
| EOF | ||
|
|
||
| bdconfig merge_configurations \ | ||
| /usr/local/bin/bdconfig merge_configurations \ |
There was a problem hiding this comment.
rocky doesn't have /usr/local/bin in its PATH
cloud-sql-proxy/cloud-sql-proxy.sh
Outdated
| 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 | ||
| local admin_connection=postgresql://"${DB_ADMIN_USER}":"$(perl -pe 'chomp' < /dev/shm/db-pw)"@127.0.0.1:"${METASTORE_PROXY_PORT}"/ |
There was a problem hiding this comment.
I was mistaken, after the wc change above, these two lines will be the final instance of assignment of classified material to bash environment variable. I'm not certain how best to address this problem, but I will stand up a PG instance to see if there's a way to specify DSN components from a config file as is done for mysql with --defaults-file=${admin_defaults_file}
cjac
force-pushed
the
hive-secret-manager-20250402
branch
2 times, most recently
from
b41174f to
680dd55
Compare
Add secret manager as a database password specification source
cjac
force-pushed
the
hive-secret-manager-20250402
branch
from
680dd55 to
0f660fc
Compare
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
cloud-sql-proxy/cloud-sql-proxy.sh
Outdated
| touch /dev/shm/db-pw | ||
| fi | ||
| if [[ "${CLOUDSQL_INSTANCE_TYPE}" == "POSTGRES" && -z "${DB_ADMIN_PASSWORD}" ]]; then | ||
| if [[ "${CLOUDSQL_INSTANCE_TYPE}" == "POSTGRES" ]] && [[ "$(perl -pe 'chomp' < /dev/shm/db-pw | wc -c)" != "0" ]]; then |
There was a problem hiding this comment.
What does the perl validation do in this context?
There was a problem hiding this comment.
Thanks for asking, Axel!
[ETA: Composed on phone en route to/from stadium]
The < directs to perl's STDIN file handle the contents of the named file
perl -e instructs Perl to evaluate the value of the -e argument instead of loading and evaluating a named file. For instance, the following instructs Perl to print hello world (followed by a newline character [$/]) to STDOUT: perl -e 'print "hello world$/"'
perl -pe instructs Perl to act as a filter, printing to STDOUT, one line at a time, anything that it receives on STDIN, after evaluating the code in the argument to -e. A string containing the line being processed is assigned to the default variable, $_ before the -e code is evaluated, and $_ is printed to STDOUT after the code is evaluated.
The Perl chomp function strips a trailing newline ($/) from a string variable named as chomp's first argument. If no argument is passed, strips trailing newline from the default variable, $_. If no trailing newline exists, chomp is a null operation.
So in toto, this command reads the file, strips the newline, and compares the result to an empty string. The functionality of the Perl code is the same as the bash code, but does not expose the secret to the log when bash is executed with the -x option.
I have made minor alterations to this password length check to be pushed anon. Must have proper keyboard to use emacs.
cjac
changed the title
2 participants
Add secret manager as a password specification source