From a1f2c0de5e89d7f95ccbc7884e4b0f8eeb66c3de Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 13 May 2025 21:19:24 +0000 Subject: [PATCH 1/4] refactor: improve overlayFS errors --- coderd/files/overlay.go | 63 ++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/coderd/files/overlay.go b/coderd/files/overlay.go index d7e2adf8db4e8..56fb3c0ca1d7f 100644 --- a/coderd/files/overlay.go +++ b/coderd/files/overlay.go @@ -8,11 +8,10 @@ import ( "golang.org/x/xerrors" ) -// overlayFS allows you to "join" together the template files tar file fs.FS -// with the Terraform modules tar file fs.FS. We could potentially turn this -// into something more parameterized/configurable, but the requirements here are -// a _bit_ odd, because every file in the modulesFS includes the -// .terraform/modules/ folder at the beginning of it's path. +// overlayFS allows you to "join" together multiple fs.FS. Files in any specific +// overlay will only be accessible if their path starts with the base path +// provided for the overlay. eg. An overlay at the path .terraform/modules +// should contain files with paths inside the .terraform/modules folder. type overlayFS struct { baseFS fs.FS overlays []Overlay @@ -23,64 +22,50 @@ type Overlay struct { fs.FS } -func NewOverlayFS(baseFS fs.FS, overlays []Overlay) (fs.FS, error) { - if err := valid(baseFS); err != nil { - return nil, xerrors.Errorf("baseFS: %w", err) - } - - for _, overlay := range overlays { - if err := valid(overlay.FS); err != nil { - return nil, xerrors.Errorf("overlayFS: %w", err) - } - } - +func NewOverlayFS(baseFS fs.FS, overlays []Overlay) fs.FS { return overlayFS{ baseFS: baseFS, overlays: overlays, - }, nil + } } func (f overlayFS) Open(p string) (fs.File, error) { + target := f.baseFS for _, overlay := range f.overlays { if strings.HasPrefix(path.Clean(p), overlay.Path) { - return overlay.FS.Open(p) + target = overlay.FS + break } } - return f.baseFS.Open(p) + return target.Open(p) } func (f overlayFS) ReadDir(p string) ([]fs.DirEntry, error) { + target := f.baseFS for _, overlay := range f.overlays { if strings.HasPrefix(path.Clean(p), overlay.Path) { - //nolint:forcetypeassert - return overlay.FS.(fs.ReadDirFS).ReadDir(p) + target = overlay.FS + break } } - //nolint:forcetypeassert - return f.baseFS.(fs.ReadDirFS).ReadDir(p) + it, ok := target.(fs.ReadDirFS) + if !ok { + return nil, xerrors.New("provided fs.FS does not implement fs.ReadDirFS") + } + return it.ReadDir(p) } func (f overlayFS) ReadFile(p string) ([]byte, error) { + target := f.baseFS for _, overlay := range f.overlays { if strings.HasPrefix(path.Clean(p), overlay.Path) { - //nolint:forcetypeassert - return overlay.FS.(fs.ReadFileFS).ReadFile(p) + target = overlay.FS + break } } - //nolint:forcetypeassert - return f.baseFS.(fs.ReadFileFS).ReadFile(p) -} - -// valid checks that the fs.FS implements the required interfaces. -// The fs.FS interface is not sufficient. -func valid(fsys fs.FS) error { - _, ok := fsys.(fs.ReadDirFS) - if !ok { - return xerrors.New("overlayFS does not implement ReadDirFS") - } - _, ok = fsys.(fs.ReadFileFS) + it, ok := target.(fs.ReadFileFS) if !ok { - return xerrors.New("overlayFS does not implement ReadFileFS") + return nil, xerrors.New("provided fs.FS does not implement fs.ReadFileFS") } - return nil + return it.ReadFile(p) } From 20263c6a7014bc669d74e3d195b5b12ff416b4d1 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 13 May 2025 21:49:34 +0000 Subject: [PATCH 2/4] revert calls --- coderd/files/overlay_test.go | 3 +-- coderd/parameters.go | 9 +-------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/coderd/files/overlay_test.go b/coderd/files/overlay_test.go index 8d30f6e0a5a1f..29209a478d552 100644 --- a/coderd/files/overlay_test.go +++ b/coderd/files/overlay_test.go @@ -21,11 +21,10 @@ func TestOverlayFS(t *testing.T) { afero.WriteFile(b, ".terraform/modules/modules.json", []byte("{}"), 0o644) afero.WriteFile(b, ".terraform/modules/example_module/main.tf", []byte("terraform {}"), 0o644) - it, err := files.NewOverlayFS(afero.NewIOFS(a), []files.Overlay{{ + it := files.NewOverlayFS(afero.NewIOFS(a), []files.Overlay{{ Path: ".terraform/modules", FS: afero.NewIOFS(b), }}) - require.NoError(t, err) content, err := fs.ReadFile(it, "main.tf") require.NoError(t, err) diff --git a/coderd/parameters.go b/coderd/parameters.go index 6b6f4db531533..24a91d3a7514b 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -97,14 +97,7 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http return } defer api.FileCache.Release(tf.CachedModuleFiles.UUID) - templateFS, err = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error creating overlay filesystem.", - Detail: err.Error(), - }) - return - } + templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) } } else if !xerrors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ From d4206d31dde7124b4666045ed91624f6244f321e Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 13 May 2025 22:03:13 +0000 Subject: [PATCH 3/4] use the fs package functions rather than methods --- coderd/files/overlay.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/coderd/files/overlay.go b/coderd/files/overlay.go index 56fb3c0ca1d7f..fd329fe99aa90 100644 --- a/coderd/files/overlay.go +++ b/coderd/files/overlay.go @@ -4,8 +4,6 @@ import ( "io/fs" "path" "strings" - - "golang.org/x/xerrors" ) // overlayFS allows you to "join" together multiple fs.FS. Files in any specific @@ -48,11 +46,7 @@ func (f overlayFS) ReadDir(p string) ([]fs.DirEntry, error) { break } } - it, ok := target.(fs.ReadDirFS) - if !ok { - return nil, xerrors.New("provided fs.FS does not implement fs.ReadDirFS") - } - return it.ReadDir(p) + return fs.ReadDir(target, p) } func (f overlayFS) ReadFile(p string) ([]byte, error) { @@ -63,9 +57,5 @@ func (f overlayFS) ReadFile(p string) ([]byte, error) { break } } - it, ok := target.(fs.ReadFileFS) - if !ok { - return nil, xerrors.New("provided fs.FS does not implement fs.ReadFileFS") - } - return it.ReadFile(p) + return fs.ReadFile(target, p) } From fdd6a43d9809660f828c2545dd3e6d65cdaf8301 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 13 May 2025 22:08:54 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=A7=96=E2=80=8D=E2=99=80=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- coderd/files/overlay.go | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/coderd/files/overlay.go b/coderd/files/overlay.go index fd329fe99aa90..fa0e590d1e6c2 100644 --- a/coderd/files/overlay.go +++ b/coderd/files/overlay.go @@ -27,7 +27,7 @@ func NewOverlayFS(baseFS fs.FS, overlays []Overlay) fs.FS { } } -func (f overlayFS) Open(p string) (fs.File, error) { +func (f overlayFS) target(p string) fs.FS { target := f.baseFS for _, overlay := range f.overlays { if strings.HasPrefix(path.Clean(p), overlay.Path) { @@ -35,27 +35,17 @@ func (f overlayFS) Open(p string) (fs.File, error) { break } } - return target.Open(p) + return target +} + +func (f overlayFS) Open(p string) (fs.File, error) { + return f.target(p).Open(p) } func (f overlayFS) ReadDir(p string) ([]fs.DirEntry, error) { - target := f.baseFS - for _, overlay := range f.overlays { - if strings.HasPrefix(path.Clean(p), overlay.Path) { - target = overlay.FS - break - } - } - return fs.ReadDir(target, p) + return fs.ReadDir(f.target(p), p) } func (f overlayFS) ReadFile(p string) ([]byte, error) { - target := f.baseFS - for _, overlay := range f.overlays { - if strings.HasPrefix(path.Clean(p), overlay.Path) { - target = overlay.FS - break - } - } - return fs.ReadFile(target, p) + return fs.ReadFile(f.target(p), p) }