diff --git a/build/policy_loader.go b/build/policy_loader.go index 61c68018160a..219f85ecf374 100644 --- a/build/policy_loader.go +++ b/build/policy_loader.go @@ -152,11 +152,21 @@ func newPolicyPathFS(ctx context.Context, resolver *sourcemeta.Resolver, popt po } return func() (fs.StatFS, func() error, error) { - return p, p.Close, nil + ref := &policyPathFSRef{policyPathFS: p} + return ref, ref.Close, nil } } -func (p *policyPathFS) Open(name string) (fs.File, error) { +type policyPathFSRef struct { + *policyPathFS + mu sync.Mutex + cwdRoot fs.StatFS + cwdClose func() error + contextRoot fs.StatFS + contextClose func() error +} + +func (p *policyPathFSRef) Open(name string) (fs.File, error) { backend, target, err := p.resolve(name) if err != nil { return nil, err @@ -167,7 +177,7 @@ func (p *policyPathFS) Open(name string) (fs.File, error) { return backend.Open(target) } -func (p *policyPathFS) Stat(name string) (fs.FileInfo, error) { +func (p *policyPathFSRef) Stat(name string) (fs.FileInfo, error) { backend, target, err := p.resolve(name) if err != nil { return nil, err @@ -178,14 +188,7 @@ func (p *policyPathFS) Stat(name string) (fs.FileInfo, error) { return backend.Stat(target) } -func (p *policyPathFS) Close() error { - if err := p.cwdFS.close(); err != nil { - return err - } - return p.contextFS.close() -} - -func (p *policyPathFS) resolve(name string) (fs.StatFS, string, error) { +func (p *policyPathFSRef) resolve(name string) (fs.StatFS, string, error) { if name == "" { return nil, "", errors.New("policy filename is empty") } @@ -193,14 +196,14 @@ func (p *policyPathFS) resolve(name string) (fs.StatFS, string, error) { if v == "" { return nil, "", errors.Errorf("invalid policy filename %q", name) } - cwd, err := p.cwdFS.get() + cwd, err := p.getCwdFS() if err != nil { return nil, "", err } return cwd, filepath.Clean(v), nil } - contextFS, err := p.contextFS.get() + contextFS, err := p.getContextFS() if err != nil { return nil, "", err } @@ -214,6 +217,62 @@ func (p *policyPathFS) resolve(name string) (fs.StatFS, string, error) { return contextFS, normalizeLocalPolicyPath(name, p.contextDir), nil } +func (p *policyPathFSRef) getCwdFS() (fs.StatFS, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.cwdClose != nil { + return p.cwdRoot, nil + } + cwd, err := p.cwdFS.get() + if err != nil { + return nil, err + } + p.cwdRoot = cwd + p.cwdClose = p.cwdFS.close + return cwd, nil +} + +func (p *policyPathFSRef) getContextFS() (fs.StatFS, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.contextClose != nil { + return p.contextRoot, nil + } + contextFS, err := p.contextFS.get() + if err != nil { + return nil, err + } + p.contextRoot = contextFS + p.contextClose = p.contextFS.close + return contextFS, nil +} + +func (p *policyPathFSRef) Close() error { + p.mu.Lock() + cwdClose := p.cwdClose + contextClose := p.contextClose + p.cwdRoot = nil + p.contextRoot = nil + p.cwdClose = nil + p.contextClose = nil + p.mu.Unlock() + + var firstErr error + if cwdClose != nil { + if err := cwdClose(); err != nil { + firstErr = err + } + } + if contextClose != nil { + if err := contextClose(); err != nil { + if firstErr == nil { + firstErr = err + } + } + } + return firstErr +} + func normalizeLocalPolicyPath(name, contextDir string) string { if filepath.IsAbs(name) && contextDir != "" { if rel, err := filepath.Rel(contextDir, name); err == nil { @@ -405,7 +464,7 @@ func (i policyFileInfo) Sys() any { return &types.Stat{Mode: uint32(i.mode), Size: i.size, ModTime: i.tm.UnixNano()} } -var _ fs.StatFS = (*policyPathFS)(nil) +var _ fs.StatFS = (*policyPathFSRef)(nil) var _ fs.StatFS = (*remotePolicyFS)(nil) var _ fs.File = (*policyReadFile)(nil) var _ io.ReaderAt = (*bytes.Reader)(nil) diff --git a/build/policy_loader_test.go b/build/policy_loader_test.go index 584f3150435b..cfdadc0d5b3a 100644 --- a/build/policy_loader_test.go +++ b/build/policy_loader_test.go @@ -1,7 +1,10 @@ package build import ( + "context" "io/fs" + "os" + "path/filepath" "testing" "testing/fstest" @@ -83,3 +86,61 @@ func TestMemoizedPolicyFSReinitializesAfterAllRefsClosed(t *testing.T) { require.NoError(t, m.close()) require.Equal(t, 2, closeCalls) } + +func TestLoadPolicyDataReleasesPolicyDir(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "policy.rego"), []byte("package docker\n"), 0600)) + + provider := newPolicyPathFS(context.Background(), nil, policyOpt{ + ContextDir: dir, + }) + + _, ok, err := loadPolicyData(provider, "policy.rego") + require.NoError(t, err) + require.True(t, ok) + require.NoError(t, os.RemoveAll(dir)) +} + +func TestPolicyPathFSRefClose(t *testing.T) { + var initCalls int + var closeCalls int + + p := &policyPathFS{} + p.contextFS.init = func() (fs.StatFS, func() error, error) { + initCalls++ + root := fstest.MapFS{ + "policy.rego": &fstest.MapFile{Data: []byte("package docker\n")}, + } + return root, func() error { + closeCalls++ + return nil + }, nil + } + + first := &policyPathFSRef{policyPathFS: p} + _, err := first.Stat("policy.rego") + require.NoError(t, err) + f, err := first.Open("policy.rego") + require.NoError(t, err) + require.NoError(t, f.Close()) + require.Equal(t, 1, initCalls) + + require.NoError(t, first.Close()) + require.Equal(t, 1, closeCalls) + + shared := &policyPathFSRef{policyPathFS: p} + _, err = shared.Stat("policy.rego") + require.NoError(t, err) + require.Equal(t, 2, initCalls) + + second := &policyPathFSRef{policyPathFS: p} + _, err = second.Stat("policy.rego") + require.NoError(t, err) + require.Equal(t, 2, initCalls) + + require.NoError(t, second.Close()) + require.Equal(t, 1, closeCalls) + + require.NoError(t, shared.Close()) + require.Equal(t, 2, closeCalls) +}