fix: keep the GitLab repo scheme so http self-hosted instances can re-sync - #6294
Open
marmar9615-cloud wants to merge 1 commit into
Open
fix: keep the GitLab repo scheme so http self-hosted instances can re-sync#6294marmar9615-cloud wants to merge 1 commit into
marmar9615-cloud wants to merge 1 commit into
Conversation
…-sync resyncGitlab rebuilt the repo URL as "https:" plus the stored pathname, and the GitLab chunkSource never recorded the scheme, so a watched document from a self-hosted GitLab served over http was fetched over https on every sync run and un-watched after five failures. Store the scheme in the chunkSource payload and restore it on resync, defaulting to https for chunkSources written before this change, which is the convention the GitHub and Gitea connectors already use.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Type
Relevant Issues
resolves #6293
Description
A watched document imported from a self-hosted GitLab served over plain http never re-syncs.
resyncGitlabrebuilt the repo URL ashttps:plus the stored pathname, and the chunkSource payload had no scheme to restore. Every sync run of anhttp://gitlab.internal:8080/group/projectdocument requestedhttps://gitlab.internal:8080/..., the fetch failed, and after five failed runs the server dropped the document from the watched set.The import works over http, since the loader accepts either scheme and builds
apiBasefrom the URL origin. Only re-sync lost it:new URL("gitlab://http://host:8080/group/project?payload=...")readshttpas the host and leaves//host:8080/group/projectas the pathname, andresyncGitlabputhttps:in front of that.The fix is the convention #6117 added for GitHub and #6087 shipped with for Gitea, transposed to GitLab:
collector/utils/extensions/RepoLoader/GitlabRepo/index.js:generateChunkSourcestoresscheme: new URL(repo.repo).protocol.replace(":", "")in the encrypted payload. The function is now exported so the test can build a real chunkSource, asGithubRepo/index.jsdoes.collector/extensions/resync/index.js:resyncGitlabrebuilds the URL as`${source.searchParams.get("scheme") || "https"}:${source.pathname}`.ChunkSources written before this change have no
schemeand keep defaulting to https, so existing https documents are unaffected. A document already imported over http keeps its old chunkSource, still resyncs over https, and has to be re-imported once to pick the scheme up. #6117 has the same limitation for GitHub.Additional Information
collector/__tests__/utils/extensions/RepoLoader/GitlabRepo.test.jsis new: 24 tests in seven describe blocks, the same layout asGithubRepo.test.jsandGiteaRepo.test.js. Five run a chunkSource fromgenerateChunkSourcethrough a realEncryptionWorkerand the realresyncGitlab. Four of those assert that the URL the user entered is the URLfetchGitlabFilereceives on re-sync (https with a PAT, http on a port, a legacy payload withoutscheme, public gitlab.com). The fifth asserts the{ success: false, content: null }response on a failed fetch.Which case fails where, same test file each time:
eb7df1e8, both filesgenerateChunkSource, so the four tests that build one through the loader cannot run; the fifth builds its payload by hand and passes.resync/index.js, chunkSource change keptan http self-hosted host keeps its scheme and port through a resync.fetchGitlabFilereceivedhttps://gitlab.example.com:8080/acme/widgets.source.pathname//gitlab.example.com:8080/acme/widgets; the scheme is not in it.|| "https"defaulta chunkSource stored before the protocol was recorded still resyncs over https. The repo URL becamenull://gitlab.com/acme/widgets.yarn testfrom the repo root, three runs: 55 suites, 772 tests, against 54 suites and 748 tests on this base.server/__tests__/utils/helpers/modelPricing/index.test.jsfailed one of the three runs and passes on its own; it is untouched by this change.I did not run
yarn lintat the root, which also lints the frontend.eslintandprettier --checkare clean on the changed files; the collector eslint config ignores__tests__/**.Not in this PR:
resyncConfluencehas the samehttps:${source.pathname}line (resync/index.js:67) but fails elsewhere. Its requests go through the storedbaseUrl, which keeps its scheme, and eachdoc.metadata.urlis built from it (ConfluenceLoader/index.js:134).pageUrlis only compared,doc.metadata.url === pageUrl(Confluence/index.js:217), so on an http instance the pages load and the match fails with "Target page could not be found in Confluence space." That is a comparison fix, not a payload fix, so it belongs in its own change.Developer Validations
yarn lintfrom the root of the repo & committed changes