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

[6.x] Fix Cache store with a name other than 'dynamodb' #37145

Merged
merged 2 commits into from
Apr 27, 2021

Conversation

deleugpn
Copy link
Contributor

@deleugpn deleugpn commented Apr 27, 2021

This pull request partially reverts changes introduced in #36749 (ping @driesvints).

The problem particularly is located here: a20c9e5#diff-5c56e532b7bf21453188f98f7999ed558eff31a6374ebd83c9f811961fde02b1R37

It is forcing applications to reserve the cache.stores.dynamodb as the only way to configure an AWS DynamoDb Client instance. If, for instance, we rename dynamodb in our application to anything else, then the region, key and secret won't take effect because it will rely on the DynamoDb Client bound by cache.stores.dynamodb. The consequences of relying on this binding are:

  • Applications cannot rename dynamodb on the cache configuration.
  • Applications cannot have 2 or more dynamodb cache configurations which are not in the same region / access key / secret key.
  • A secondary cache not called dynamodb will still use the region and access configured for the dynamodb key.

By reverting that, we bring control back to users so that they're free to change the skeleton configuration without invisible side effects.

For my particular use-case, I created a Cache Store configuration called session and then configured it as follows:

        'session' => [
            'driver' => 'dynamodb',
            'region' => env('AWS_REGION'),
            'table' => env('DYNAMODB_SESSION_TABLE'),
        ],

This means that access is determined by IAM Role (assume role) instead of key and secret. I also configured session.store as session so that Laravel's Session is managed by the Cache Store called session. This is how I detected the problem.

@deleugpn deleugpn changed the title Fix Cache store with a name other than 'dynamodb' [6.x] Fix Cache store with a name other than 'dynamodb' Apr 27, 2021
@taylorotwell taylorotwell merged commit 74a79b2 into laravel:6.x Apr 27, 2021
@driesvints
Copy link
Member

@deleugpn Thanks for fixing this!

@deleugpn deleugpn deleted the dynamodb branch April 27, 2021 20:34
@deleugpn
Copy link
Contributor Author

My pleasure!

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

Successfully merging this pull request may close these issues.

3 participants