Added region support to CLI

[rob-ryszewski]

Added support for (in priority order):

• --region
• AMP_REGION
• amp set:region

Otherwise defaults to "us"

Example script

The example script:

+ cat ./validate-region.sh
#!/usr/bin/env bash
set -x

cat "${BASH_SOURCE[0]}"

cd "$(dirname "${0}")"

ls -l ./bin/amp; rm -f ./bin/amp; ls -l ./bin/amp
go build -ldflags "-X github.com/amp-labs/cli/vars.Stage=staging \
  -X github.com/amp-labs/cli/vars.ApiURL=https://staging-api.withampersand.com \
  -X github.com/amp-labs/cli/vars.LoginURL=https://staging-cli-signin.withampersand.com \
  -X github.com/amp-labs/cli/vars.ClerkRootURL=https://clerk.withampersand.com" -o bin/amp .

# brand new config file per testing session
export XDG_CONFIG_HOME=$(mktemp -d)

# fake clerk JWT file to pretend have logged in
S='{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
plant() { mkdir -p "${XDG_CONFIG_HOME}/amp"
          for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-region-staging; do echo "${S}" > "${XDG_CONFIG_HOME}/amp/${f}.json"; done; }
plant

cat "${XDG_CONFIG_HOME}/Ampersand/config.json"; echo

./bin/amp
./bin/amp set:region
./bin/amp get:region

./bin/amp set:region eu
./bin/amp get:region
./bin/amp list:projects --key not-a-real-key --debug 2>&1 | grep -E '^Host:'
./bin/amp generate-request-token --debug 2>&1 | grep -E '^Host:'
timeout 4 ./bin/amp login 2>&1 | grep 'please open'

./bin/amp set:region regionDoesntExistYet
./bin/amp get:region
./bin/amp list:projects --key not-a-real-key --debug 2>&1 | grep -E '^Host:'
./bin/amp generate-request-token --debug 2>&1 | grep -E '^Host:'
timeout 4 ./bin/amp login 2>&1 | grep 'please open'

./bin/amp --region la list:projects --key not-a-real-key --debug 2>&1 | grep -E '^Host:'
./bin/amp --region la generate-request-token --debug 2>&1 | grep -E '^Host:'
timeout 4 ./bin/amp --region la login 2>&1 | grep 'please open'

export AMP_REGION=ap
./bin/amp get:region
./bin/amp list:projects --key not-a-real-key --debug 2>&1 | grep -E '^Host:'
./bin/amp generate-request-token --debug 2>&1 | grep -E '^Host:'
timeout 4 ./bin/amp login 2>&1 | grep 'please open'

export AMP_REGION=envvar_region
./bin/amp --region arg_region get:region
./bin/amp --region arg_region list:projects --key not-a-real-key --debug 2>&1 | grep -E '^Host:'
./bin/amp --region arg_region generate-request-token --debug 2>&1 | grep -E '^Host:'
timeout 4 ./bin/amp --region arg_region login 2>&1 | grep 'please open'

fresh local build:

++ dirname ./validate-region.sh
+ cd .
+ ls -l ./bin/amp
-rwxr-xr-x  1 robryszewski  staff  17018690 Aug 25 22:19 ./bin/amp
+ rm -f ./bin/amp
+ ls -l ./bin/amp
ls: ./bin/amp: No such file or directory
+ go build -ldflags '-X github.com/amp-labs/cli/vars.Stage=staging   -X github.com/amp-labs/cli/vars.ApiURL=https://staging-api.withampersand.com   -X github.com/amp-labs/cli/vars.LoginURL=https://staging-cli-signin.withampersand.com   -X github.com/amp-labs/cli/vars.ClerkRootURL=https://clerk.withampersand.com' -o bin/amp .
++ mktemp -d
+ export XDG_CONFIG_HOME=/var/folders/gt/l0cthjsj7p9djnhfvbpgq92c0000gn/T/tmp.798K4b6zDo
+ XDG_CONFIG_HOME=/var/folders/gt/l0cthjsj7p9djnhfvbpgq92c0000gn/T/tmp.798K4b6zDo
+ S='{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ plant
+ mkdir -p /var/folders/gt/l0cthjsj7p9djnhfvbpgq92c0000gn/T/tmp.798K4b6zDo/amp
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ for f in jwt jwt-staging jwt-eu-staging jwt-la-staging jwt-ap-staging jwt-regiondoesntexistyet-staging jwt-arg-reason-staging
+ echo '{"userId":"u","sessionId":"s","token":"t","cookies":{"__c":"x"}}'
+ cat /var/folders/gt/l0cthjsj7p9djnhfvbpgq92c0000gn/T/tmp.798K4b6zDo/Ampersand/config.json
cat: /var/folders/gt/l0cthjsj7p9djnhfvbpgq92c0000gn/T/tmp.798K4b6zDo/Ampersand/config.json: No such file or directory
+ echo

basic help messages:

+ ./bin/amp
The Ampersand CLI allows you to interact with the Ampersand platform.

Usage:
  amp [command]

Available Commands:
  delete:installation    Delete installation
  delete:integration     Delete integration
  deploy                 Deploy changes to integrations
  get:region             Print the region the CLI is talking to
  help                   Help about any command
  list:destinations      List destinations
  list:installations     List installations
  list:integrations      List integrations
  list:projects          List projects
  login                  Log into an Ampersand account
  logout                 Log out of an ampersand account
  set:region             Save the region the CLI should talk to (default "us")
  version                Print this CLI's version

Flags:
  -d, --debug            Enable debug logging mode, defaults to false.
  -h, --help             help for amp
  -k, --key string       Ampersand API key
  -p, --project string   Ampersand project name or ID
  -r, --region string    Ampersand region to talk to, e.g. us or eu. If never set, defaults to us.

