S3Control api in moto

[SimonToftegaard]

I needed to use the s3control api in moto and according to https://github.com/spulec/moto/blob/master/IMPLEMENTATION_COVERAGE.md it was not yet implemented.
So I started working on the implementation, but then I noticed that part of the s3control api was actually already implemented under the s3 api in moto.
So before continuing the work I would really like to be sure what is the right way to continue?
Do a full separate implementation under s3control or something else?

Also the current implementation only seem to work with the builtin account:
from moto.s3.models import ACCOUNT_ID

[bblommers]

Hi @SimonToftegaard, good catch! Ideally everything related to s3control would be separated out, but that would be a breaking change at this point.
How many features are you looking to implement? To ensure backwards compatibility, it might be better to make it part of the s3 api (mock_s3) for now. Once we get around to doing a major release, we can yoink everything out and make it available under mock_s3control.

[SimonToftegaard]

My plan was to initially implement:
get_public_access_block
put_public_access_block
delete_public_access_block
And I have made an urls.py for s3control like this:
`from .responses import S3ControlResponse

url_bases = [
r"https?://(.+).s3-control.(.+).amazonaws.com",
]

url_paths = {
"{0}/v20180820/configuration/publicAccessBlock$": S3ControlResponse.dispatch
}`

Could I just put something similar into the urls py of s3? The main problem for me right now it seems like the current implementation only works for the builtin accountid and I need to setup additional configuration for the account for my test, including an organization structure and that does not seem possible with the builtin one.

Another solution would just to implement the new s3control and leave the old implementation in s3. That way anyone could use the old implementation with mock_s3 and the new implementation could be used with mock_s3control.
Let me know what you think.

[bblommers]

The *_public_access_block-features are already supported, as far as I know? At least, we have tests for them:

https://github.com/spulec/moto/blob/ce9fb906635d266610abe2acd1f47c750bb5257d/tests/test_s3/test_s3.py#L2326-L2688

The fact that we only support the builtin accountid is definitely an issue, but that's Moto-wide - every service is single-account/single-user based. I don't see an easy way of implementing support for this, without losing the current functionality (accepting one account, rejecting unknown accounts).

The builtin accountid is centralized though - can you mock it during the test?

moto.core.ACCOUNT_ID = "myotheraccountid"

[SimonToftegaard]

Could you give a more specific example of how to override the existing builtin accountId?

[bblommers]

The simplest example:

    @mock_s3
    def test_get_public_access_block_for_account():

        client = boto3.client("s3control", region_name="us-west-2")
        with pytest.raises(ClientError) as ce:
            client.get_public_access_block(AccountId="111111111111")
        ce.value.response["Error"]["Code"].should.equal("AccessDenied")

        # With a valid account ID:
        moto.s3.models.ACCOUNT_ID = "111111111111"
        with pytest.raises(ClientError) as ce:
            client.get_public_access_block(AccountId="111111111111")
        assert ce.value.response["Error"]["Code"] == "NoSuchPublicAccessBlockConfiguration"

Note the change to moto.s3.models.ACCOUNT_ID - this value is cached very early on, so overriding it directly ensures the caching is not an issue

[SimonToftegaard]

Tried that exact same thing first, but for some weird reason it failed. Tried again now and it works great. Thank you for the help.
Any value in keeping the work I have already done for s3control? Most of it is similar to what exists in s3 part of moto already. I guess the plan in the future is just to extract the existing implementation in s3 out into the s3control module.

Anyway to update the implementation coverage documentation, so the next person looking for this can see that some of the s3control api is implemented?

[bblommers]

Tried that exact same thing first, but for some weird reason it failed. Tried again now and it works great. Thank you for the help.

All good - glad to hear it works!

Any value in keeping the work I have already done for s3control? Most of it is similar to what exists in s3 part of moto already. I guess the plan in the future is just to extract the existing implementation in s3 out into the s3control module.

Yeah, it could be useful as a basis for the refactor. If you could push the changes to a branch, that would be great.

Anyway to update the implementation coverage documentation, so the next person looking for this can see that some of the s3control api is implemented?

The coverage documentation is completely automated, so there's no clean way of doing that, unfortunately..

[SimonToftegaard]

ok, I will finish up the code I already have and make a pull request.

[bblommers]

Moto 3.0.0 (just released) now has these features under the new mock_s3control decorator - see http://docs.getmoto.org/en/latest/docs/services/s3control.html

I'll close this, but please let us know if you're running into any other issues!