Declarative function storage mounts are ignored

[ashlineldridge]

Describe the bug

When storage mounts are specified to a declarative function definition they are ignored. It looks like this code has been written to cater for command line --mount arguments, which get stored in the RunFns.StorageMounts slice, but declarative mounts which are stored in spec.Container.Network.StorageMounts are ignored.

Files that can reproduce the issue

In a new directory create the following files:

# kpt.yaml

apiVersion: kpt.example.com/v1alpha1
kind: FunctionConfig
metadata:
  name: example-function
  annotations:
    config.kubernetes.io/function: |
      container:
        image: example:latest
        mounts:
        - type: bind
          src: .
          dst: /src
spec: {}

# Dockerfile

FROM bash:latest
CMD ls /src >&2

First, build the function image:

docker build -t example:latest .

Then, run the Kpt function:

> kpt fn run .

ls: /src: No such file or directory
error: exit status 1

But if a command line mount is specified the function executes successfully:

> kpt fn run --mount type=bind,src="$(pwd)",dst=/src .

Dockerfile
kpt.yaml

Expected Output

It should be possible to configure storage mounts for kyaml functions declaratively.

[ashlineldridge]

Happy to submit a PR.

[Shell32-Natsu]

That's intended.In most of the cases the KRM function config file author is different from the user. It makes no sense to add a path in user's system when the author is creating a function config. So we have decided to make the mount purely imperative.

[Shell32-Natsu]

We haven't updated all documentation about this. Sorry for any confusion.

[ashlineldridge]

@Shell32-Natsu But what about the use case for functions being applied during kpt sync? Kptfiles allow you to declaratively specify functions that get applied during a sync operation (here).

IME, package consumer often wants to be able to define a pipeline of transformations and validations that get applied to pulled packages and that's what sync + declarative functions allow. Making certain properties of KRM functions imperative-only feels like it goes against the direction of the recent declarative setters proposal.

[Shell32-Natsu]

Please open an issue in kpt repo and we can discuss there.