Helm OCI chart deployment fails in Windows

[EronWright]

Proposed changes

Closes #2644

This PR fixes support for OCI charts on Windows, by making the code be more consistent with Helm (see code). I believe the error comes from the call to os.Stat (see Golang implementation which is based on CreateFile, here).

Related issues (optional)

[github-actions]

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (1944a52) 18.31% compared to head (ed5d5e1) 18.31%.

Files	Patch %	Lines
provider/pkg/provider/helm_release.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2648   +/-   ##
=======================================
  Coverage   18.31%   18.31%           
=======================================
  Files          47       47           
  Lines        9644     9643    -1     
=======================================
  Hits         1766     1766           
+ Misses       7779     7778    -1     
  Partials       99       99           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[iwahbe]

I believe that we run tests on windows? Can we make sure that the scenario we are addressing is under test.

[rquitales]

I believe that we run tests on windows? Can we make sure that the scenario we are addressing is under test.

GHA supports Windows runners, but I believe our providers only tests on Linux environments currently. It makes sense to increase the scope of testing to all supported envs/architectures, but perhaps we need a bigger discussion about the approach for this and any optimizations we might need to undertake?

It'd be great to get Windows testing enabled to ensure we don't have any regressions here, but that is probably a larger orthogonal effort to get it landed. I'd be happy if we can manually validate that this fix works within a Windows environment to get it submitted for now.

[iwahbe]

I believe that we run tests on windows? Can we make sure that the scenario we are addressing is under test.

GHA supports Windows runners, but I believe our providers only tests on Linux environments currently. It makes sense to increase the scope of testing to all supported envs/architectures, but perhaps we need a bigger discussion about the approach for this and any optimizations we might need to undertake?

I agree.

It'd be great to get Windows testing enabled to ensure we don't have any regressions here, but that is probably a larger orthogonal effort to get it landed. I'd be happy if we can manually validate that this fix works within a Windows environment to get it submitted for now.

We still need to test that localChart swallows all errors here, not just file not found. We can do this on unix systems (permission errors come to mind). Otherwise I worry about someone accidentally reverting this change. Since we don't test on windows here, we can keep the integration test manual.

[EronWright]

Thanks both for the discussion. I'll provision a Windows VM to test this PR, and will incorporate the notion of a cross-platform test pass into a future "Release Criteria" design doc.

[EronWright]

I tested the fix on a Windows PC, seems to work well.

[rquitales]

LGTM. Thanks for validating on Windows.