Skip to content

fix: web updater is now disabled by default + discourage users to use the web updater - #41385

Merged
DeepDiver1975 merged 1 commit into
masterfrom
fix/disable-web-updater-by-default
Jul 3, 2025
Merged

DeepDiver1975 merged 1 commit into
masterfrom
fix/disable-web-updater-by-default

Conversation

@DeepDiver1975

@DeepDiver1975 DeepDiver1975 commented Jul 2, 2025 •

Copy link
Copy Markdown
Contributor

Description

The web updater is a relic of old times and can cause more trouble than help.
More details on #41149

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@update-docs

update-docs Bot commented Jul 2, 2025

Copy link
Copy Markdown

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@jvillafanez

Copy link
Copy Markdown
Member

I'll probably need some additional context...

If we want to just disable the web updater, adding a migration to set the web-updater.enabled config to false should be enough. However, it seems we're doing more than that.

I assume we don't want people to run the web-updater.
What is the behavior when the web-updater is disabled? I guess it will just show a message telling people that a new version is available. However, what if it's enabled? What is the difference between having the web-updater enabled or disabled?

/**
* @return DataResponse
*/
public function createCredentials() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

['name' => 'Admin#createCredentials', 'url' => '/credentials', 'verb' => 'GET'],
should also be removed.

@DeepDiver1975
DeepDiver1975 force-pushed the fix/disable-web-updater-by-default branch 2 times, most recently from d613006 to 5544551 Compare July 2, 2025 13:12
@DeepDiver1975 DeepDiver1975 changed the title fix: web updater is now disabled by default fix: web updater is now disabled by default + discourage users to use the web updater Jul 2, 2025
@DeepDiver1975
DeepDiver1975 force-pushed the fix/disable-web-updater-by-default branch from 5544551 to 8fd1aba Compare July 2, 2025 14:02
@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2025

Copy link
Copy Markdown

@DeepDiver1975
DeepDiver1975 merged commit f1b00f4 into master Jul 3, 2025
@mmattel
mmattel deleted the fix/disable-web-updater-by-default branch July 3, 2025 10:07
oc-tmueller added a commit to owncloud/docs.owncloud.com that referenced this pull request Sep 18, 2026
The 10.16 page was stale: core added 'web-updater.enabled' to
config/config.sample.php in owncloud/core#41385 and the generated page was
never regenerated, so the parameter has been undocumented for the 10.16 line
ever since.

Regenerated with config-to-docs (`php convert.php config:convert-adoc`) from
core's 10.16 branch, which is the authoritative source for this page. The diff
is purely the one missing section; every other line already matched, and the
hand-written header above the "header end" marker is untouched.

Found while establishing a regeneration baseline for a different change - the
same run confirmed both config_apps_sample_php_parameters.adoc pages and the
11.0 config_sample_php_parameters.adoc are already in sync.

Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
phil-davis pushed a commit to owncloud/docs.owncloud.com that referenced this pull request Sep 22, 2026
The same staleness just fixed for 10.16 in #138: core added
'web-updater.enabled' to config/config.sample.php in owncloud/core#41385,
which is present on the 10.15 branch too, and the generated page was never
regenerated.

Regenerated with config-to-docs from core's 10.15 branch. The diff is the one
missing section, byte-identical to the block #138 added for 10.16, and the
hand-written header above the "header end" marker is untouched.

With this, all three published server versions regenerate to an empty diff
against their core branch, which is the precondition for checking that
automatically rather than discovering drift years later.

The same caveat as #138 applies: the pre-existing 'upgrade.disable-web'
comment a few lines above still claims the web updater is enabled by default,
which owncloud/core#41385 invalidated. That wants fixing in core, not here.

Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
phil-davis pushed a commit to owncloud/docs.owncloud.com that referenced this pull request Sep 22, 2026
* docs: sync config.sample.php parameters for 10.15

The same staleness just fixed for 10.16 in #138: core added
'web-updater.enabled' to config/config.sample.php in owncloud/core#41385,
which is present on the 10.15 branch too, and the generated page was never
regenerated.

Regenerated with config-to-docs from core's 10.15 branch. The diff is the one
missing section, byte-identical to the block #138 added for 10.16, and the
hand-written header above the "header end" marker is untouched.

With this, all three published server versions regenerate to an empty diff
against their core branch, which is the precondition for checking that
automatically rather than discovering drift years later.

The same caveat as #138 applies: the pre-existing 'upgrade.disable-web'
comment a few lines above still claims the web updater is enabled by default,
which owncloud/core#41385 invalidated. That wants fixing in core, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>

* ci: fail when the generated config pages drift from core

The two config parameter pages per server version are generated by
config-to-docs from core's config/config.sample.php and
config.apps.sample.php. Everything below the "header end do not delete or edit
this line" marker is machine output. Nothing enforced that, so a hand edit
there looked correct, rendered correctly, passed CI, and would have been
destroyed the next time anyone regenerated - and drift in the other direction
went unnoticed for two releases, which is what #138 and #142 just repaired.

This regenerates every version from its core branch and fails on any
difference, so the pages are provably generator output.

Nothing here infers success from an exit status, because convert.php exits 0 on
both of its own failure paths - unreadable input and unwritable output. Every
file is checked before use and the pages actually regenerated are counted and
asserted. Without that, a sample renamed in core would leave the pages
untouched, the comparison would find nothing, and the job would pass having
checked nothing at all - which is precisely the failure this workflow exists to
catch, one level up.

The comparison snapshots each page before regenerating and then plain-diffs the
two, rather than asking git. The pages are just files, and git inside this
container cannot see the workspace repository even though actions/checkout
populated it with git - which is worth knowing, but not worth depending on.

Other choices worth stating:

The loop is driven by the content/server/* directories themselves rather than a
version list, so a new server version cannot be added and silently left
unchecked. The core branch is resolved by name, falling back to master only for
the version still in development - and that fallback is verified against
master's own OC_VersionString, so a directory that matches no core branch fails
loudly instead of being regenerated from the wrong source.

git ls-remote's "no such branch" (exit 2) is distinguished from every other
failure. Conflating them would let a transient network error resolve 10.16 to
master and then fail the comparison for a baffling reason.

It is deliberately not path-filtered: a required check that is skipped reports
as perpetually pending and blocks merges, the trap already documented at length
in lint-pr-title.yml. The cost is that drift caused by a core commit surfaces
on an unrelated author's PR, so there is also a nightly run to find it first.

PHP comes from owncloudci/php:7.4 as a job container rather than a third-party
setup action, per the Actions policy in AGENTS.md. Inside a container the
default shell is sh, so bash is requested explicitly. The converter is
installed with `composer install`, not `update`: its comment parsing lives in
phpdocumentor/reflection-docblock, accepted as ^4.3.0 but pinned in the lock,
and floating it can change the generated output. The converter checkout itself
is deliberately unpinned - the question being asked is whether the pages match
what a maintainer running ctd.sh today would produce.

On failure the explanation and the full diff go to the job summary, and the
regenerated pages are uploaded as an artifact, so the fix can be downloaded
rather than copy-pasted out of a log.

Verified by running the job's own script in that container against four cases:
an in-sync tree passes and reports 6 pages across 3 versions; a hand edit below
the marker fails with the diff; an edit above the marker still passes, because
the generator preserves that region by design; and a version directory with no
corresponding core branch fails naming the mismatch.

Stacked on #142 - the check only goes green once 10.15 is in sync.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>

---------

Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Suggestion: Disable (Web-)Updater by default.

3 participants