diff --git a/coderd/dynamicparameters/render.go b/coderd/dynamicparameters/render.go index 9c4c73f87e5bc..b6a77c4704225 100644 --- a/coderd/dynamicparameters/render.go +++ b/coderd/dynamicparameters/render.go @@ -131,46 +131,48 @@ func (r *loader) Renderer(ctx context.Context, db database.Store, cache *files.C return r.staticRender(ctx, db) } - return r.dynamicRenderer(ctx, db, cache) + return r.dynamicRenderer(ctx, db, files.NewCacheCloser(cache)) } // Renderer caches all the necessary files when rendering a template version's // parameters. It must be closed after use to release the cached files. -func (r *loader) dynamicRenderer(ctx context.Context, db database.Store, cache *files.Cache) (*dynamicRenderer, error) { +func (r *loader) dynamicRenderer(ctx context.Context, db database.Store, cache *files.CacheCloser) (*dynamicRenderer, error) { + closeFiles := true // If the function returns with no error, this will toggle to false. + defer func() { + if closeFiles { + cache.Close() + } + }() + // If they can read the template version, then they can read the file for // parameter loading purposes. //nolint:gocritic fileCtx := dbauthz.AsFileReader(ctx) - templateFS, err := cache.Acquire(fileCtx, r.job.FileID) + + var templateFS fs.FS + var err error + + templateFS, err = cache.Acquire(fileCtx, r.job.FileID) if err != nil { return nil, xerrors.Errorf("acquire template file: %w", err) } - var terraformFS fs.FS = templateFS var moduleFilesFS *files.CloseFS if r.terraformValues.CachedModuleFiles.Valid { moduleFilesFS, err = cache.Acquire(fileCtx, r.terraformValues.CachedModuleFiles.UUID) if err != nil { - templateFS.Close() return nil, xerrors.Errorf("acquire module files: %w", err) } - terraformFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) + templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) } + closeFiles = false // Caller will have to call close return &dynamicRenderer{ data: r, - templateFS: terraformFS, + templateFS: templateFS, db: db, ownerErrors: make(map[uuid.UUID]error), - close: func() { - // Up to 2 files are cached, and must be released when rendering is complete. - // TODO: Might be smart to always call release when the context is - // canceled. - templateFS.Close() - if moduleFilesFS != nil { - moduleFilesFS.Close() - } - }, + close: cache.Close, }, nil } diff --git a/coderd/files/cache.go b/coderd/files/cache.go index 6e4dc9383b6f1..170abb10b1ff7 100644 --- a/coderd/files/cache.go +++ b/coderd/files/cache.go @@ -19,6 +19,10 @@ import ( "github.com/coder/coder/v2/coderd/util/lazy" ) +type FileAcquirer interface { + Acquire(ctx context.Context, fileID uuid.UUID) (*CloseFS, error) +} + // NewFromStore returns a file cache that will fetch files from the provided // database. func NewFromStore(store database.Store, registerer prometheus.Registerer, authz rbac.Authorizer) *Cache { diff --git a/coderd/files/closer.go b/coderd/files/closer.go new file mode 100644 index 0000000000000..9bd98fdd60caf --- /dev/null +++ b/coderd/files/closer.go @@ -0,0 +1,57 @@ +package files + +import ( + "context" + "sync" + + "github.com/google/uuid" + "golang.org/x/xerrors" +) + +// CacheCloser is a cache wrapper used to close all acquired files. +// This is a more simple interface to use if opening multiple files at once. +type CacheCloser struct { + cache FileAcquirer + + closers []func() + mu sync.Mutex +} + +func NewCacheCloser(cache FileAcquirer) *CacheCloser { + return &CacheCloser{ + cache: cache, + closers: make([]func(), 0), + } +} + +func (c *CacheCloser) Close() { + c.mu.Lock() + defer c.mu.Unlock() + + for _, doClose := range c.closers { + doClose() + } + + // Prevent further acquisitions + c.cache = nil + // Remove any references + c.closers = nil +} + +func (c *CacheCloser) Acquire(ctx context.Context, fileID uuid.UUID) (*CloseFS, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if c.cache == nil { + return nil, xerrors.New("cache is closed, and cannot acquire new files") + } + + f, err := c.cache.Acquire(ctx, fileID) + if err != nil { + return nil, err + } + + c.closers = append(c.closers, f.close) + + return f, nil +}