Update laravel sqs documentation

[giagara]

Updated Laravel SQS documentation to achive this: https://github.com/brefphp/laravel-bridge/issues/155

In the documentation is mentioned how to use SQS with the default connection, but not with a custom one (with a name different than sqs).

I've explained how to achieve it.

[deleugpn]

As I mentioned in the linked issue, removing all the keys related to AWS auth would also solve the problem.

I don't recommend us to document the use of env() in a Service Provider as that would break with Laravel config:cache functionality

[giagara]

@deleugpn agree but, in the actual BrefServiceProvider is what is already done:

https://github.com/brefphp/laravel-bridge/blob/1dd21c63c3b14a12b4a73bb79fbc2854d2556349/src/BrefServiceProvider.php#L50

[mnapoli]

Thanks for opening this, this is a very common issue unfortunately.

I agree with removing the lines in the config, but I wonder what's the simplest and most robust solution we can provide to Laravel users. Is there any way we can automate this somehow? (in the Bref bridge) it needs to be reliable of course, I'm not sure I can think of a good strategy…

[giagara]

@mnapoli i think the doc should explain the situation and let the developers choose what they want to do.

I didn't check if removing key and secret it will work. This is something I will investigate.

For sure there is some lack in the documentation.

[mnapoli]

let the developers choose what they want to do.

The thing here is that there is only one thing to do, else it doesn't work 😄

But yeah it sounds best to document removing the extra lines of config, rather than adding code in the service provider.

[giagara]

But yeah it sounds best to document removing the extra lines of config, rather than adding code in the service provider.

Ok i'm gonna work on it. Do you mind to remove the token parameter injection in the BrefServiceProvider too?

Because i agree with saying to remove the keys, but the BrefServiceProvider will still be there

[mnapoli]

We need to keep that line in the BrefServiceProvider because it fixes the default configuration without users having to do anything. We want things to work out of the box for new users.

Maybe what we should do in BrefServiceProvider is:

• if the default config contains the key and secret key, then add token
• if not, then don't do anything

[giagara]

@mnapoli I generally agree but the problem persists: we need to document what is running behind the scenes

I'm still concerned about custom queue connection: If i understood correctly the suggestion is to manage the default (queue.connections.sqs) connection, but not the others.

We can do both at this point:

• check if key and secret in default one and add/remove
• document what is running behind the scenes: if you set secret and key remember to add a token, else, remove the secret and key. Best practice is to remove secret and key

[mnapoli]

Oh yes sorry I wasn't clear, 100% agree.

• for the default, we try to auto-fix the config
• for additional connections, we document how to fix the config

[giagara]

@mnapoli I've updated the PR and I was going to edit the ServiceProvider but i found that all the services (cache, filesystem, queue, ses) are the same. Should we need to update all of them? I think so but want to share it before working on it.

[hschimpf]

Hi guys! I'll like to join this discussion.

What do you think about changing the way that laravel-bridge fixes the connection of SQS?

Instead of changing only the default one named sqs, we could change the settings of all queue connections where the driver is sqs. Something like this:

BrefServiceProvider.php::register()

- Config::set('queue.connections.sqs.token', env('AWS_SESSION_TOKEN'));
+ foreach (Config::get('queue.connections') as $name => $connection) {
+     if ($connection['driver'] !== 'sqs') continue;
+
+     Config::set("queue.connections.$name.token", env('AWS_SESSION_TOKEN'));
+ }

⚠️ above code is not tested!

---

Also, we could do the same could be for cache.stores.* and filesystems.disks.*. Check against the driver and append the AWS_SESSION_TOKEN on all services where an AWS driver is used.

[mnapoli]

@hschimpf oh that's a really good idea, I like it!

Can anyone think of a downside to this?

I think the only one I can imagine is if one configures AWS keys explicitly (they don't use the ones provided by AWS Lambda) and as such the AWS token shouldn't be included… Maybe that can be worked around with something like this:

foreach (Config::get('queue.connections') as $name => $connection) {
    if ($connection['driver'] !== 'sqs') continue;

    // If a different key is in the config than in the environment variables
    $accessKeyId = $_SERVER['AWS_ACCESS_KEY_ID'];
    if ($connection['key'] && $connection['key'] !== $accessKeyId) continue;

    Config::set("queue.connections.$name.token", env('AWS_SESSION_TOKEN'));
}

(not tested as well)