refactor: improve overlayFS errors (#17808)

coderd/files/overlay.go

@@ -4,15 +4,12 @@ import (
 	"io/fs"
 	"path"
 	"strings"
-
-	"golang.org/x/xerrors"
 )
 
-// overlayFS allows you to "join" together the template files tar file fs.FS
+// overlayFS allows you to "join" together multiple fs.FS. Files in any specific
-// with the Terraform modules tar file fs.FS. We could potentially turn this
+// overlay will only be accessible if their path starts with the base path
-// into something more parameterized/configurable, but the requirements here are
+// provided for the overlay. eg. An overlay at the path .terraform/modules
-// a _bit_ odd, because every file in the modulesFS includes the
+// should contain files with paths inside the .terraform/modules folder.
-// .terraform/modules/ folder at the beginning of it's path.
 type overlayFS struct {
 	baseFS   fs.FS
 	overlays []Overlay
@@ -23,64 +20,32 @@ type Overlay struct {
 	fs.FS
 }
 
-func NewOverlayFS(baseFS fs.FS, overlays []Overlay) (fs.FS, error) {
+func NewOverlayFS(baseFS fs.FS, overlays []Overlay) fs.FS {
-	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)
-		}
-	}
-
 	return overlayFS{
 		baseFS:   baseFS,
 		overlays: overlays,
-	}, nil
+	}
+}
+
+func (f overlayFS) target(p string) fs.FS {
+	target := f.baseFS
+	for _, overlay := range f.overlays {
+		if strings.HasPrefix(path.Clean(p), overlay.Path) {
+			target = overlay.FS
+			break
+		}
+	}
+	return target
 }
 
 func (f overlayFS) Open(p string) (fs.File, error) {
-	for _, overlay := range f.overlays {
+	return f.target(p).Open(p)
-		if strings.HasPrefix(path.Clean(p), overlay.Path) {
-			return overlay.FS.Open(p)
-		}
-	}
-	return f.baseFS.Open(p)
 }
 
 func (f overlayFS) ReadDir(p string) ([]fs.DirEntry, error) {
-	for _, overlay := range f.overlays {
+	return fs.ReadDir(f.target(p), p)
-		if strings.HasPrefix(path.Clean(p), overlay.Path) {
-			//nolint:forcetypeassert
-			return overlay.FS.(fs.ReadDirFS).ReadDir(p)
-		}
-	}
-	//nolint:forcetypeassert
-	return f.baseFS.(fs.ReadDirFS).ReadDir(p)
 }
 
 func (f overlayFS) ReadFile(p string) ([]byte, error) {
-	for _, overlay := range f.overlays {
+	return fs.ReadFile(f.target(p), p)
-		if strings.HasPrefix(path.Clean(p), overlay.Path) {
-			//nolint:forcetypeassert
-			return overlay.FS.(fs.ReadFileFS).ReadFile(p)
-		}
-	}
-	//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)
-	if !ok {
-		return xerrors.New("overlayFS does not implement ReadFileFS")
-	}
-	return nil
 }

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)

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}})
+			templateFS = 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
-			}
 		}
 	} else if !xerrors.Is(err, sql.ErrNoRows) {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{