Skip to content
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

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

Closed
mchadwick opened this issue Mar 24, 2023 · 5 comments
Closed

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

mchadwick opened this issue Mar 24, 2023 · 5 comments

Comments

@mchadwick
Copy link

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
Copy link

rubas commented Mar 24, 2023

@mchadwick Thanks. And 100% agreed.

@bernardd
Copy link
Contributor

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
Copy link

gabrielgiordan commented Apr 17, 2023

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:

e952a2c#diff-41e419090cf21eed9e3859653c528e2b63f84f8c7aff4e22afa775dd712f8925R95-R102

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

@bernardd
Copy link
Contributor

bernardd commented Jun 9, 2023

@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
Copy link
Contributor

bernardd commented Jun 20, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants