From 46b7ee0e33d8126f43f7fd030dd748413b9ff27b Mon Sep 17 00:00:00 2001 From: Jonathan Cohen Date: Mon, 18 Nov 2019 18:16:24 -0500 Subject: [PATCH] Use the server config instead of env variables for the API port and the server hostname (#488) * Get public hostname from config file * Tidy up * Respond to Review Comments At some point down the line I royally screwed up my git history and reverted a bunch of changes. I'm not sure what or when, so I `git reset --hard 1cd28e4` and tried to redo my past responses to comments from there. * Export SB_PUBLIC_IP in cmd.sh * respond to review comments * Review response --- .../install_scripts/install_server.sh | 9 ++++---- src/shadowbox/README.md | 13 ++++++++++- src/shadowbox/docker/run_action.sh | 14 ++++++++---- .../integration_test/docker-compose.yml | 4 ++-- src/shadowbox/integration_test/test.sh | 7 +++++- src/shadowbox/server/main.ts | 22 +++++++++---------- src/shadowbox/server/server_config.ts | 3 +++ 7 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/server_manager/install_scripts/install_server.sh b/src/server_manager/install_scripts/install_server.sh index 0f02257b..6f63b8a8 100755 --- a/src/server_manager/install_scripts/install_server.sh +++ b/src/server_manager/install_scripts/install_server.sh @@ -251,19 +251,17 @@ function write_config() { if [[ $FLAGS_KEYS_PORT != 0 ]]; then config+=("\"portForNewAccessKeys\":$FLAGS_KEYS_PORT") fi - if [[ ${#config[@]} > 0 ]]; then - echo "{"$(join , "${config[@]}")"}" > $STATE_DIR/shadowbox_server_config.json - fi + config+=$(printf '"hostname": "%q"' ${PUBLIC_HOSTNAME}) + echo "{"$(join , "${config[@]}")"}" > $STATE_DIR/shadowbox_server_config.json } function start_shadowbox() { - # TODO(fortuna): Write PUBLIC_HOSTNAME and API_PORT to config file, + # TODO(fortuna): Write API_PORT to config file, # rather than pass in the environment. declare -a docker_shadowbox_flags=( --name shadowbox --restart=always --net=host -v "${STATE_DIR}:${STATE_DIR}" -e "SB_STATE_DIR=${STATE_DIR}" - -e "SB_PUBLIC_IP=${PUBLIC_HOSTNAME}" -e "SB_API_PORT=${API_PORT}" -e "SB_API_PREFIX=${SB_API_PREFIX}" -e "SB_CERTIFICATE_FILE=${SB_CERTIFICATE_FILE}" @@ -330,6 +328,7 @@ function add_api_url_to_config() { } function check_firewall() { + # TODO(cohenjon) This is incorrect if access keys are using more than one port. local readonly ACCESS_KEY_PORT=$(curl --insecure -s ${LOCAL_API_URL}/access-keys | docker exec -i shadowbox node -e ' const fs = require("fs"); diff --git a/src/shadowbox/README.md b/src/shadowbox/README.md index 0b1e3958..17e5edae 100644 --- a/src/shadowbox/README.md +++ b/src/shadowbox/README.md @@ -165,7 +165,18 @@ yarn shadowbox_docker_build && docker tag quay.io/outline/shadowbox $USER/shadow If you need to test an unsigned image (e.g. your dev one): ``` -DOCKER_CONTENT_TRUST=0 SHADOWBOX_IMAGE=$USER/shadowbox yarn do shadowbox/integration_test/run +DOCKER_CONTENT_TRUST=0 SB_IMAGE=$USER/shadowbox yarn do shadowbox/integration_test/run ``` You can add tags if you need different versions in different clients. + +### Testing Changes to the Server Config + +If your change includes new fields in the server config which are needed at server +start-up time, then you mey need to remove the pre-existing test config: + +``` +rm /tmp/outline/persisted-state/shadowbox_server_config.json +``` + +This will warn about deleting a write-protected file, which is okay to ignore. You will then need to hand-edit the JSON string in src/shadowbox/docker/run_action.sh. diff --git a/src/shadowbox/docker/run_action.sh b/src/shadowbox/docker/run_action.sh index a8bea657..bc0ca980 100755 --- a/src/shadowbox/docker/run_action.sh +++ b/src/shadowbox/docker/run_action.sh @@ -17,13 +17,17 @@ do_action shadowbox/docker/build readonly OUTLINE_DIR=/tmp/outline -mkdir -p $OUTLINE_DIR && touch "$OUTLINE_DIR/config.json" +readonly HOST_STATE_DIR=$OUTLINE_DIR/persisted-state +readonly CONTAINER_STATE_DIR=/root/shadowbox/persisted-state +readonly STATE_CONFIG=$HOST_STATE_DIR/shadowbox_server_config.json +mkdir -p $HOST_STATE_DIR && touch "$HOST_STATE_DIR/shadowbox_config.json" +[[ -e $STATE_CONFIG ]] || echo '{"hostname":"127.0.0.1"}' > $STATE_CONFIG source $ROOT_DIR/src/shadowbox/scripts/make_test_certificate.sh "${OUTLINE_DIR}" # TODO: mount a folder rather than individual files. declare -a docker_bindings=( - -v "$OUTLINE_DIR/config.json":/root/shadowbox/shadowbox_config.json - -v "$OUTLINE_DIR/stats.json":/root/shadowbox/shadowbox_stats.json + -v "$HOST_STATE_DIR":${CONTAINER_STATE_DIR} + -e "SB_STATE_DIR=${CONTAINER_STATE_DIR}" -v ${SB_CERTIFICATE_FILE}:${SB_CERTIFICATE_FILE} -v ${SB_PRIVATE_KEY_FILE}:${SB_PRIVATE_KEY_FILE} -e "LOG_LEVEL=${LOG_LEVEL:-debug}" @@ -32,4 +36,6 @@ declare -a docker_bindings=( -e SB_PRIVATE_KEY_FILE=${SB_PRIVATE_KEY_FILE} ) -docker run --rm -it --network=host --name shadowbox "${docker_bindings[@]}" outline/shadowbox +echo "Running image ${SB_IMAGE}" + +docker run --rm -it --network=host --name shadowbox "${docker_bindings[@]}" ${SB_IMAGE} diff --git a/src/shadowbox/integration_test/docker-compose.yml b/src/shadowbox/integration_test/docker-compose.yml index c3805d61..2b8f87fa 100644 --- a/src/shadowbox/integration_test/docker-compose.yml +++ b/src/shadowbox/integration_test/docker-compose.yml @@ -16,9 +16,8 @@ services: stop_signal: SIGKILL shadowbox: - image: ${SHADOWBOX_IMAGE:-outline/shadowbox} + image: ${SB_IMAGE:-outline/shadowbox} environment: - - SB_PUBLIC_IP=shadowbox - SB_API_PORT=443 - SB_API_PREFIX=${SB_API_PREFIX} - LOG_LEVEL=debug @@ -34,6 +33,7 @@ services: volumes: - ${SB_CERTIFICATE_FILE}:/root/shadowbox/test.crt - ${SB_PRIVATE_KEY_FILE}:/root/shadowbox/test.key + - ${TMP_STATE_DIR}:/root/shadowbox/persisted-state # The user management service doesn't quit with SIGTERM stop_signal: SIGKILL diff --git a/src/shadowbox/integration_test/test.sh b/src/shadowbox/integration_test/test.sh index 66fec1f8..82b2fd69 100755 --- a/src/shadowbox/integration_test/test.sh +++ b/src/shadowbox/integration_test/test.sh @@ -74,7 +74,10 @@ function fail() { function cleanup() { status=$? - (($DEBUG != 0)) || docker-compose --project-name=integrationtest down + if (($DEBUG != 0)); then + docker-compose --project-name=integrationtest down + rm -r ${TMP_STATE_DIR} + fi return $status } @@ -92,6 +95,8 @@ function cleanup() { # Sets everything up export SB_API_PREFIX=TestApiPrefix SB_API_URL=https://shadowbox/${SB_API_PREFIX} + export TMP_STATE_DIR=$(mktemp -d) + echo '{"hostname": "shadowbox"}' > ${TMP_STATE_DIR}/shadowbox_server_config.json docker-compose --project-name=integrationtest up --build -d # Wait for target to come up. diff --git a/src/shadowbox/server/main.ts b/src/shadowbox/server/main.ts index 6305921a..26441782 100644 --- a/src/shadowbox/server/main.ts +++ b/src/shadowbox/server/main.ts @@ -65,7 +65,6 @@ async function main() { prometheus.collectDefaultMetrics({register: prometheus.register}); - const proxyHostname = process.env.SB_PUBLIC_IP; // Default to production metrics, as some old Docker images may not have // SB_METRICS_URL properly set. const metricsCollectorUrl = process.env.SB_METRICS_URL || 'https://metrics-prod.uproxy.org'; @@ -73,16 +72,6 @@ async function main() { logging.warn('process.env.SB_METRICS_URL not set, using default'); } - if (!proxyHostname) { - logging.error('Need to specify SB_PUBLIC_IP for invite links'); - process.exit(1); - } - - logging.debug(`=== Config ===`); - logging.debug(`SB_PUBLIC_IP: ${proxyHostname}`); - logging.debug(`SB_METRICS_URL: ${metricsCollectorUrl}`); - logging.debug(`==============`); - const DEFAULT_PORT = 8081; const apiPortNumber = Number(process.env.SB_API_PORT || DEFAULT_PORT); if (isNaN(apiPortNumber)) { @@ -94,6 +83,17 @@ async function main() { const serverConfig = server_config.readServerConfig(getPersistentFilename('shadowbox_server_config.json')); + const proxyHostname = serverConfig.data().hostname; + if (!proxyHostname) { + logging.error('Need to specify hostname in shadowbox_server_config.json'); + process.exit(1); + } + + logging.debug(`=== Config ===`); + logging.debug(`Hostname: ${proxyHostname}`); + logging.debug(`SB_METRICS_URL: ${metricsCollectorUrl}`); + logging.debug(`==============`); + logging.info('Starting...'); const prometheusPort = await portProvider.reserveFirstFreePort(9090); diff --git a/src/shadowbox/server/server_config.ts b/src/shadowbox/server/server_config.ts index 12e8274f..be33a30c 100644 --- a/src/shadowbox/server/server_config.ts +++ b/src/shadowbox/server/server_config.ts @@ -35,6 +35,8 @@ export interface ServerConfigJson { // Sliding timeframe, in hours, used to measure data usage and enforce data limits. dataUsageTimeframe?: DataUsageTimeframe; // We don't serialize the shadowbox version, this is obtained dynamically from node. + // Public proxy hostname. + hostname?: string; } // Serialized format for rollouts. @@ -54,6 +56,7 @@ export function readServerConfig(filename: string): json_config.JsonConfig