Arvados

Updates at db3234c - branch 20665-installer-secrets-handling
Doc tests run: developer-run-tests-doc-and-sdk-R: #1867
Manually tested by creating a new cluster.

• Separates all security-sensitive data to a new local.params.secrets file, including a-d-c's SSH private key.
• Updates documentation.

Lucas Di Pentima wrote in #note-2:

Updates at db3234c - branch 20665-installer-secrets-handling

This is great, thank you for the documentation updates especially. Just a couple of style suggestions.

-    if [[ ! -s $CONFIG_FILE ]] ; then
+    if [ ! -s ${CONFIG_FILE} -o ! -s ${CONFIG_FILE}.secrets ]; then

As long as we're committed to using bash, my understanding is [[ ]] provides better style because it has richer operators and less surprising precedence rules. So this version would both match the rest of the script better and also be preferred bash style as I understand it:

    if ! [[ -s "${CONFIG_FILE}" && -s "${CONFIG_FILE}.secrets" ]]; then

If we're trying to work towards POSIX compatibility, then ignore my comment.

In the documentation:

this file may need to be handled differently from the others.

When I read this I feel like I don't understand what this is alluding to. Can we make this a little more concrete with an example? e.g., "you may wish to store this file in a secrets store like [name an example or two]."

Thanks.

Updates at 73a972b5a

Addressed above suggestions:

• Improves bash code styling.
• Provides examples on how to securely handle the secrets file.

Lucas Di Pentima wrote in #note-4:

• Improves bash code styling.

-    if [ ! -s ${CONFIG_FILE} -o ! -s ${CONFIG_FILE}.secrets ]; then
+    if [[ -s ${CONFIG_FILE} && -s ${CONFIG_FILE}.secrets ]]; then

This new version needs a negation ! in front of the test condition. As written this version would error out on a good checkout.

• Provides examples on how to securely handle the secrets file.

"AWS Secrets Manager" should be titlecased since it's the name of a specific service.

Go ahead and merge with those changes. Thanks.

Brett Smith wrote in #note-5:

This new version needs a negation ! in front of the test condition. As written this version would error out on a good checkout.

• Provides examples on how to securely handle the secrets file.

"AWS Secrets Manager" should be titlecased since it's the name of a specific service.

Whoops! Thanks for spotting that error. Fixes at 81e02a8fc -- will merge, thanks!