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

Jump to bottom

recreate all webserver config on updates #17225

Merged
merged 1 commit into from Feb 22, 2021
Merged

recreate all webserver config on updates #17225

merged 1 commit into from Feb 22, 2021

Conversation

Findus23
Copy link
Member

It seems like the full createFilesForSecurity that also creates the files for IIS is only run on install and some updates and not on the normal update.

➜  ~/public_html/matomo git:(4.x-dev) ✗ rg "createFilesForSecurity"
core/Updates/4.0.0-b1.php
261:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/3.13.4-b1.php
21:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/3.0.0-b4.php
56:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/2.16.3-b1.php
20:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/3.0.1-b1.php
22:        ServerFilesGenerator::createFilesForSecurity();

plugins/Installation/Controller.php
313:        ServerFilesGenerator::createFilesForSecurity();

plugins/Installation/ServerFilesGenerator.php
17:    public static function createFilesForSecurity()

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@sgiehl
Copy link
Member

sgiehl commented Feb 17, 2021

@Findus23 When doing that in updater, guess we could remove it from the single update scripts. Wouldn't make sense to do it multiple times when updating from an older version.

@Findus23
Copy link
Member Author

I'm not sure if modifying historic update scripts is worth it if the worst case is creating the same file twice after each other.

@sgiehl
Copy link
Member

sgiehl commented Feb 17, 2021

Don't have a strong opinion on that. That just came up to my mind when reviewing... It would only remove some unneeded operations. But should also be fine to keep them 🤷

@tsteur
Copy link
Member

tsteur commented Feb 17, 2021

Be fine to keep them as the operation is quickly done and not happening too often

@diosmosis diosmosis added this to the 4.2.0 milestone Feb 18, 2021
@mattab mattab modified the milestones: 4.2.0, 4.3.0 Feb 22, 2021
@diosmosis diosmosis merged commit 2e238b0 into 4.x-dev Feb 22, 2021
@diosmosis diosmosis deleted the recreate-all-webserver-config-on-update branch February 22, 2021 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diosmosis diosmosis diosmosis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.3.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Findus23 @sgiehl @tsteur @diosmosis @mattab