Fix non-working migration instructions#9017
Conversation
``` root@gitlab ~ # sudo gitlab-psql -- id psql: error: connection to server on socket "/var/opt/gitlab/postgresql/.s.PGSQL.5432" failed: FATAL: Peer authentication failed for user "id" root@gitlab ~ # sudo -u gitlab-psql -- id uid=998(gitlab-psql) gid=1003(gitlab-psql) groups=1003(gitlab-psql) ``
|
Hello @tpo, Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here. Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project. Please help complete the Mattermost contribution license agreement? This is a standard procedure for many open source projects. Please let us know if you have any questions. We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team. |
📝 WalkthroughWalkthroughDocumentation update for GitLab Omnibus Mattermost migration: the PostgreSQL database dump command syntax was corrected to use ChangesGitLab Omnibus Migration Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/administration-guide/onboard/migrate-gitlab-omnibus.rst (1)
73-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid documenting an insecure DB TLS default without environment scoping
"sslmode=disable"normalizes an insecure production posture unless explicitly scoped to local/test-only usage. Please provide a secure default (for examplerequire/verify-full) and clearly mark anydisableexample as non-production only.Suggested minimal diff
- "DataSource": "postgres://mmuser:password@new-postgres-server:5432/mattermost?sslmode=disable&connect_timeout=10" + "DataSource": "postgres://mmuser:password@new-postgres-server:5432/mattermost?sslmode=require&connect_timeout=10"As per coding guidelines, documentation must not normalize insecure defaults and must distinguish test shortcuts from production security guidance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/onboard/migrate-gitlab-omnibus.rst` at line 73, Replace the insecure example value for "DataSource" that sets sslmode=disable with a secure default (e.g., sslmode=require or sslmode=verify-full) and explicitly annotate any sslmode=disable example as "for local/testing only" so readers know it is non-production; update the "DataSource" example string and the surrounding explanatory text to show the secure default and include a short note clarifying that disabling TLS is only for test/dev environments.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/onboard/migrate-gitlab-omnibus.rst`:
- Line 40: The dump step writes a date-stamped file named
mattermost_dbdump_$(date --rfc-3339=date).sql.gz but the restore step expects
/tmp/mattermost_dbdump.sql.gz; update the instructions so the filenames/paths
match by either changing the dump command to write /tmp/mattermost_dbdump.sql.gz
(remove the date suffix and set /tmp/) or change the restore step to point at
the generated mattermost_dbdump_YYYY-MM-DD.sql.gz filename; ensure the example
uses the exact same filename token (mattermost_dbdump_$(date
--rfc-3339=date).sql.gz) or a fixed /tmp/mattermost_dbdump.sql.gz in both steps
for clarity and reliability.
---
Outside diff comments:
In `@source/administration-guide/onboard/migrate-gitlab-omnibus.rst`:
- Line 73: Replace the insecure example value for "DataSource" that sets
sslmode=disable with a secure default (e.g., sslmode=require or
sslmode=verify-full) and explicitly annotate any sslmode=disable example as "for
local/testing only" so readers know it is non-production; update the
"DataSource" example string and the surrounding explanatory text to show the
secure default and include a short note clarifying that disabling TLS is only
for test/dev environments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31413700-d0e1-4c06-84a7-4271b6b46824
📒 Files selected for processing (1)
source/administration-guide/onboard/migrate-gitlab-omnibus.rst
| .. code-block:: bash | ||
|
|
||
| sudo gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql --no-owner mattermost_production | gzip > mattermost_dbdump_$(date --rfc-3339=date).sql.gz | ||
| sudo -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql --no-owner mattermost_production | gzip > mattermost_dbdump_$(date --rfc-3339=date).sql.gz |
There was a problem hiding this comment.
Fix dump filename/path mismatch with restore step
The dump command writes mattermost_dbdump_YYYY-MM-DD.sql.gz in the current directory, but Step 3 restores /tmp/mattermost_dbdump.sql.gz. Following this as written can fail at restore time.
Suggested minimal diff
- sudo -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql --no-owner mattermost_production | gzip > mattermost_dbdump_$(date --rfc-3339=date).sql.gz
+ sudo -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql --no-owner mattermost_production | gzip > /tmp/mattermost_dbdump.sql.gzAs per coding guidelines, procedural steps must be clear, atomic, and likely to work as written for novice administrators.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sudo -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql --no-owner mattermost_production | gzip > mattermost_dbdump_$(date --rfc-3339=date).sql.gz | |
| sudo -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql --no-owner mattermost_production | gzip > /tmp/mattermost_dbdump.sql.gz |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/administration-guide/onboard/migrate-gitlab-omnibus.rst` at line 40,
The dump step writes a date-stamped file named mattermost_dbdump_$(date
--rfc-3339=date).sql.gz but the restore step expects
/tmp/mattermost_dbdump.sql.gz; update the instructions so the filenames/paths
match by either changing the dump command to write /tmp/mattermost_dbdump.sql.gz
(remove the date suffix and set /tmp/) or change the restore step to point at
the generated mattermost_dbdump_YYYY-MM-DD.sql.gz filename; ensure the example
uses the exact same filename token (mattermost_dbdump_$(date
--rfc-3339=date).sql.gz) or a fixed /tmp/mattermost_dbdump.sql.gz in both steps
for clarity and reliability.
Source: Coding guidelines
2 participants
Summary
Fix not working instructions:
sudo user -- commanddoesn't do what the author thought it does.