bringyourownideas / silverstripe-maintenance

Helps with the day by day work to run a SilverStripe application or website.
https://bringyourownideas.com
BSD 3-Clause "New" or "Revised" License
31 stars 10 forks source link

Can't use module on SC platform #142

Open brynwhyman opened 4 years ago

brynwhyman commented 4 years ago

Background

It looks like the same issue was previously resolved for the CWP platform, and while there are notes explaining the fixes were tested on SC, we've had reports of issues

Related issues:

Further information

We've had an internal report of this here: https://silverstripeltd.slack.com/archives/CLXKD9X51/p1583114132100800

brynwhyman commented 4 years ago

Excuse the very much internal reference in an open source issue - but given that this is likely installed on a number of projects in on the Silverstripe Cloud platform, I'm hoping we can use it to elicit some feedback from others.

brynwhyman commented 4 years ago

I count this module being used on 24 production environments on our own SC platform.

rafaeldsousa commented 4 years ago

I believe what I'm experiencing is related, just started happening for one of my SC sites. Error: E_WARNING: chdir(): Permission denied (errno 13)

SS had a look and could trace it down to this module, but couldn't tell me why the permission error was happening.

From SS Odd, this is part of the UpdatePackageInfoTask in the silverstripe-maintenance module and its coming up during the queued job run but the task is not being run.

https://github.com/bringyourownideas/silverstripe-composer-update-checker/blob/master/src/Extensions/ComposerLoaderExtension.php#L111-L127

https://github.com/bringyourownideas/silverstripe-maintenance/blob/d1435bbec086fae937924b62a8514f2f79aaabeb/src/Util/ComposerLoader.php#L65

brynwhyman commented 4 years ago

Hey @rafaeldsousa, what version of this module (bringyourownideas/silverstripe-maintenance) and bringyourownideas/silverstripe-composer-update-checker are you running?

rafaeldsousa commented 4 years ago

Hi @brynwhyman bringyourownideas/silverstripe-maintenance - 2.3.1 Screen Shot 2020-03-12 at 3 33 48 PM

bringyourownideas/silverstripe-composer-update-checker - 2.0.3 Screen Shot 2020-03-12 at 3 34 11 PM

andrewandante commented 3 years ago

Just to add - I've just seen this behaviour. It breaks all dev/tasks for me, because composer isn't installed on the SCP servers. This means that neither HOME or COMPOSER_HOME are defined, and it throws a server error:

error-log.ERROR: Uncaught Exception RuntimeException: "The HOME or COMPOSER_HOME environment variable must be
set for composer to run correctly" at /var/www/mysite/releases/63e07fc364b9f90ab583e83a6f04bb72a26faf20/vendor
/composer/composer/src/Composer/Factory.php line 652 {"exception":"[object] (RuntimeException(code: 0): The HOME 
or COMPOSER_HOME environment variable must be set for composer to run correctly at /var/www/mysite/releases
/63e07fc364b9f90ab583e83a6f04bb72a26faf20/vendor/composer/composer/src/Composer/Factory.php:652)"}
brynwhyman commented 3 years ago

@michalkleiner shared a theory in a duplicate issue, thinking the usage of the syntax in the prior fix might be off.

See: https://github.com/silverstripe/cwp-recipe-basic/issues/4#issuecomment-828146426

GuySartorelli commented 2 years ago

I believe this should be closed - seems to be working fine now.

michalkleiner commented 2 years ago

@GuySartorelli doesn't work for us on CWP nor on AWS with private repos due to some permission issues. If this module is an umbrella module for the security checker and update checker, then the issues still exist.

GuySartorelli commented 2 years ago

with private repos

Is that to say that if you had no private repositories it works fine? If so that would be the difference I think - the project I saw it working with today doesn't have any private repositories.

The other difference I guess being that I was testing the beta at the time, which uses the new major release of composer-update-checker.

michalkleiner commented 2 years ago

Actually not even private repos, but forks that have entries under repositories key and use a custom dev-branch instead of a tagged version from Packagist.

GuySartorelli commented 2 years ago

Okay. So there are situations where it doesn't work. Does it work for you with the "happy path" i.e. no custom branches or repository replacements or anything, just 1:1 public packagist entries?

michalkleiner commented 2 years ago

I don't think we even have a project like that on SCPS/AWS, there's always something that requires a fork in the plethora of modules we use.

GuySartorelli commented 2 years ago

Fair enough. I'll try make some time to do some experimentation.

rafaeldsousa commented 2 years ago

Any updates on this ? still happens a lot on SC. For now the only thing we've able to do was to ignore the error permanently so that our logs don't get flooded with entries of this.

michalkleiner commented 2 years ago

@andrewandante might get someone from his team to look into this maybe? :)

andrewandante commented 2 years ago

Before I do... 😉

Just want to confirm exactly what the issue is that we are trying to validate.

Seems that the point of this module is to tell the user when there are updates available for the composer packages installed.

It's currently not working on SC boxes because neither HOME or COMPOSER_HOME is set? Is that still true? If so, is a workaround to add one of those as an environment variable?

Or have we now pivoted to "it doesn't work if there are private repos or we are using a fork"? In this case, what's the expected behaviour - for a private repo, it should simply skip it? For a fork, are we expecting notices about the main repo, or the fork, or just to skip it as well?

It feels to me like these can be handled in the module code, rather than through server configuration - if it should work with the above two scenarios, but is relying on composer-based auth, then it's never going to work on SC, because we won't be adding composer to the web boxes just for this. I'd advocate for configuration along the lines of skip_forks: true and skip_private: true that should be toggled for SC.

Always open to discussions, let me know your thoughts. ❤️

rafaeldsousa commented 2 years ago

@andrewandante Personally I've only experienced the Error: E_WARNING: chdir(): Permission denied (errno 13) error, which relates to Michal's comment above. So either having private repos or non packagist tagged modules will trigger this issue. I guess for this scenario having the ability to skip_forks and skip_private repos could work.

andrewandante commented 2 years ago

Looks like the above is being attempted at #170

rafaeldsousa commented 1 year ago

@GuySartorelli anyone looking at your PR above #170? Logging channels still keep being flooded with permission error messages, if not manually ignored. Would be good to have this merged and allow us to ignore the private repos.

GuySartorelli commented 1 year ago

@rafaeldsousa Thanks for bringing this back to my attention. It's been added to our backlog to review.