zarf icon indicating copy to clipboard operation
zarf copied to clipboard

Refactor RecursiveFileList to handle error tests and include unit tests.

Open phillebaba opened this issue 6 months ago • 0 comments

Describe what should be investigated or refactored

We are currently using RecursiveFileList in a few places in the code base. This helper function comes from the pkg repository that contains shared helpers. We are not checking for errors at any of the places where we call this function, which could result in unintended consequences.

We would experience a couple of failing tests if we actually checked the error returned from the function when we pass a path that does not exists. An example where this happens is here.

https://github.com/zarf-dev/zarf/blob/51fcc0336680e877a92b0dc305496ce08e500d35/src/pkg/packager/prepare.go#L178

The reason this occurs is because we will always set the values file path even if not configured, but we only actually create the directory if values have been configured for the component.

https://github.com/zarf-dev/zarf/blob/51fcc0336680e877a92b0dc305496ce08e500d35/src/pkg/layout/component.go#L185-L191

We need to do a couple of things to remediate this debt.

  • Start checking errors returned.
  • Evaluate if we should stop depending on the pkg repository.
  • Add unit tests to the function.

Links to any relevant code

https://github.com/defenseunicorns/pkg/blob/72f16722035a1c454cada715e698a5c934e2d039/helpers/files.go#L69-L99

Additional context

N/A

phillebaba avatar Aug 13 '24 09:08 phillebaba