-
Notifications
You must be signed in to change notification settings - Fork 11
Shift UI env variables to runtime config #5
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,2 @@ | ||
| # Management Service API | ||
| ./management-service/bin | ||
|
|
||
| # XP UI | ||
| ./ui/build |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| #!/bin/bash | ||
| set -e | ||
|
|
||
| CMD=() | ||
| XP_UI_CONFIG_PATH= | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should these config be part of https://github.com/gojekfarm/xp/blob/main/CONTRIBUTING.md ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be captured as part of the Helm chart, in |
||
| XP_UI_DIST_DIR=${XP_UI_DIST_DIR:-} | ||
| XP_UI_DIST_CONFIG_FILE=${XP_UI_DIST_DIR}/app.config.js | ||
| XP_API_BIN=${XP_API_BIN:?"ERROR: XP_API_BIN is not specified"} | ||
|
|
||
| show_help() { | ||
| cat <<EOF | ||
| Usage: $(basename "$0") <options> <...> | ||
| -ui-config JSON file containing configuration of XP UI | ||
| EOF | ||
| } | ||
|
|
||
| main(){ | ||
| parse_command_line "$@" | ||
|
|
||
| if [[ -n "$XP_UI_CONFIG_PATH" ]]; then | ||
| echo "XP UI config found at ${XP_UI_CONFIG_PATH}..." | ||
| if [[ -n "$XP_UI_DIST_DIR" ]]; then | ||
| echo "Overriding UI config at $XP_UI_DIST_CONFIG_FILE" | ||
|
|
||
| echo "var xpConfig = $(cat $XP_UI_CONFIG_PATH);" > "$XP_UI_DIST_CONFIG_FILE" | ||
|
|
||
| echo "Done." | ||
| else | ||
| echo "XP_UI_DIST_DIR: XP UI static build directory not provided. Skipping." | ||
| fi | ||
| else | ||
| echo "XP UI config is not provided. Skipping." | ||
| fi | ||
| } | ||
|
|
||
| parse_command_line(){ | ||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| -ui-config) | ||
| if [[ -n "$2" ]]; then | ||
| XP_UI_CONFIG_PATH="$2" | ||
| shift | ||
| else | ||
| echo "ERROR: '-ui-config' cannot be empty." >&2 | ||
| show_help | ||
| exit 1 | ||
| fi | ||
| ;; | ||
| *) | ||
| CMD+=("$1") | ||
| ;; | ||
| esac | ||
|
|
||
| shift | ||
| done | ||
|
|
||
| if [[ -n "$XP_UI_CONFIG_PATH" ]]; then | ||
| if [ ! -f "$XP_UI_CONFIG_PATH" ]; then | ||
| echo "ERROR: config file $XP_UI_CONFIG_PATH does not exist." >&2 | ||
| show_help | ||
| exit 1 | ||
| fi | ||
| fi | ||
| } | ||
|
|
||
| main "$@" | ||
|
|
||
| echo "Launching xp-api server: " "$XP_API_BIN" "${CMD[@]}" | ||
| exec "$XP_API_BIN" "${CMD[@]}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| var xpConfig = { | ||
| "apiConfig": { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible we set default for all (less auth)? im assuming user need to overwrite this file
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, user should override this file via configuring I've left sensible default values that are commented out for values which are configurable, I think that should suffice? Unless there's certain values you think I should further include. |
||
| /* | ||
| * Timeout (in milliseconds) for requests to API | ||
| * "apiTimeout": 10000, | ||
| * | ||
| * Endpoint to XP API | ||
| * "xpApiUrl": "/api/xp/v1", | ||
| * | ||
| * Endpoint to MLP API | ||
| * "mlpApiUrl": "/api/v1" | ||
| */ | ||
| }, | ||
|
|
||
| "authConfig": { | ||
| /* | ||
| * OAuth2 Client ID | ||
| * "oauthClientId": "CLIENT_ID" | ||
| */ | ||
| }, | ||
|
|
||
| "appConfig": { | ||
| /* | ||
| * Environment name | ||
| * "environment": "dev", | ||
| * | ||
| * Default page for documentation | ||
| * "docsUrl": "https://github.com/gojek/xp" | ||
| */ | ||
| }, | ||
|
|
||
| "sentryConfig": { | ||
| /* | ||
| * DSN of Sentry project | ||
| * "dsn": "SENTRY_DSN", | ||
| * | ||
| * Sentry environment (if it's different from appConfig.environment) | ||
| * "environment": "dev" | ||
| */ | ||
| } | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any particular reasons why creating user and group is required?
I would thought its for security reasons why the user and group are created to limit which user can execute the binaries in entrypoint.sh, or is this configure there via
USER ${XP_USER}?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, containers run as root. A container running as root has full control of the host system. If a service can run without privileges, ideally, we should use a non-root user to restrict privileges. This works in our case, since we're essentially just launching an application service and don’t require root access.
USERdoes not create a non-root user for use, it simply switches from defaultROOTuser to the specified non-root user. Hence, we have to create it before using it.