Boavizta / cloud-scanner

📡 Get Boavizta impact data for your aws cloud account usage.
GNU Affero General Public License v3.0
32 stars 7 forks source link

Improve error message when a region is incorrect #439

Closed jnioche closed 4 months ago

jnioche commented 5 months ago

Problem

cargo run -- --aws-region eu-east-1 estimate --hours-use-time 10 | jq
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/cloud-scanner-cli --aws-region eu-east-1 estimate --hours-use-time 10`
cloud_scanner_cli: Using default API at:  https://api.boavizta.org
thread 'main' panicked at cloud-scanner-cli/src/aws_inventory.rs:91:14:
called `Result::unwrap()` on an `Err` value: Cannot list instances

Caused by:
    0: dispatch failure
    1: io error
    2: error trying to connect: dns error: failed to lookup address information: Name or service not known
    3: dns error: failed to lookup address information: Name or service not known
    4: failed to lookup address information: Name or service not known
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The problem is simply that eu-east-1 is a typo. us-east-1 was the correct value. The error message could be more explicit and tell the user that there is no such region.

Solution

An internal lookup of the allowed values and a message indicating that the value does not exist.

demeringo commented 5 months ago

Hi @jnioche, thank you for reporting this issue.

I fully agree, error handling deserve better error messages. For info, we have a similar issue when credentials are incorrects #164

demeringo commented 5 months ago

@jnioche please just let me know if you intend to propose a PR for this, (otherwise I will put it on my todo list).

jnioche commented 5 months ago

@jnioche please just let me know if you intend to propose a PR for this, (otherwise I will put it on my todo list).

my RUST skills are very limited, I'm afraid.

demeringo commented 5 months ago

@jnioche please just let me know if you intend to propose a PR for this, (otherwise I will put it on my todo list).

my RUST skills are very limited, I'm afraid.

No worries with Rust ;-)

I much appreciate you reported the issue in the first place.

Until we have a proper fix, I will mention it in the documentation #442 .

jnioche commented 5 months ago

Had a quick look, there is no way of validating a region in the AWS SDK. There is a JSON file in this repo listing all the regions; it would need to be imported as a resource.

jnioche commented 5 months ago

Actually, since I had specified an AWS profile with export AWS_PROFILE and that the config for that profile has

region = us-east-1

shouldn't the region specified there override the default value?

Setting export AWS_DEFAULT_REGION=us-east-1 does not have an impact either

demeringo commented 4 months ago

Thanks to your feedback, I simplified the code related to how cloud-scanners picks up the region.

See this PR #447.

Now, the logic is:

  1. by default, cloud scanner uses the region that is explicitly passed in parameter of the CLI (or in the http request).
  2. if no region is explicitly passed, it falls back to using the region of the user environment (either in the AWS profile or using the AWS_DEFAULT_REGION environment variable).

Furthermore I added a check (and hoppefully a more explicit error message) after this step so that we should be warned if the region is not supported by cloud scanner (like if AWS opens a new region or if in case of a typo in the region name).

jnioche commented 4 months ago

Thanks @demeringo, just tested your change, it works fine