fix: local file upload client fix for paths [PS-87] #502

alina-d-m · 2025-12-11T15:38:00Z

Updated the local file upload with the following fixes:

CreateRevisionFromPaths was previously creating single file batches which limited the client to uploading only 50 files using this method, updated the method to correctly read from a chan of files and correctly spread the files over the 50 batch (part) limit per revision, this prompted the refactor of CreateRevisionFromPaths using a producer - consumer design: producer walks the dir/reads the files from input and adds them to a chan, the consumer (batchPaths) reads from the chan and creates batches to be uploaded, error contexts are synced between the 2 (if producer fails, consumer will also stop and not hang indefinitely, if consumer fails, the producer stops adding file paths to the chan)
Removed additional filtering that was added when a directory was uploaded
For simplicity, removed helper functions for uploading a directory/paths that were either way calling addPathsToRevision
Added the rootPath argument for all client methods such that the relative path of the files that are uploaded is correctly computed relative to the desired rootPath
Addressed bug that was leaving open files in a batch if the batchPaths consumer errored and the entire upload failed

Added a new method on the client for creating a revision given as input a chan of file paths: CreateRevisionFromChan.

Jira: https://snyksec.atlassian.net/browse/PS-87

snyk-io · 2025-12-11T15:38:13Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

snyk-io · 2025-12-11T15:38:14Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

pkg/apiclients/fileupload/client.go

bastiandoetsch · 2025-12-17T09:19:37Z

pkg/apiclients/fileupload/client.go

+			info, statErr := os.Stat(pth)
+			if statErr != nil {
+				c.logger.Error().Err(statErr).Str("path", pth).Msg("failed to stat path")
+				return uploadrevision2.NewFileAccessError(pth, statErr)


Should we maybe be lenient and just upload the files that are there instead of failing the whole upload ?

I kept the previous logic here and for now this is the behaviour we also want, scan everything or nothing at all because we don't have a way to specify to the user which files we were able to scan and which not. This is something we wanted to consider further on (partial scans).

From a framework/sdk perspective, which GAF is, I think what @bastiandoetsch said is correct. the library function should be implemented independent of constraints of a specific consumer.

The library needs to make errors clear and the consumer, like a specific extension can react the way they want to or are able to.

From a library perspective a possible futrue performance improvement could then be to support a configuration value that defines to fail early,

Got it. I think it would be quite straightforward to extend uploadOptions with this config value and extend the interface (either way we are doing it to add the rootPath), wdyt?

Taking a closer look at the code and this functionality, if we want to be more lenient we'll also need to update how
batchPaths works since right now it fails if there's any error related to one of the files it batches.

@bastiandoetsch, I had a quick sync with @PeterSchafer and decided we can have another iteration for the client in order to enable partial scans & be lenient about individual file errors (upload the files that don't error out instead of failing the whole upload).

pkg/apiclients/fileupload/client.go

PeterSchafer · 2025-12-17T10:59:53Z

pkg/apiclients/fileupload/client.go

 // Client defines the interface for the high level file upload client.
 type Client interface {
-	CreateRevisionFromPaths(ctx context.Context, paths []string) (UploadResult, error)
+	CreateRevisionFromPaths(ctx context.Context, paths []string, rootPath string) (UploadResult, error)


@alina-d-m quick thought/question, will you be using any of these other Create functions? Or will you be working with CreateRevisionFromChan()?
If you will be using CreateRevisionFromChan(), I would propose to remove these convenience functions actually.
Everything can be achieved through CreateRevisionFromChan and it is not clear if any of them is actually helpful/required.

We will be using CreateRevisionFromChan(), I can remove the other ones if you think it's not worth having them here (I tried to make them work just to align with the previous iteration of the client).

Okay, this sounds awesome. Thank you for making them work! I only left them there because of the missing channel version, so now that you have it, I think we can make this implementation even more focussed and slim, which will make it easier to test and maintain.

Sorry that this approach created some extra iterations and work. But I think CreateRevisionFromChan() is the perfect minimal interface and we should encourage its usage!

However, I think the cli-extension-os-flows uses CreateRevisionFromDir which would make it a bit difficult for them to migrate to the GAF client 🤔

The migration will not work 1:1 anyways, since we will exclude filtering from the fileupload API. This means, they will use channel based API anyways.

Removed the other convenience functions & cleaned-up the client + tests, if you could take another look 🙏

alina-d-m force-pushed the fix/PS-87/local-file-upload-client-fix-for-paths branch from 368f772 to 497bbde Compare December 11, 2025 15:46

fix: create revision from paths & from chan [PS-87]

187f6c6

alina-d-m force-pushed the fix/PS-87/local-file-upload-client-fix-for-paths branch from 497bbde to 187f6c6 Compare December 11, 2025 15:51

alina-d-m changed the title ~~Fix/ps 87/local file upload client fix for paths~~ fix: local file upload client fix for paths [PS-87] Dec 12, 2025

alina-d-m added 3 commits December 12, 2025 17:15

fix: test case [PS-87]

ca29bb6

fix: files left open on failure [PS-87]

1112a2e

fix: extra tests for error case when path issues [PS-87]

177431e

alina-d-m marked this pull request as ready for review December 15, 2025 09:34

alina-d-m requested review from a team as code owners December 15, 2025 09:34

mihai-snyk approved these changes Dec 16, 2025

View reviewed changes

pkg/apiclients/fileupload/client.go Outdated Show resolved Hide resolved

bastiandoetsch reviewed Dec 17, 2025

View reviewed changes

pkg/apiclients/fileupload/client.go Outdated Show resolved Hide resolved

fix: refactor CreateRevisionFromPaths[PS-87]

4fa0d40

PeterSchafer reviewed Dec 17, 2025

View reviewed changes

pkg/apiclients/fileupload/client.go Show resolved Hide resolved

PeterSchafer reviewed Dec 17, 2025

View reviewed changes

alina-d-m added 2 commits December 17, 2025 15:28

fix: cleanup client, leave CreateRevisionFromChan only [PS-87]

21d08f2

fix: linting [PS-87]

14bcfdc

PeterSchafer approved these changes Dec 17, 2025

View reviewed changes

PeterSchafer merged commit e90bc95 into main Dec 17, 2025
13 checks passed

PeterSchafer deleted the fix/PS-87/local-file-upload-client-fix-for-paths branch December 17, 2025 15:37

PeterSchafer mentioned this pull request Dec 17, 2025

chore: ensure to use latest gaf version snyk/cli#6393

Merged

9 tasks

fix: local file upload client fix for paths [PS-87] #502

fix: local file upload client fix for paths [PS-87] #502

Uh oh!

Conversation

alina-d-m commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snyk-io bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

snyk-io bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alina-d-m Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alina-d-m commented Dec 11, 2025 •

edited

Loading

snyk-io bot commented Dec 11, 2025 •

edited

Loading

snyk-io bot commented Dec 11, 2025 •

edited

Loading

alina-d-m Dec 17, 2025 •

edited

Loading