Fix backwards compatibility issue in AWS provider's _get_credentials

[potiuk]

The #19815 change introduced backwards incompatibility for
the _get_credentials method - which is a centerpiece of AWS
provider and is likely to be overwritten by the user who want
for example inject auditing or other credentials-related custom
beheviours when interfacing with AWS even if the method is
protected.

The change added default for region, which caused signature
incompatibility with such derived classes. Unfortunately, we
already released 2.5.0 provider with this change. We had to
yank it and in order to avoid adding backwards-incompatible
3.0.0 release we are going to release 2.5.1 with this change
included.

Fixes: #20457

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

[potiuk]

Once we review and merge this one I will prepare 2.5.1 release of the provider.

[potiuk]

All test_base_aws.py passed locally after the changes.

[kaxil]

lgtm but would wait till @dstandish or @uranusjr reviews it

[github-actions]

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

[potiuk]

Aaaaah tooo fast :) (just saw your comment @kaxil @dstandish @uranusjr - let me know if there is anything to fix :)

[dstandish]

nothing to fix i'd say but i am curious why we no longer let the hook handle auth more completely....

like it used to be

        s3_resource = self.get_resource_type('s3')
        return s3_resource.Bucket(bucket_name)

but as of this change we have much more complicated expression and get our hands more dirty. just curious for the reasoning on that. is there a reason not to let base hook handle giving us an authenticated resource, instead of repeating that logic within method calls?

also why not let region_name be optional (i.e. have a default of None) in _get_credentials? that doesn't necessarily harm backcompat

[potiuk]

You'll find answers here @dstandish : #20457 (comment)

The backcompat is kinda unexpected. But somewhat realistic case (and I guess @kaxil and @uranusjr already got a report about it) when someone overrides the _get_credentials() "an old way" and expand_role() would fail in this case.

This is not obvious but might happen.

[potiuk]

But yeah. In this case when I added the default to expand_role - it could be back set to None default.