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.

Sign up for GitHub

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

[SWP-5969] - Allow missing credentials to make use of injected IAM role credentials #65

Merged
merged 5 commits into from
Nov 3, 2022
Merged

[SWP-5969] - Allow missing credentials to make use of injected IAM role credentials #65

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc08ee9
Updating the EventServiceProvider so it can be used with null credent…
adteulade Oct 20, 2022
9a4c092
Run composer install with --no-plugins option in CI
jimmy718 Nov 1, 2022
716ce80
Fix incomplete docblock
jimmy718 Nov 2, 2022
0dcc672
Apply fixes from StyleCI
StyleCIBot Nov 2, 2022
a54a6ea
Add EventBridge and Sns braodcaster tests from 0.4.x
jimmy718 Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/run-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
run: |
composer require "laravel/framework:${{ matrix.laravel }}" --no-interaction --no-update
composer require "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update
composer update --prefer-dist --no-interaction --no-progress
composer update --prefer-dist --no-interaction --no-progress --no-plugins
oaklees marked this conversation as resolved.
Show resolved Hide resolved

- name: Execute tests
run: vendor/bin/phpunit
24 changes: 17 additions & 7 deletions src/EventServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ protected function registerSnsBroadcaster()
* @param array $config
* @return \Illuminate\Contracts\Broadcasting\Broadcaster
*/
protected function createSnsDriver(array $config): \Illuminate\Contracts\Broadcasting\Broadcaster
public function createSnsDriver(array $config): \Illuminate\Contracts\Broadcasting\Broadcaster
{
$config = self::prepareConfigurationCredentials($config);

Expand Down Expand Up @@ -81,9 +81,7 @@ protected function registerEventBridgeBroadcaster()
{
$this->app->resolving(BroadcastManager::class, function (BroadcastManager $manager) {
$manager->extend('eventbridge', function (Container $app, array $config) {
return $this->createEventBridgeDriver(array_merge($config, [
'version' => '2015-10-07',
]));
return $this->createEventBridgeDriver($config);
});
});
}
Expand All @@ -94,12 +92,12 @@ protected function registerEventBridgeBroadcaster()
* @param array $config
* @return \Illuminate\Contracts\Broadcasting\Broadcaster
*/
protected function createEventBridgeDriver(array $config): \Illuminate\Contracts\Broadcasting\Broadcaster
public function createEventBridgeDriver(array $config): \Illuminate\Contracts\Broadcasting\Broadcaster
oaklees marked this conversation as resolved.
Show resolved Hide resolved
{
$config = self::prepareConfigurationCredentials($config);

return new EventBridgeBroadcaster(
new EventBridgeClient($config),
new EventBridgeClient(array_merge($config, ['version' => '2015-10-07'])),
$config['source'] ?? ''
);
}
Expand All @@ -112,10 +110,22 @@ protected function createEventBridgeDriver(array $config): \Illuminate\Contracts
*/
public static function prepareConfigurationCredentials(array $config): array
{
if (Arr::has($config, ['key', 'secret'])) {
if (static::configHasCredentials($config)) {
$config['credentials'] = Arr::only($config, ['key', 'secret', 'token']);
}

return $config;
}

/**
* Make sure some AWS credentials were provided to the configuration array.
*
* @return bool
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this JavaDoc string, can you type the input argument as well?

Copy link
Contributor

@oaklees oaklees Nov 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's PHPDoc, which I guess is pretty much the same. We could indeed add @param array $config - for consistency, but believe we just took the code that was added as part of the PR to bring this functionality into the 0.4 branch.

Could we also bring in over the EventBridgeBroadcasterTest and SnsBroadcasterTest?

*/
private static function configHasCredentials(array $config): bool
{
return Arr::has($config, ['key', 'secret'])
&& is_string(Arr::get($config, 'key'))
&& is_string(Arr::get($config, 'secret'));
}
}