Remediate potential pre-install bugs for sites with UTF-8 in node.json

Overview

We need to find a way to allow upgrades from the published 17.10.x series to the patched 17.10.y series.

See the great write up from @clemensbeck on how to reproduce the installation error: !8284 (comment 2434532844)

Context

The following discussion from !8284 (merged) should be addressed:

@clemensbeck As a result any user already on 17.10 with UTF chars in the node js will run into this error, even if he upgrades to a package with the fix of this MR.

AFAIK the only workaround for these user is to either:

  • change the systems locale settings,
  • patch the preinstall script,
  • patch the node js or the package files on the system.

I am approving this here, but we will need additional guidance/documentation for users on 17.10.

@rmarshall There is an alternative option to consider:

Apply the patch in the pre-install as it only impacts our own files if the version matches a case. We keep it in place and remove it after an upgrade stop.

Given that we:

  • control the files in question
  • the files are all code, not configuration

This limits risk and makes a smoother experience for the impacted customers.

@balasankarc , @WarheadsSE - what do you think?

@rmarshall Also, as another alternative:

The same logic that would detect and apply a patch could also simply stop installation and throw a link to documentation of the detected issue and steps to fix.

This would technically be safer, but a lesser customer experience.

@WarheadsSE I don't love pre-install performing this patch, but there may not be a better ways, outside of a means to force LANG to a UTF8 format inside of the call.

Can we split that outside of this change set, as a follow-up problem solution, rather than blocking this future-looking fix MR?

@balasankarc

force LANG to a UTF8 format

We can do that. We can call check_config with a specific UTF8 value for LANG - https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/82ba4498a6745a7554f4a693c8119e96b6b9bee1/config/templates/package-scripts/preinst.erb#L132.

preinst modifying files feels a bit icky to me. I don't know if we can even do that without dpkg complaining on next upgrade that someone modified the installed files.

Deliverables

  • Patch the pre-install to prevent failure to upgrade from 17.10.x to 17.10.y
  • Provide explicit test plan with GitLab versions where we can show that 17.10.x to 17.10.y fails, but 17.10.x to 17.10.y-patched succeeds
  • Define the communications plan to amplify this message louder in the release post for customer visibility