KRM Exec Function can't locate executable when referencing a base

[pmcjury]

Describe the bug

When using a KRM exec function with a relative path to the kustomization.yaml file, the executable can't be found when referencing a base from a variant.

It appears the function is "exec"-d relative to the base not the variant.

Things are further complicated when the base references another remote base.

Files that can reproduce the issue

# dir
.
├── base
│   ├── deployment.yaml
│   └── kustomization.yaml
└── krm-exec
    ├── krm-config.yaml
    ├── kustomization.yaml
    └── plugins
        └── function.sh

# base/kustomization.yaml

resources:
 - deployment.yaml

# base/deployment.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: app1
spec:
  selector:
    matchLabels:
      app: hello
  template:
    metadata:
      labels:
        app: hello
    spec:
      containers:
      - name: busybox
        image: busybox:v1.0.0

#krm-exec/kustomization.yaml
resources:
- ../base
transformers:
- krm-config.yaml

#krm-exec/krm-config.yaml
apiVersion: my.example.io/v1
kind: MyPlugin
metadata:
  name: notImportantHere
  annotations:
    config.kubernetes.io/function: |
      exec: 
        path: ./plugins/function.sh

#krm-exec/plugins/function.sh
#!/bin/bash
resourceList=$(cat) 
# dummy
echo "$resourceList"

Expected output

The dummy function should just echo out the resources read from stdin.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: app1
spec:
  selector:
    matchLabels:
      app: hello
  template:
    metadata:
      labels:
        any: thing
    spec:
      containers:
      - name: busybox
        image: busybox:v1.0.0
     

Actual output

#from krm-exec dir
kustomize build --enable-alpha-plugins --enable-exec  .
Error: couldn't execute function: fork/exec ./plugins/function.sh: no such file or directory

Kustomize version

kustomize version
{Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:36:27Z GoOs:darwin GoArch:amd64}

Platform

macOS ( M1 )
darwin21.0
arm64

Additional context

Some other scenarios:

• The plugin works as expected when the variant kustomization does not reference the base, but a relative deployment.

#krm-exec/kustomizaiton.yaml
resources:
- deployment.yaml

• With the original setup If I move the ./plugins/function.sh to the base, the plugin works even if the krm-function.yaml is located in the variant

To further complicate things, if I add a remote base to the local base I get this error:

#base/kustomization.yaml
resources:
 - https://github.com/pmcjury/somerepo/manifests/base?ref=main
 - deployment.yaml

#from krm-exec dir
kustomize build --enable-alpha-plugins --enable-exec  .
Error: couldn't execute function: chdir /private/var/folders/yn/88ww9kqx1b35xylvc0wt0fb80000gn/T/kustomize-124974724/deploy/manifests/base: no such file or directory 

[pmcjury]

btw a work around is to just put the plugin in your $PATH and reference it without a path

[seh]

This looks like a duplicate of #4350. What timing!

[KnVerey]

/triage accepted

The fact that the Kustomization in question reading a child Kustomization interferes with the path is highly suspicous.

I think the root cause of both this and #4350 is that we're not properly deep copying the plugin loader here:

kustomize/api/internal/target/kusttarget.go

Lines 44 to 45 in a8f58b4

	pLdrCopy := *pLdr
	pLdrCopy.SetWorkDir(ldr.Root())

Since the loader contains a pointer to a PluginConfig, duplicating it will still point to the same plugin config (Simplified demo). So when we reset the root in creating the child target, we end up modifying the plugin config for the parent. 😱

@seh @pmcjury Since both issues are quite detailed (thank you!) I'm going to keep this one just because it was opened first. If either of you is interested in contributing a fix, please /assign yourself here. https://github.com/kubernetes-sigs/kustomize/pull/4125/files#diff-34d93fef5d3973a037f7871d2010ebaa89547617081d5f672bac16be3bf88160R125 can provide a pointer as to how to craft the necessary regression test.

cc @natasha41575

[seh]

a work around is to just put the plugin in your $PATH and reference it without a path

That's due to exec.Cmd using exec.LookPath here.

[m-Bilal]

/assign

[seh]

@m-Bilal, have you had a chance to attempt a fix for this defect yet?

[m-Bilal]

@seh sorry, haven't had the time yet. I'll leave a comment here when I do start working on this, until then if anyone else wants to take this up, please feel free to do so.

[aabouzaid]

If I get it right, all that's needed to fix this is just to change the line no. 45 from pLdrCopy := *pLdr to pLdrCopy := pLdr? 🤔

In that case, it will make a full copy instead of referring to the pointer.

If not, does anyone know what exactly is needed to fix this issue?

[seh]

In that case, it will make a full copy instead of referring to the pointer.

It will make a shallow copy. If you look at the loader.Loader struct type, you'll see that it's mostly pointers. Copying a Loader gives you the same pointer values, pointing at the same referenced values. Per Katrina's earlier #4347 (comment), at minimum we need to copy the types.PluginConfig referenced by the "pc" field.

Next, how we copy a types.PluginConfig? Two of its fields are ints, two are structs. Both of the structs contain only simple scalar values, so copying a types.PluginConfig can be achieved via assignment.

I am not sure whether there are more fields that we'd need to copy deeply, but we do need to copy the PluginConfig, because its "FnLoadingOptions" field of type 'FnPluginLoadingOptions` has a "WorkingDir" field that holds the crucial value, such that us using the wrong value there leads to this defect.

[aabouzaid]

/assign

[aabouzaid]

@seh Thanks for your tip.
I've fixed it by creating a new copy of PluginConfig with the values of pc.

I've tested it and it works fine now. If that approach is accepted, I will add the unit test for it.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[seh]

/remove-lifecycle stale

[smoyer64]

This issue was killing me - I know functions are alpha but it took me a while to find this bug report. Since I'm executing in a CD environment, changing the PATH has some unintended consequences. An alternate work-around that avoids messing with the environment is to put all the plugins in one of the bases. The caveat is that it has to be the last base that's loaded so I simply created a base that's nothing but plugins/ and an empty Kustomization manifest. My overlay's resources look something like:

resources:
- ../base1/
- ../base2/
- ../plugins/
- manifest1.yaml
- manifest2.yaml

[aabouzaid]

@smoyer64 it's really annoying issue, and the fix is already there #4654
I hope to find someone to review the PR.

[annasong20]

/close

Issue should be fixed by #4654

[k8s-ci-robot]

@annasong20: Closing this issue.

In response to this:

/close

Issue should be fixed by #4654

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.