Conversation
Dentrax
commented
Jul 8, 2024
cbd5210 to
0218df1
Compare
pkg/cli/scan.go
Outdated
| // getRepositoryURL returns the URL of the APKINDEX.tar.gz file for the given | ||
| // repository and architecture. If the repository URL already points to an | ||
| // APKINDEX.tar.gz file, it will be returned as-is. User input may or may not | ||
| // have included the architecture or the APKINDEX.tar.gz suffix, so construct | ||
| // the full URL to provide better UX. | ||
| func getRepositoryURL(repository, arch string) string { |
There was a problem hiding this comment.
Would it be possible to make the behavior simpler and more predictable, by requiring the caller to pass the URL to the repo and not to the index tar gz?
There was a problem hiding this comment.
Could you provide some example on this?
There was a problem hiding this comment.
Just wondering if we need to support both with and without the .../APKINDEX.tar.gz... It'd be simpler to say the URL has to be just to the repo, so like https://packages.wolfi.dev/os, instead of accepting multiple forms, unless we really need to support both?
There was a problem hiding this comment.
if we need to support both
IIUC, apk ls command supports both, and some packages does provide only single architecture, thats where we may need to pass the ARCH in the URL.
There was a problem hiding this comment.
some packages does provide only single architecture
If I'm following you, this problem exists with the remote scanning feature with or without this new enhancement, is that right?
Would a better solution here be to show a warning if not all architectures are found? And still error if none can be found?
I guess I'm not following how architecture availability is specific to this new flag, but maybe you can help me follow :)
bae53cb to
594a172
Compare
Signed-off-by: Dentrax <furkan.turkal@chainguard.dev>
594a172 to
6c99b9e
Compare