Use "amp [command] --help" for more information about a command.
+ ./bin/amp set:region
Error: accepts 1 arg(s), received 0
Usage:
  amp set:region <region> [flags]

Flags:
  -h, --help   help for set:region

Global Flags:
  -d, --debug            Enable debug logging mode, defaults to false.
  -k, --key string       Ampersand API key
  -p, --project string   Ampersand project name or ID
  -r, --region string    Ampersand region to talk to, e.g. us or eu. If never set, defaults to us.

basic get/set, and showing it changes the URLs:

+ ./bin/amp get:region
us
+ ./bin/amp set:region eu
Region set to eu.
+ ./bin/amp get:region
eu
+ ./bin/amp list:projects --key not-a-real-key --debug
+ grep -E '^Host:'
Host: staging-api.eu.withampersand.com
+ ./bin/amp generate-request-token --debug
+ grep -E '^Host:'
Host: clerk.eu.withampersand.com
+ timeout 4 ./bin/amp login
+ grep 'please open'
No browser detected, please open https://staging-cli-signin.eu.withampersand.com in your browser to log in.

again, but with region regionDoesntExistYet:

+ ./bin/amp set:region regionDoesntExistYet
Region set to regiondoesntexistyet.
Warning: this version of amp does not know region "regiondoesntexistyet"; requests will go to https://staging-api.regiondoesntexistyet.withampersand.com.
+ ./bin/amp get:region
regiondoesntexistyet
+ ./bin/amp list:projects --key not-a-real-key --debug
+ grep -E '^Host:'
Host: staging-api.regiondoesntexistyet.withampersand.com
+ ./bin/amp generate-request-token --debug
+ grep -E '^Host:'
Host: clerk.regiondoesntexistyet.withampersand.com
+ timeout 4 ./bin/amp login
+ grep 'please open'
No browser detected, please open https://staging-cli-signin.regiondoesntexistyet.withampersand.com in your browser to log in.

with --region la:

+ ./bin/amp --region la list:projects --key not-a-real-key --debug
+ grep -E '^Host:'
Host: staging-api.la.withampersand.com
+ ./bin/amp --region la generate-request-token --debug
+ grep -E '^Host:'
Host: clerk.la.withampersand.com
+ timeout 4 ./bin/amp --region la login
+ grep 'please open'
No browser detected, please open https://staging-cli-signin.la.withampersand.com in your browser to log in.

with AMP_REGION=ap;

+ export AMP_REGION=ap
+ AMP_REGION=ap
+ ./bin/amp get:region
ap
+ ./bin/amp list:projects --key not-a-real-key --debug
+ grep -E '^Host:'
Host: staging-api.ap.withampersand.com
+ ./bin/amp generate-request-token --debug
+ grep -E '^Host:'
Host: clerk.ap.withampersand.com
+ timeout 4 ./bin/amp login
+ grep 'please open'
No browser detected, please open https://staging-cli-signin.ap.withampersand.com in your browser to log in.

with AMP_REGION and --region both set:

+ export AMP_REGION=envvar-region
+ AMP_REGION=envvar-region
+ ./bin/amp --region arg-region get:region
arg-region
+ ./bin/amp --region arg-region list:projects --key not-a-real-key --debug
+ grep -E '^Host:'
Host: staging-api.arg-region.withampersand.com
+ ./bin/amp --region arg-region generate-request-token --debug
+ grep -E '^Host:'
Host: clerk.arg-region.withampersand.com
+ timeout 4 ./bin/amp --region arg-region login
+ grep 'please open'
No browser detected, please open https://staging-cli-signin.arg-region.withampersand.com in your browser to log in.

[rob-ryszewski]

Since this was only Config.Token and not being used anywhere, (token was being managed in amp/jwt.json by clerk/clerk.go, this particular changeset doesn't break any backwards compatibility for this config

[rob-ryszewski]

the explicit priority list

[rob-ryszewski]

figured this would be preferable to a new region being created and needing to release a new CLI version and users having to update to it (or do the 3 env var export)

[laurenzlong]

@rob-ryszewski I'll let @eberle1080 review this for technical accuracy, but can you swap the priority order so that it is:

• --region flag
• amp set:region
• AMP_REGION env var

[eberle1080]

A few minor suggestions but no blockers

[eberle1080]

Check for the proper error to exclude (e.g.os.IsNotExist(err)) instead of assuming that's what the error is and unconditionally returning nil

[rob-ryszewski]

ah, ty! looks like with this particular xdg method, we only get a broad catchall error:

• xdg.SearchConfigFile
  • base_dirs.searchConfigFile
    • pathutil.Search

fmt.Errorf("could not locate `%s` in any of the following paths: %v",
		filepath.Base(name), searchedPaths)

so ended up switching to xdg.ConfigFile, and doing the os.IsNotExist(err) check after the os.ReadFile(), let me know what you think!

[rob-ryszewski]

@rob-ryszewski I'll let @eberle1080 review this for technical accuracy, but can you swap the priority order so that it is:

• --region flag
• amp set:region
• AMP_REGION env var

Order changed, and just wanted to confirm with you:

• --region always takes effect
• if no --region, amp set:region takes effect
• if no --region no set:region, AMP_REGION takes effect (or if unset, defaults to "us")

If somebody uses AMP_REGION but it's going to be ignored (--region or set:region have values):

• I added a log line to stderr (not stdout, to not pollute cli output) saying that it's being ignored

logger.Warnf("ignoring %s=%q; using the region saved by 'amp set:region' (%s) instead.",
        region.EnvVar, fromEnv, selected

In short:

1. is the log line in line with what you were thinking?
2. just double checking that, once someone has ever amp set:region, then AMP_REGION is basically forever ignored, is correct?