[cherry-pick] fix: minimal fix for airgap sample trust and factory URL source comparison#1568
Merged
Conversation
Contributor
|
Click here to review and test in web IDE: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: olexii4 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…rison (#1567) * fix: minimal fix for airgap sample trust and factory URL source comparison Four targeted changes to fix workspace source comparison and airgap sample trust for factory flows like: /load-factory?url=https%3A%2F%2Fregistry.devfile.io%2F...&storageType=per-user 1. Preferences/helpers.ts — automatically trust airgap sample URLs (matching /dashboard/api/airgap-sample/devfile/download) so the factory flow proceeds without requiring an explicit trust grant. Minor refactor: extract urlString before early-return guards. 2. ImportFromGit/helpers.ts — rename REVISION import to REVISION_ATTR (both are 'revision', so no functional change; aligns with the canonical constant name used in buildFactoryParams.ts). 3. buildFactoryParams.ts — apply decodeURIComponent() in getSourceUrl() so factoryParams.sourceUrl matches workspace.source. The factory hash double-encodes airgap URLs (%253F → %3F after first URLSearchParams decode), while workspace.source applies a second decode via URLSearchParams.get() (%3F → ?). Without this normalization the two sides differ and CheckExistingWorkspaces always creates a new workspace. 4. workspace-adapter/index.ts — replace split('&') + split('=')[1] with URLSearchParams.get('url') to extract the URL from the DevWorkspace annotation. The old approach truncated the value at the second '=' sign, breaking URLs whose query parameters contain '=' characters (e.g. base64-encoded values). URLSearchParams.get() also applies the same percent-decoding as point 3, keeping both sides consistent. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * fix(typo): rename devfileSourse → devfileSource in workspace-adapter "Sourse" is a misspelling of "Source". Rename the two local variables devfileSourseStr and devfileSourse to devfileSourceStr and devfileSource. No functional change. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * chore(license): add jsbn@0.1.1 to dev dependency excludes Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * fix: GitRepo component appends factory params to the Git repo URL (CRW-9659) The GitRepoFormGroup.getSource() method parsed the DevWorkspace factory annotation with split('&') + split('=') and then reconstructed the URL by appending every other factory param (che-editor, storageType, …). For airgap sample URLs this produced a malformed "Git repo URL" like: "…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user" Two bugs in the old parser: 1. split('=')[1] truncated the URL value at the second '=' sign. 2. All non-url params were re-joined and appended to the URL, adding factory orchestration params that have no place in the Git repo URL. Fix: replace the parser with URLSearchParams.get('url'). URLSearchParams.get() correctly handles '=' inside values (e.g. ?id=foo) and decodes percent-encoded characters (%3F → ?, %3D → =) so the URL shown in the Overview tab matches the clean devfile download URL. Add regression test: airgap sample URL with che-editor and storageType in the factory params — verifies that only the decoded 'url' value appears and factory params are never appended. Also stage a CSS line-wrap format fix in SamplesList/Gallery/Card. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * fix: GitRepo URL truncated and double-'?' for airgap factory params (CRW-9659) The GitRepoFormGroup.getSource() method used split('=')[1] to extract the 'url' value from the factory annotation params string. For airgap sample URLs whose query string contains '=' characters (e.g. %3Fid%3Dnodejs-express, decoded by URLSearchParams to ?id=nodejs-express) this truncated the value at the second '=' and then appended other factory params with '?' as separator, producing a malformed double-'?' URL: "…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user" ^ missing =nodejs-express ^ wrong separator Fix: use URLSearchParams.get('url') to extract the full URL value. URLSearchParams handles '=' inside values correctly and decodes percent-encoded characters (%3F→?, %3D→=). The remaining factory params (editor-image, storageType, etc.) are still appended to represent the full workspace config; the separator is chosen based on whether the URL already has a '?'. Add regression test that verifies: - No double '?' in the resulting URL - Decoded query value (id=nodejs-express) is preserved - Factory params are appended with '&', not a second '?' Also commit a CSS line-wrap format fix in SamplesList/Gallery/Card. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> --------- Signed-off-by: Oleksii Orel <oorel@redhat.com>
|
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1568 (linux/amd64, linux/arm64) kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1568", name: che-dashboard}]}}]" |
Contributor
|
/retest |
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
3 participants
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.
What does this PR do?
This PR fixes related issues that caused the "Create New" workspace policy to be ignored
when creating workspaces from airgap samples.
cherry-pick of #1567 to the 7.117.x branch
What issues does this PR fix or reference?
fixes https://redhat.atlassian.net/browse/CRW-9659
Is it tested? How?
Release Notes
Docs PR