Add GitLab export diagnostics command - #1607
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Unit Test Results 1 files 1 suites 25s ⏱️ Results for commit 2e0fca3. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
API coverage and export-job diagnostics must be corrected before approval; release notes are also missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 3
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
src/Octoshift/Services/GitlabApi.cs — The handler tests mock both new API methods, so they do not verify either endpoint URL,… |
|
src/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandHandler.cs — This value is the project ID, not an export-job ID: GitLab's export-status entity inherits id… |
|
src/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandHandler.cs — import_state describes an import into GitLab, so this command can return an unrelated import JID… |
 |
src/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommand.cs — This introduces a customer-facing CLI command, but RELEASENOTES.md is unchanged. The repository… |
What changed in this PR
Adds gl2gh diagnose-gitlab-export to generate Markdown diagnostics for failed GitLab exports.
Changes:
- Adds command options, validation, reporting, and overwrite handling.
- Adds GitLab project/export API retrieval.
- Adds command and handler unit tests.
| File | Summary and review |
|---|---|
src/OctoshiftCLI.Tests/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandTests.cs |
Tests command options and API construction. |
src/OctoshiftCLI.Tests/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandHandlerTests.cs |
Tests report creation and overwrite protection. |
src/OctoshiftCLI.Tests/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandArgsTests.cs |
Tests required argument validation. |
src/Octoshift/Services/GitlabApi.cs |
Adds project/export retrieval. Moderate: Add focused API tests for URLs, path encoding, and response mapping. |
src/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandHandler.cs |
Builds the report. Moderate: “Export ID” is actually the project ID; the administrator query also uses unrelated import_state instead of the export job. |
src/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommandArgs.cs |
Defines and validates command arguments. |
src/gl2gh/Commands/DiagnoseGitlabExport/DiagnoseGitlabExportCommand.cs |
Defines the CLI command. Nit: Add the required RELEASENOTES.md entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ); | ||
| } | ||
|
|
||
| public virtual async Task<GitlabExportDetails> GetExportDetails(string groupPath, string projectPath) |
| builder.AppendLine($"- Export ID: {ValueOrUnknown(exportDetails.Id)}"); | ||
| builder.AppendLine(); |
| builder.AppendLine("Run these commands on the GitLab instance to retrieve the server-side export job error. GitLab does not expose these logs through the project export API."); | ||
| builder.AppendLine(); | ||
| builder.AppendLine("```bash"); | ||
| builder.AppendLine($"sudo gitlab-rails runner \"p = Project.find_by_full_path({quotedProjectPath}); puts p.import_state.slice(:jid, :status, :last_error)\""); |
| name: "diagnose-gitlab-export", | ||
| description: "Collects GitLab project export diagnostics and writes a report with GitLab admin log commands.") |
brianaj
left a comment
brianaj
left a comment
There was a problem hiding this comment.
took a peak since this popped up on my radar cutting a new release today, leaving early feedback before you get too far
TL;DR
I'd lean away from a new diagnose-gitlab-export command. I think the real value is a clearer error at the existing failure point that links to GitLab's own import/export docs — most of the benefit, without baking GitLab's server internals (and their upkeep) into our CLI. Separately, it's worth following up with GitLab to improve export-failure visibility at the source.
Context worth keeping in mind: this repo is maintained by a small team without dedicated CLI-support resources, so I'd weight the ongoing maintenance cost of anything we add fairly heavily.
Concerns with the command as proposed
- Maintenance burden. It hardcodes GitLab server internals (
/var/log/gitlab/...,gitlab-ctl,gitlab-rails runner, a version-specific Ruby snippet) — things we'd have to keep correct as GitLab changes. - Wrong for many installs. Those paths assume an omnibus install; they don't fit Kubernetes/Helm, Docker, source, or GitLab.com SaaS (no shell access), yet the command emits them regardless of install type.
- Some guidance is already off (flagged by Copilot review):
import_stateis the import model, not export (can return an unrelated JID ornil; exports useProject#export_jobs), and "Export ID" is really the project ID. - Would fail CI as-is: an "unnecessary using directive" warning breaks the build under
TreatWarningsAsErrors=true. - Test gap: no
GitlabApiTestsfor the newGetExportDetails/GetProjectDetails. - Missing
RELEASENOTES.mdentry. Nice touches worth keeping:[Secret]on the PAT,ShellQuoteescaping,--output/--overwriteconventions.
Do we need a new command?
Probably not. A user shouldn't have to find and run a second command to learn why the first one failed — this reads more like an error-messaging gap on the existing migrate path than a missing tool.
For GitHub migrations: GetMigration returns a failureReason surfaced in the gei/ado2gh/bbs2gh and WaitForMigration handlers — but only after a GitHub migration ID exists. For the GitLab export failure, there's nothing to log: GitlabApi.GetExport returns only (ExportStatus, DownloadUrl), and GitLab's export API (/projects/:id/export) doesn't expose a reason — it just reports export_status: failed. That gap is exactly why this PR reached for server-side logs, and it's the strongest argument for raising this with GitLab so the reason is available via the API rather than us inferring it.
Proposed change (no new command)
Location: src/gl2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs (in GenerateArchive, the ExportState.IsError starement).
Before
if (ExportState.IsError(exportState))
{
throw new OctoshiftCliException($"GitLab archive export failed!");
}After
if (ExportState.IsError(exportState))
{
throw new OctoshiftCliException(
$"GitLab reported the project export for {args.GitlabGroup}/{args.GitlabProject} as '{exportState}'. " +
"GitLab's export API does not provide a failure reason, and no GitHub migration was created, so there are no migration logs to download. " +
"Ask a GitLab administrator to inspect the project export job on your GitLab instance. " +
"See GitLab's project import/export documentation for details: https://docs.gitlab.com/ee/user/project/settings/import_export");
}Ideally we should link to GitLab's official docs, not our own (please confirm the exact URL/anchor)
Suggested follow-up with GitLab
The root issue is that GitLab's project-export API doesn't surface a failure reason. Worth having customers raise with GitLab (issue/support) so export failures expose a machine-readable reason we can relay directly — which would make this whole class of problem self-explanatory without either a bespoke command or a docs hunt.
Happy to be discussed over slack as well. Maybe we can discuss with product for unofficially supported commands which fall into the grey area.
Collect bounded export-job and server-log evidence over verified OpenSSH, with optional Docker execution and API-only fallback reports. Update documentation, release notes, and focused tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep previously released notes in their archived file and retain the new GitLab diagnostics note in RELEASENOTES.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| public void Rejects_Missing_Key_File() | ||
| { | ||
| var args = Args(); | ||
| args.SshKey = Path.Combine(_key, "missing"); |
|
|
Thanks for the follow up @brianaj ! I've been getting a few requests lately from users who are seeing failures pre-migration id generation, from the GitLab export. My goal is to try to reduce the barrier for users to discover these errors. I'm fine with with keeping the diagnostics and log fetching outside of GEI, and creating a separate gh cli extension instead. I do think there would be value in having it built into GEI to reduce the friction in discovering these errors. |
ThirdPartyNotices.txt(if applicable)Summary
Adds a
gl2gh diagnose-gitlab-exportcommand that collects API-visible GitLab project export diagnostics and writes a Markdown report. Optional administrator SSH access collects retained export jobs, child-job errors and project-matched server logs from Linux-package GitLab installations, directly or inside Docker.This is intended for failures where GitLab reports
export_status: failedbefore a GitHub migration ID is created, sodownload-logscannot retrieve GitHub-side migration logs yet.Validation
dotnet format src/OctoshiftCLI.slndotnet test src/OctoshiftCLI.Tests/OctoshiftCLI.Tests.csproj --filter 'FullyQualifiedName~DiagnoseGitlabExport|FullyQualifiedName~GitlabApiTests|FullyQualifiedName~GitlabSshDiagnosticsCollector'— 35 tests passed.Notes
RELEASENOTES.md.SSH.NETNU1903 advisory inbbs2gh; this change does not add or update that dependency.Example output: administrator SSH diagnostics
This is an excerpt from an actual isolated GitLab CE 18.3.1 run using synthetic projects, not customer data. Making one project's temporary export directory unwritable produced a real export failure. The API exposed only
export_status: failed; administrator SSH collection retrieved the failed export/child jobs, concrete error and backtrace.Invocation
The GitLab PAT is supplied through
GITLAB_PAT. The SSH key authenticates an OS administrator, not GitLab's Git-over-SSH endpoint.For GitLab inside Docker on the SSH host, add
--gitlab-container NAME. Omit it when SSH already reaches the GitLab installation. OpenSSH host-key verification remains enabled.Terminal output
Selected actual lines; startup configuration and verbose HTTP responses are omitted:
Generated Markdown report
Summary excerpt:
The server-side section contains JSON like the following. This is a selected subset of the actual output: one failed relation/log entry is retained, the backtrace is shortened, and long temporary paths are replaced with
<export-directory>for readability. Status3is the failed state on the tested GitLab version.{ "collected_at": "2026-09-10T18:40:51Z", "gitlab_version": "18.3.1", "project_id": 1, "project_path": "ssh-fixture-lab/permission-denied-export", "export_jobs": [ { "id": 1, "jid": "f75e42d9bb968c5af2b60261", "status": 3, "user_id": 1, "created_at": "2026-09-10 18:30:38 UTC", "updated_at": "2026-09-10 18:31:51 UTC", "relations": [ { "relation": "wiki_repository", "jid": "ea4c6460583e02bb57a7e836", "status": 3, "export_error": "Permission denied @ dir_s_mkdir - <export-directory>" } ] } ], "logs": [ { "path": "/var/log/gitlab/gitlab-rails/exporter.log", "matches": [ { "time": "2026-09-10T18:31:31.597Z", "severity": "ERROR", "message": "Project relation export failed", "relation": "wiki_repository", "project_export_job_id": "1", "export_error": "Permission denied @ dir_s_mkdir - <export-directory>", "exception.class": "Errno::EACCES", "exception.backtrace": [ "/opt/gitlab/embedded/lib/ruby/3.2.0/fileutils.rb:403:in `mkdir'", "/opt/gitlab/embedded/lib/ruby/3.2.0/fileutils.rb:403:in `fu_mkdir'", "/opt/gitlab/embedded/lib/ruby/3.2.0/fileutils.rb:384:in `block (2 levels) in mkdir_p'" ] } ] } ] }The full report also retains project statistics, raw export API data, administrator follow-up commands, collection scope and explicit warnings. This run reported a missing
exceptions_json.logand bounded/truncated Sidekiq results rather than silently implying complete collection.Interpretation: unlike the API-only
failedstatus, these records identify a filesystem permission error and the affected export relation. This establishes the cause of the synthetic failure, not the cause of any customer incident. Reports can contain sensitive data and must be reviewed/redacted before sharing.