fix: save fnConfig for custom path

[yuwenma]

Problem:

When saving a function to the Kptfile with --fn-config flag, current code saves the fn-config relevant path to current dir. This is incorrect, the fn-config should be relevant path to the kpt package dir. See below exmaple

Example:

Users run kpt fn eval against a custom path "yuyu" and not using the current working dir, e.g.

kpt fn eval yuyu-s --image set-namespace:v0.3.4 --fn-path=yuyu/package-context.yaml`

The Kptfile is

apiVersion: kpt.dev/v1
kind: Kptfile
...
pipeline:
  mutators:
    - image: gcr.io/kpt-fn/set-namespace:v0.3.4
      configPath: yuyu/package-context.yaml # Wrong

After the fix, it will be

apiVersion: kpt.dev/v1
kind: Kptfile
...
pipeline:
  mutators:
    - image: gcr.io/kpt-fn/set-namespace:v0.3.4
      configPath: package-context.yaml

Updates:

Restrict conditions that the fn-config must be under kpt pkg if saving flags. if fn-config is outside pkg, here's the error message:

kpt fn eval test -s -t mutator --image set-namespace:v0.3.4 --fn-config package-context.yaml     
error: --fn-config must be under /Users/yuwenma/kpt-example/set-namespace-v2/test if saving functions to Kptfile (--save=true)

[droot]

I liked current behavior of fn eval where it can accept fn-config outside of the package as well. It allowed to experiment with different config especially when I am testing out a function and it also allowed fn eval to be used in an adhoc way. When I am happy with a fn-config and I want to make that function part of the pipeline (fn render) I would move that config to be part of the package and want the path to be relative.

I believe we can have best of both the behaviors:

• Preserve existing fn eval behavior to support any --fn-config path (if invoked without --save)
• For --save option, we enforce that the path is relative to the package (one nit: current code is ignoring the error if path isn't relative)

[yuwenma]

I liked current behavior of fn eval where it can accept fn-config outside of the package as well. It allowed to experiment with different config especially when I am testing out a function and it also allowed fn eval to be used in an adhoc way. When I am happy with a fn-config and I want to make that function part of the pipeline (fn render) I would move that config to be part of the package and want the path to be relative.

I believe we can have best of both the behaviors:

• Preserve existing fn eval behavior to support any --fn-config path (if invoked without --save)
• For --save option, we enforce that the path is relative to the package (one nit: current code is ignoring the error if path isn't relative)

That's a good point. Would users ever want to use a fn-config outside of the kpt package? I am thinking of rather than enforcing the path be relative to the kpt package, how about keep a copy fn-config if the path is outside the pkg dir? Would that be more convenient for users or cause unnecessary confusions?

[droot]

Would users ever want to use a fn-config outside of the kpt package?

Note users use kpt fn eval in general scripts and compose their own pipeline using unix | as well. So having that flexibility is a good thing. So erring on the side of preserving current behavior is better (and it also remove any concerns for breaking existing behavior).

I am thinking of rather than enforcing the path be relative to the kpt package, how about keep a copy fn-config if the path is outside the pkg dir? Would that be more convenient for users or cause unnecessary confusions?

Copying fn-config if outside the package can be added later if we see users running in to it. I have a feeling cases where users eventually want to add function to render, they will start with the fn-config being part of their package, so copying the file may not of much value.

[yuwenma]

Would users ever want to use a fn-config outside of the kpt package?

Note users use kpt fn eval in general scripts and compose their own pipeline using unix | as well. So having that flexibility is a good thing. So erring on the side of preserving current behavior is better (and it also remove any concerns for breaking existing behavior).

I am thinking of rather than enforcing the path be relative to the kpt package, how about keep a copy fn-config if the path is outside the pkg dir? Would that be more convenient for users or cause unnecessary confusions?

Copying fn-config if outside the package can be added later if we see users running in to it. I have a feeling cases where users eventually want to add function to render, they will start with the fn-config being part of their package, so copying the file may not of much value.

Sounds good. I added a check to only save function if fn-config is inside kpt pkg. Please see the last commit