Fix accidental enable of mattermost by deprecation check

What does this MR do?

Fixes bundled Mattermost being enabled on gitlab-ctl reconfigure for users who never configured it, introduced by the 19.0 Mattermost removal deprecation check.

Gitlab::Deprecations.remove_mattermost reads the config with plain []:

config = existing_config['mattermost']
return [] unless config.is_a?(Hash)
return [] unless config['enable'] == true

On a real run existing_config is Gitlab['node'].normal, a Chef::Node::VividMash, where reading a missing key creates it. And because package::config does Gitlab[:node] = node, that object is the attributes of the node being converged — so the read writes to them.

Which of the two reads misses is the whole story. GitlabMattermost.parse_secrets populates the mattermost block on every reconfigure (but never enable), so mattermost is always present and only enable is missing. config['enable'] therefore creates enable => {}, which is truthy and outranks default['mattermost']['enable'] = false, and gitlab::default selects mattermost::enable. Vivifying mattermost itself would be harmless — the merge falls back to the default when no enable key exists — so fixing only the outer read changes nothing on a real install.

This affects every platform, but only fails the pipeline on arm64, where the bundled binary cannot start:

  • 18.11.7+ee.0 arm64 (job 37522486): Recipe: mattermost::disable
  • 18.11.8+ee.0 arm64 (job 38095068): Recipe: mattermost::enable, then runit_service[mattermost] ... Expected process to exit with [0], but received '1'

Fix

Use dig, the non-mutating accessor, for both reads — matching every other helper in this file, which already reads via dig or guards with key?:

config = existing_config.dig('mattermost') # rubocop:disable Style/SingleArgumentDig
return [] unless config.is_a?(Hash)
return [] unless config.dig('enable') == true # rubocop:disable Style/SingleArgumentDig

The rubocop:disable comments are required: that cop assumes [] and single-argument dig are interchangeable, which is precisely the assumption that fails on a VividMash. is_a?(Hash) and == true are kept — gitlab-ctl check_config passes a plain Hash where mattermost can be absent, and ConfigMash fills root blocks with empty hashes, so a truthiness test would fire on {}. Users who do enable Mattermost still get the 19.0 removal message.

Verification

gitlab-ee 18.11.8-ee.0 on Ubuntu arm64, gitlab.rb containing only external_url:

  • as shipped → mattermost::enable, run fails
  • outer read using dig only → mattermost::enable, run fails (also with /opt/gitlab/embedded/nodes/*.json removed first, ruling out stale cached attributes)
  • both reads using digRecipe: mattermost::disable, runit_service[mattermost] action disable, exit 0

Verified on live ARM instance !9650 (comment 3649484460), and on Linux machine using 18.11.8 docker image and hot patching the code from MR

Issue

Mattermost recipe is enabled by default (#10036 - closed)

Edited by Nailia Iskhakova

Merge request reports

Loading