[GitHub] [airflow] o-nikolas commented on a diff in pull request #26946: Fix assume role if user explicit set credentials

[GitBox <gi...@apache.org>]

o-nikolas commented on code in PR #26946:
URL: https://github.com/apache/airflow/pull/26946#discussion_r992732045


##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -125,7 +125,13 @@ def create_session(self) -> boto3.session.Session:
             return boto3.session.Session(region_name=self.region_name)
         elif not self.role_arn:
             return self.basic_session
-        return self._create_session_with_assume_role(session_kwargs=self.conn.session_kwargs)
+        # Values stored in AwsConnectionWrapper.session_kwargs intend to use only create initial boto3 session
+        # If user want to use 'assume_role' mechanism we need provide only 'region_name'
+        # otherwise other parameters might conflict with base botocore session.

Review Comment:
   ```suggestion
           # Values stored in AwsConnectionWrapper.session_kwargs are intended to be used only to create the initial boto3 session.
           # If the user wants to use the 'assume_role' mechanism then only the 'region_name' needs to be
           # provided, otherwise other parameters might conflict with the base botocore session.
   ```



##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -125,7 +125,13 @@ def create_session(self) -> boto3.session.Session:
             return boto3.session.Session(region_name=self.region_name)
         elif not self.role_arn:
             return self.basic_session
-        return self._create_session_with_assume_role(session_kwargs=self.conn.session_kwargs)
+        # Values stored in AwsConnectionWrapper.session_kwargs intend to use only create initial boto3 session
+        # If user want to use 'assume_role' mechanism we need provide only 'region_name'
+        # otherwise other parameters might conflict with base botocore session.
+        assume_session_kwargs = {}
+        if self.conn.region_name:
+            assume_session_kwargs["region_name"] = self.conn.region_name

Review Comment:
   Do you have a doc link covers this (the fact that only region name is required and adding other kwargs can cause issues) that you can place here in a comment?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org