thumbor-community / aws

Thumbor AWS extensions
MIT License
155 stars 70 forks source link

Use a more up-to-date aiobotocore to allow assumeRole iam credentials #156

Open gcavalcante8808 opened 2 years ago

gcavalcante8808 commented 2 years ago

Hi Folks,

I'd like to ask about the bump of aiobotocore from version 0.12.0 to 2.22.0 to be able to use IRSA and other assumeRole options during credentials discovery.

Actually, based on the tag 7.0.1b I've made the required changes in this fork https://github.com/gcavalcante8808/aws/tree/improvement/use-botocore-2-2-0-to-support-assume-role-credentials

~but I'm having some issues on the following tests:~

~ tests/test_storage.py::CryptoS3StorageTestCase::test_can_store_crypto~ ~ tests/test_storage.py::DetectorS3StorageTestCase::test_can_store_detector_data~ ~* tests/test_result_storage.py::S3StorageTestCase::test_can_get_image~

~Sometimes the tests passes and others from storage fails ... even changing that tornado testing timeout env var to 30 or more.~

~But if I use the pycharm debugger and make some "pauses" they work flawlessly... seem to be something related with async or even moto, but I didn't investigate far.~

Edit: actually, the tests failling were a real symptom: any tests that relied on response['Body'].read() was getting stuck.

After some tests, I just circumvected the situation with the following solution: https://github.com/gcavalcante8808/aws/blob/improvement/use-botocore-2-2-0-to-support-assume-role-credentials/tc_aws/aws/bucket.py#L74-L81.

Now all tests are working and some e2e tests that I made using thumbor, worked as well.

Versions

tc_aws == 7.0.1b thumbor==7.0.7.

Steps to reproduce

  1. Clone the https://github.com/gcavalcante8808/aws repo and checkout the improvement/use-botocore-2-2-0-to-support-assume-role-credentials branch;
  2. Create a docker container by running docker run -it -e ASYNC_TEST_TIMEOUT=30 --rm -v $(pwd):/data python:3 bash;
  3. Run the tests by calling make test;

Question

What you do you think? Is it possible to update aiobotocore to the latest version or even take advantage of the PR that I did?

Thanks in advance.

gcavalcante8808 commented 2 years ago

Ah just to let you know, those are some knowledge tests that I write to the test the loader (my main case is a loader related case) as well:

import os
from contextvars import ContextVar

import aiobotocore.session
import pytest
import pytest_asyncio
from derpconf.config import Config
from thumbor.context import Context

from tc_aws.loaders import s3_loader
from tests import start_service
from .fixtures.storage_fixture import IMAGE_PATH, IMAGE_BYTES, s3_bucket

MOTO_PROCCESS = ContextVar('moto_proccess', default=None)

@pytest_asyncio.fixture
def session():
    return aiobotocore.session.get_session()

@pytest.fixture
def moto_server():
    if not MOTO_PROCCESS.get():
        os.environ['AWS_SHARED_CREDENTIALS_FILE'] = ''
        os.environ['AWS_ACCESS_KEY_ID'] = 'test-key'
        os.environ['AWS_SECRET_ACCESS_KEY'] = 'test-secret-key'
        os.environ['AWS_SESSION_TOKEN'] = 'test-session-token'

        moto_pid = start_service("localhost", 5000)

        MOTO_PROCCESS.set(moto_pid)

@pytest.fixture
def tc_aws_config():
    return Config(
        TC_AWS_LOADER_BUCKET=s3_bucket,
        TC_AWS_LOADER_ROOT_PATH='root_path',
        TC_AWS_REGION='us-east-1',
        TC_AWS_ENDPOINT="http://localhost:5000"
    )

@pytest.mark.asyncio
async def test_test_can_load_image_from_s3(moto_server, session, tc_aws_config):
    async with session.create_client('s3',
                                     **{'region_name': 'us-east-1', 'endpoint_url': "http://localhost:5000"}) as client:
        await client.create_bucket(Bucket=s3_bucket)
        await client.put_object(
            Bucket=s3_bucket,
            Key=''.join(['root_path', IMAGE_PATH]),
            Body=IMAGE_BYTES,
            ContentType='image/jpeg')

    loader_result = await s3_loader.load(Context(config=tc_aws_config), IMAGE_PATH)

    assert loader_result
Bladrak commented 2 years ago

Hi @gcavalcante8808 and thanks for reporting this. Could you submit your devs as a pull request here? We'll review it and see that it gets integrated :)

gcavalcante8808 commented 2 years ago

As I started the branch from the commit/tag 7.0.1b, the changes between the current repo master and the branch improvement/use-botocore-2-2-0-to-support-assume-role-credentials (in my forked repo) are slightly far from those that I made in the branch.

Hi @Bladrak, with this in mind I ask: what branch Do I target with the PR?

Bladrak commented 2 years ago

@gcavalcante8808 hi, can you target the py3 branch?

Tenzer commented 2 years ago

I would like to see aiobotocore get an update as well. I have just tried to upgrade from Thumbor 6.x to 7.0.8 using 7.0.1b0 of this plugin, and I am seeing cannot pickle 'coroutine' object exceptions, which seem to be related to this.

The exception looks similar to what was reported to aiobotocore in https://github.com/aio-libs/aiobotocore/issues/779 and that was fixed in version 1.0.0.

hp197 commented 1 year ago

An update to this issue would be very welcome.

oliverschewe commented 9 months ago

+1