Skip to content

refactor: Parsing adr poc url#259

Draft
ashearin wants to merge 5 commits into
bomctl:mainfrom
ashearin:parsing-adr-poc-url
Draft

refactor: Parsing adr poc url#259
ashearin wants to merge 5 commits into
bomctl:mainfrom
ashearin:parsing-adr-poc-url

Conversation

@ashearin
Copy link
Copy Markdown
Member

@ashearin ashearin commented Jan 9, 2025

Description

Proof of concept for 2 items:

  • unified cmd url parsing
  • using net/url parsing to remove custom parsing code and rely on standard format

The URL standard is the closest to what bomctl currently uses and was the logical first choice for a Proof of Concept. The command URL is largely the same as currently utilized, however any custom elements are being removed and moved into a url compliant field. example below reworks this command url in the currently implemented format:
- https://username:password@github.com:12345/bomctl/bomctl.git@main#sbom.cdx.json

URL: [scheme:][//[userinfo@]host][/]path[?query][#fragment]

https://username:password@github.com:12345/bomctl/bomctl.git?ref=main#sbom.cdx.json

scheme: https
userinfo: [username, password]
host: github.com:12345 (parse port with url.Port())
path: bomctl/bomctl.git
fragment: sbom.cdx.json
Query: ref=main (parse query with url.Query() and get value with query.Get("ref"))

Main difference is the @main used to designate the branch is moved into the query field. Another option is removing it entirely and creating a branch flag and defaulting to the default branch if a branch is not provided (although this may only be applicable to the git client)

Optional improvements to what is changed, specifically in OCI client: (Can be implemented in separate issues)

  • Init function for each client that validates the given URL (Implemented)
  • Init function could also:
    • Replace functionality common to prepareFetch and preparePush
    • Ensure needed infrastructure is initialized, like createRepository in OCI client
  • Store target url in client implementation structs to avoid repeated url parsing and reduce function parameters
    • Store target ref in a similar fashion
  • In clients that utilizes (the git flavored ones), do not require the branch to be passed in and assume the user intends the default branch if it's not given by a user
    • potentially make a flag to specify a different flag

@ghost
Copy link
Copy Markdown

ghost commented Jan 9, 2025

Minder Vulnerability Report ✅

Minder analyzed this PR and found it does not add any new vulnerable dependencies.

Vulnerability scan of bae61b1c:

  • 🐞 vulnerable packages: 0
  • 🛠 fixes available for: 0

@ashearin ashearin force-pushed the parsing-adr-poc-url branch from d83be63 to bf37dd5 Compare January 10, 2025 03:07
@ashearin ashearin force-pushed the parsing-adr-poc-url branch from bf37dd5 to 8cebe63 Compare January 10, 2025 03:45
Comment thread internal/pkg/client/client.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant