Skip to content

fix: parse import-url suffixes from right - #291

Open
cotishq wants to merge 2 commits into
microcks:masterfrom
cotishq:fix/import-url-parsing
Open

fix: parse import-url suffixes from right#291
cotishq wants to merge 2 commits into
microcks:masterfrom
cotishq:fix/import-url-parsing

Conversation

@cotishq

@cotishq cotishq commented May 5, 2026

Copy link
Copy Markdown
Contributor

Description

  • Fix import-url argument parsing so URLs containing : (scheme/ports/etc.) are preserved when using the optional :true|false[:secret] suffixes.
  • Add unit tests covering ports, paths, query strings, fragments, and secret suffix parsing.
  • Update import-url docs with the new parsing behavior and examples.
  • Make context_test create its testdata/ directory before writing the test config file (stabilizes test runs).

Before:

image

After the fix:

image

Related issue(s)

Fixes #260

@cotishq

cotishq commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

@Harsh4902 would love your opinion on this and what changes it still requires

@cotishq

cotishq commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@Harsh4902 , hey can you please look into it, like what changes it still requires,

@Vaishnav88sk

Copy link
Copy Markdown

@cotishq Please resolve conflicts and rebase it as overlapping with #465

@Vaishnav88sk

Copy link
Copy Markdown

@Harsh4902 Can we add it to Next milestone?

@Caesarsage

Copy link
Copy Markdown
Contributor

Thanks — you had the right idea! The parse-from-the-right fix landed via #465 (parseImportURLArg), so this is superseded. Note the same class of bug still exists for file paths in import (#352) if you want a follow-up target.

@Harsh4902

Copy link
Copy Markdown
Member

@Vaishnav88sk @Caesarsage we're keeping this open and will address it in the next release after 1.0.3.

@Harsh4902 Harsh4902 added component/cli keep-open Explicitily keep open kind/enhancement New feature or request labels Aug 19, 2026
@cotishq
cotishq force-pushed the fix/import-url-parsing branch from 914cb0e to 0d35e42 Compare August 19, 2026 15:08
@cotishq

cotishq commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@Vaishnav88sk rebased it and resolved the merge conflicts too

@Harsh4902

Copy link
Copy Markdown
Member

@cotishq You might have to make changes in you code as per the latest update in codebase and CI pipelines. I thinks that's why CI is failing.

@Vaishnav88sk

Copy link
Copy Markdown

Thanks @cotishq — I re-checked against master after the rebase. The import-url parsing is already fixed by #465 (which added parseImportURLArg doing the same right-side parse). So this PR's parseImportURLSpecifier + the importURL.go changes duplicate merged code — please drop that half (revert importURL.go, remove parseImportURLSpecifier and its tests, and the importURL.md suffix section).

The part that's still valuable and missing: parseImportFileSpecifier + the cmd/import.go change (the file-path : bug, #352). Keep that, and split the context_test.go MkdirAll tweak into its own PR. Once scoped down and CI green, this is good to merge.

Signed-off-by: cotishq <tanishqp101204@gmail.com>
Signed-off-by: cotishq <tanishqp101204@gmail.com>
@cotishq
cotishq force-pushed the fix/import-url-parsing branch from 7bafc60 to 6ce5e35 Compare August 21, 2026 17:34
@cotishq

cotishq commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

@Vaishnav88sk , @Harsh4902 Thanks for the review, i've scoped the PR down as requested:

  1. Dropped the parseImportURLSpecifier + importURL.go changes - reverted importURL.go to upstream (restoring parseImportURLArg), and removed parseImportURLSpecifier and its tests from artifact_specifier.go/test
  2. Kept parseImportFileSpecifier + the cmd/import.go refactor - the right-side parse fix for the file-path : bug (microcks import truncates Windows absolute paths when parsing :primary suffix #352)
  3. Reverted the context_test.go MkdirAll tweak

One note: policy-guards is currently failing on master for all branches due to pkg/util/rand/rand_test.go (introduced in #494) missing the copyright header
I've opened an issue (#506) for that seperately, please look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/cli keep-open Explicitily keep open kind/enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: import-url loses URL path/port when parsing :primary and secret suffixes

4 participants