Skip to content

[release-1.28] Migrate to fs.FS as the sole resource loading interface#1594

Merged
istio-testing merged 3 commits intoistio-ecosystem:release-1.28from
istio-testing:cherry-pick-1561-to-release-1.28
Mar 17, 2026
Merged

[release-1.28] Migrate to fs.FS as the sole resource loading interface#1594
istio-testing merged 3 commits intoistio-ecosystem:release-1.28from
istio-testing:cherry-pick-1561-to-release-1.28

Conversation

@istio-testing
Copy link
Copy Markdown
Collaborator

This is an automated cherry-pick of #1561

Replace string-based ResourceDirectory with fs.FS throughout the codebase
to provide a unified abstraction for loading Helm charts and profiles.

Changes:
- ReconcilerConfig.ResourceDirectory string → ResourceFS fs.FS
- cmd/main.go wraps flag value with os.DirFS() at startup
- All controllers use ResourceFS directly (no path construction with ResourceDirectory)
- UpgradeOrInstallChart now takes (fs.FS, chartPath) instead of chartDir
- Renamed getChartDir() → getChartPath() (returns relative path)
- Added pkg/helm/fsloader.go with LoadChart() for loading charts from fs.FS

This enables consumers to use embed.FS for bundled resources or os.DirFS
for filesystem-based resources through a single consistent interface.

BREAKING CHANGE: ReconcilerConfig.ResourceDirectory replaced with ResourceFS fs.FS.
ChartManager.UpgradeOrInstallChart signature changed to accept fs.FS.

Signed-off-by: Aslak Knutsen <aslak@4fs.no>
Provide an embed.FS in the resources package so downstream consumers
can bundle Helm charts and profiles directly in their binary instead
of relying on filesystem paths.

The Sail Operator itself does not import this package, keeping its
binary size unchanged. This is intended for library consumers who
want self-contained binaries with embedded resources.

Usage:
  import "github.com/istio-ecosystem/sail-operator/resources"
  cfg := config.ReconcilerConfig{ResourceFS: resources.FS}

Signed-off-by: Aslak Knutsen <aslak@4fs.no>
Add embedded fs.FS from the resources package and use it as the default
resource source. The operator now embeds all Helm charts and profiles
directly in the binary, eliminating the need for external resource files.

Changes:
- Change --resource-directory default from /var/lib/sail-operator/resources to ""
- When --resource-directory is empty (default), use embedded resources.FS
- When --resource-directory is specified, use os.DirFS for filesystem access
- Removed --resource-directory from Makefile
- Removed --resource-directory from Dockerfile

This increases binary size by ~10MB but simplifies deployment by removing
the dependency on external resource files mounted into the container.

Signed-off-by: Aslak Knutsen <aslak@4fs.no>
@istio-testing istio-testing force-pushed the cherry-pick-1561-to-release-1.28 branch from f919825 to 6f8f882 Compare February 12, 2026 11:18
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 12, 2026

Codecov Report

❌ Patch coverage is 73.07692% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.63%. Comparing base (c365e82) to head (6f8f882).
⚠️ Report is 10 commits behind head on release-1.28.

Files with missing lines Patch % Lines
cmd/main.go 0.00% 8 Missing ⚠️
resources/resources.go 0.00% 7 Missing ⚠️
pkg/helm/fsloader.go 84.00% 2 Missing and 2 partials ⚠️
pkg/helm/chartmanager.go 83.33% 1 Missing ⚠️
pkg/istiovalues/profiles.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.28    #1594      +/-   ##
================================================
+ Coverage         71.22%   80.63%   +9.41%     
================================================
  Files                39       46       +7     
  Lines              2040     2345     +305     
================================================
+ Hits               1453     1891     +438     
+ Misses              424      337      -87     
+ Partials            163      117      -46     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dgn
Copy link
Copy Markdown
Collaborator

dgn commented Feb 12, 2026

/hold

actually this includes bigger changes (e.g. changing the default to embedded) that we probably don't want

@dgn
Copy link
Copy Markdown
Collaborator

dgn commented Mar 17, 2026

/retest

@istio-testing istio-testing merged commit 2463a29 into istio-ecosystem:release-1.28 Mar 17, 2026
19 checks passed
openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Mar 18, 2026
* upstream/release-1.28:
  [release-1.28] Migrate to fs.FS as the sole resource loading interface (istio-ecosystem#1594)
  Adding FIPS_CLUSTER variable to E2E test (istio-ecosystem#1698) (istio-ecosystem#1704)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants