Repository: GoogleContainerTools/kpt
Pull request #2746 (2022-02-05 02:09:32)

Fix kpt file system interactions
* missing file.Close() call (filesys' memory filesystem doesn't support
  files opened multiple times)
* pass filesystem into newFnConfig (it reads fn config from file)


martinmaly: done. unfortunately vscode tends to pick the wrong default :)

Patch:
diff --git a/internal/fnruntime/runner.go b/internal/fnruntime/runner.go
index 557b6a37..1f814be4 100644
--- a/internal/fnruntime/runner.go
+++ b/internal/fnruntime/runner.go
@@ -19,7 +19,6 @@ import (
 	goerrors "errors"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"path"
 	"path/filepath"
 	"strings"
@@ -33,6 +32,7 @@ import (
 	kptfilev1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
 	"github.com/GoogleContainerTools/kpt/pkg/fn"
 	"github.com/google/shlex"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
 	"sigs.k8s.io/kustomize/kyaml/fn/framework"
 	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
 	"sigs.k8s.io/kustomize/kyaml/kio"
@@ -43,12 +43,12 @@ import (
 // NewRunner returns a kio.Filter given a specification of a function
 // and it's config.
 func NewRunner(
-	ctx context.Context, f *kptfilev1.Function,
+	ctx context.Context, fsys filesys.FileSystem, f *kptfilev1.Function,
 	pkgPath types.UniquePath, fnResults *fnresult.ResultList,
 	imagePullPolicy ImagePullPolicy, displayResourceCount bool,
 	runtime fn.FunctionRuntime) (kio.Filter, error) {
 
-	config, err := newFnConfig(f, pkgPath)
+	config, err := newFnConfig(fsys, f, pkgPath)
 	if err != nil {
 		return nil, err
 	}
@@ -469,7 +469,7 @@ func (ri *multiLineFormatter) String() string {
 	return b.String()
 }
 
-func newFnConfig(f *kptfilev1.Function, pkgPath types.UniquePath) (*yaml.RNode, error) {
+func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath types.UniquePath) (*yaml.RNode, error) {
 	const op errors.Op = "fn.readConfig"
 	var fn errors.Fn = errors.Fn(f.Image)
 
@@ -477,11 +477,12 @@ func newFnConfig(f *kptfilev1.Function, pkgPath types.UniquePath) (*yaml.RNode,
 	switch {
 	case f.ConfigPath != "":
 		path := filepath.Join(string(pkgPath), f.ConfigPath)
-		file, err := os.Open(path)
+		file, err := fsys.Open(path)
 		if err != nil {
 			return nil, errors.E(op, fn,
 				fmt.Errorf("missing function config %q", f.ConfigPath))
 		}
+		defer file.Close()
 		b, err := ioutil.ReadAll(file)
 		if err != nil {
 			return nil, errors.E(op, fn, err)
diff --git a/internal/util/render/executor.go b/internal/util/render/executor.go
index dbc7d065..d16c8d9f 100644
--- a/internal/util/render/executor.go
+++ b/internal/util/render/executor.go
@@ -488,7 +488,7 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in
 			}
 			hctx.dockerCheckDone = true
 		}
-		validator, err = fnruntime.NewRunner(ctx, &function, pn.pkg.UniquePath, hctx.fnResults, hctx.imagePullPolicy, displayResourceCount, hctx.runtime)
+		validator, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, hctx.imagePullPolicy, displayResourceCount, hctx.runtime)
 		if err != nil {
 			return err
 		}
@@ -605,7 +605,7 @@ func fnChain(ctx context.Context, hctx *hydrationContext, pkgPath types.UniquePa
 			}
 			hctx.dockerCheckDone = true
 		}
-		runner, err = fnruntime.NewRunner(ctx, &function, pkgPath, hctx.fnResults, hctx.imagePullPolicy, displayResourceCount, hctx.runtime)
+		runner, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pkgPath, hctx.fnResults, hctx.imagePullPolicy, displayResourceCount, hctx.runtime)
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/api/kptfile/v1/validation.go b/pkg/api/kptfile/v1/validation.go
index 25914937..1f88a20b 100644
--- a/pkg/api/kptfile/v1/validation.go
+++ b/pkg/api/kptfile/v1/validation.go
@@ -177,6 +177,7 @@ func GetValidatedFnConfigFromPath(fsys filesys.FileSystem, pkgPath types.UniqueP
 	if err != nil {
 		return nil, fmt.Errorf("functionConfig must exist in the current package")
 	}
+	defer file.Close()
 	reader := kio.ByteReader{Reader: file, PreserveSeqIndent: true, WrapBareSeqNode: true, DisableUnwrapping: true}
 	nodes, err := reader.Read()
 	if err != nil {


Test patch:
diff --git a/internal/fnruntime/runner_test.go b/internal/fnruntime/runner_test.go
index 97fd2b1f..37bbe218 100644
--- a/internal/fnruntime/runner_test.go
+++ b/internal/fnruntime/runner_test.go
@@ -29,6 +29,7 @@ import (
 	"github.com/GoogleContainerTools/kpt/internal/types"
 	kptfilev1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
 	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
 	"sigs.k8s.io/kustomize/kyaml/fn/framework"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -89,7 +90,8 @@ data: {foo: bar}
 				assert.NoError(t, err, "unexpected error")
 				c.fn.ConfigPath = path.Base(tmp.Name())
 			}
-			cn, err := newFnConfig(&c.fn, types.UniquePath(os.TempDir()))
+			fsys := filesys.MakeFsOnDisk()
+			cn, err := newFnConfig(fsys, &c.fn, types.UniquePath(os.TempDir()))
 			assert.NoError(t, err, "unexpected error")
 			actual, err := cn.String()
 			assert.NoError(t, err, "unexpected error")