Update S3Provider.php

[lonnylot]

Update S3 to implement correct Updater interface

I wasn't sure if the env vars were added to the binary so I migrated from using AWS keys / secret to a profile

[shanerbaner82]

I had to add the following for the environmentVariables method:

public function environmentVariables(): array
    {
        return [
            'AWS_ACCESS_KEY_ID' => $this->config['key'],
            'AWS_SECRET_ACCESS_KEY' => $this->config['secret'],
        ];
    }

[lonnylot]

I had to add the following for the environmentVariables method:

public function environmentVariables(): array
    {
        return [
            'AWS_ACCESS_KEY_ID' => $this->config['key'],
            'AWS_SECRET_ACCESS_KEY' => $this->config['secret'],
        ];
    }

I purposely left out the key / secret and only used the profile. You can set the profile on the CLI and it will map to the credentials w/o passing them through the build process.

[nexxai]

Really hoping this can get merged soon

@mpociot or @simonhamp - can one of you look at this? Without the methods in this PR, you cannot build for S3.

[simonhamp]

This looks fine to me, but I must confess I know very little about S3 and don't have an account to test on.

I'm also not aware of how the electron-builder module expects these parameters.

It might be nice to add some comments to the Updater interface explaining how these parameters are used, as it feels like more drivers for this could come in the future.

[saeedvaziry]

Facing this error with these changes

Undefined array key "acl"

[lonnylot]

Facing this error with these changes

Undefined array key "acl"

See this PR: NativePHP/laravel#90

[nexxai]

The KEY and SECRET are definitely available and so they should probably be used for consistency purposes. Not everyone will have a profile set up (think: CI runners who are just building the app and have the key and secret injected at run time).

[lonnylot]

The KEY and SECRET are definitely available and so they should probably be used for consistency purposes. Not everyone will have a profile set up (think: CI runners who are just building the app and have the key and secret injected at run time).

The KEY and SECRET were specifically not used because they were compiled in to the release. They can all be added back in as optional params, but I wouldn't recommend taking the risk.

This PR is approved days ago so I'm not really sure how to move forward.

Any guidance would be appreciated.

[nexxai]

They've been approved but nothing has happened yet, so don't feel locked in place. If there's room for improvement, take the opportunity. Nothing has been merged so until that happens, make all the changes you want.

[simonhamp]

NativePHP/laravel#90 was closed as it doesn't feel like the right solution to that problem.

As long as your AWS_* env keys are passed either via the command line or stored in your .env file, they will not be compiled into your application.

Also worth noting that Electron's native updater has a default acl of public-read, which is what NativePHP uses under the hood as I understand it (I'm still learning Electron).

@lonnylot Please could you clean up this PR when you get a chance?

[simonhamp]

See previous comment

[lonnylot]

@simonhamp I'll get to it ASAP, but I'm fairly busy now. Wouldn't be offended if someone else made the adjustments.