ex-aws / ex_aws

A flexible, easy to use set of clients AWS APIs for Elixir
https://hex.pm/packages/ex_aws
MIT License
1.26k stars 521 forks source link

v2.4.2 refreshable feature backwards incompatible with v2.4.1 #941

Closed mchadwick closed 1 year ago

mchadwick commented 1 year ago

Recommend updating CHANGELOG.md for v2.4.2 to indicate that the refreshable feature is a backwards incompatible change in some cases. It is unexpected to see a patch version bump break functionality.

Using ex_aws_s3 2.4.0 with ex_aws 2.4.1:

aws_options = [access_key_id: <secret>, secret_access_key: <secret>, region: "us-east-2"]
{:ok, response} = ExAws.S3.get_object(aws_bucket, key) |> ExAws.request(aws_options)

Using ex_aws_s3 2.4.0 with ex_aws 2.4.2, the refreshable: false is needed to opt out of the behavior, otherwise the operation fails.

aws_options = [access_key_id: <secret>, secret_access_key: <secret>, refreshable: false, region: "us-east-2"]
{:ok, response} = ExAws.S3.get_object(aws_bucket, key) |> ExAws.request(aws_options)

Failure manifests as:

Erlang error: {{%RuntimeError{message: "Instance Meta Error: {:error, %{reason: :timeout}}\n\nYou tried to access the AWS EC2 instance meta, but it could not be reached.\nThis happens most often when trying to access it from your local computer,\nwhich happens when environment variables are not set correctly prompting\nExAws to fallback to the Instance Meta.\n\nPlease check your key config and make sure they're configured correctly:\n\nFor Example:\n```\nExAws.Config.new(:s3)\nExAws.Config.new(:dynamodb)\n```\n"}, [{ExAws.InstanceMeta, :request, 3, [file: 'lib/ex_aws/instance_meta.ex', line: 26, error_info: %{module: Exception}]}, {ExAws.InstanceMeta, :instance_role_credentials, 1, [file: 'lib/ex_aws/instance_meta.ex', line: 83]}, {ExAws.InstanceMeta, :security_credentials, 1, [file: 'lib/ex_aws/instance_meta.ex', line: 91]}, {ExAws.Config.AuthCache, :refresh_auth_now, 2, [file: 'lib/ex_aws/config/auth_cache.ex', line: 132]}, {ExAws.Config.AuthCache, :handle_call, 3, [file: 'lib/ex_aws/config/auth_cache.ex', line: 45]}, {:gen_server, :try_handle_call, 4, [file: 'gen_server.erl', line: 721]}, {:gen_server, :handle_msg, 6, [file: 'gen_server.erl', line: 750]}, {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 226]}]}, {GenServer, :call, [ExAws.Config.AuthCache, {:refresh_auth, %{access_key_id: [{:system, "AWS_ACCESS_KEY_ID"}, :instance_role], host: "s3.us-east-2.amazonaws.com", http_client: ExAws.Request.Hackney, json_codec: Jason, normalize_path: true, port: 443, refreshable: [:instance_role], region: "us-east-2", require_imds_v2: false, retries: [max_attempts: 10, base_backoff_in_ms: 10, max_backoff_in_ms: 10000], scheme: "https://", secret_access_key: [{:system, "AWS_SECRET_ACCESS_KEY"}, :instance_role]}}, 30000]}}
rubas commented 1 year ago

@mchadwick Thanks. And 100% agreed.

bernardd commented 1 year ago

Does this failure happen when running in the AWS environment or only, as the message suggests might be the case, when running locally with no access to an instance meta server? If it's the former, that's definitely a bug - it's never my intention to introduce breaking changes in a minor version. If it's only in a non-AWS environment, well then we could probably argue it either way - at the very least we should add a mention in the docs to turn this off when running like that.

gabrielgiordan commented 1 year ago

Agreed on updating the changelog: in our case, we updated from 2.4.1 to 2.4.2 and had breaking changes on the lib's behavior in AWS environment: we use 2 different regions on S3, and because of that we use config_overrides options for the ExAws.request/1. The thing is, without refreshable: false we fail to use the right region, because the config isn't being overwritten as it was before:

https://github.com/ex-aws/ex_aws/commit/e952a2c1d31ace71cb562c397a1fb495fdeb8678#diff-41e419090cf21eed9e3859653c528e2b63f84f8c7aff4e22afa775dd712f8925R95-R102

The authorization "***" is malformed; the region 'eu-west-1' is wrong; expecting 'us-east-1'
bernardd commented 1 year ago

@mchadwick @rubas @gabrielgiordan Apologies for not getting to this sooner. My real job has been a bit busy. I'm having a look at this, but I'm struggling to figure out exactly what is going wrong - in theory, those overrides are only meant to be refreshed/overwritten in the case where they involve an :awscli or :instance_role type config. Would one of you be able to give me a sample of both your base :ex_aws config, and the override config you're passing into the request function please (with appropriate redactions, obviously)? Thanks.

bernardd commented 1 year ago

I've reversed the behaviour so that the default is as it was in v2.4.1 and the refreshable behaviour must be explicitly enabled. That will, at least, fix backwards compatibility. The more I look at it, the more I feel like the whole auth system is kind of due for a rewrite since it's had new stuff tacked on to it over the years and has become pretty difficult to manage/reason about. But that's a job for when I (or someone else) has "spare time".

Fix is in 2.4.3.