skaffold icon indicating copy to clipboard operation
skaffold copied to clipboard

Configuration dependencies don't resolve paths to deploy hooks

Open neutvd opened this issue 2 years ago • 5 comments

Expected behavior

When having a toplevel configution like this:

apiVersion: skaffold/v2beta25
kind: Config
requires:
  - configs: ["cfg1"]
    path: cfg1/skaffold.yaml
  - configs: ["cfg2"]
    path: cfg2/skaffold.yaml

When the skaffold.yaml files in ./cfg1 and ./cfg2 define deploy hooks, the paths to these hooks should resolved relative to the ./cfg1, or ./cfg2 directory. For instance if ./cfg2 has:

deploy:
  kubectl:
    manifests:
    - deploy/kubernetes.yaml
    hooks:
      before:
        - host:
            command: ["sh", "-c", "deploy/skaffold-before-hook.sh"]

it should look for skaffold-before-hook.sh in ./cfg2/deploy/ and not in ./deploy. Also, any files that are created or referenced by this hook should be relative to ./cfg2

Actual behavior

hooks are resolved relative to the toplevel directory. I have to make a symlink ./deploy/skaffold-before-hook.sh --> ../cfg2/deploy/skaffold-before-hook.sh

Information

  • Skaffold version: 1.37.0
  • Operating system: Linux Ubuntu 20.04
  • Installed via: skaffold.dev

neutvd avatar Mar 31 '22 08:03 neutvd

@neutvd I think the path resolution happens from where you run skaffold. That said, if you want the skaffold runs for same skaffold.yaml to work from top level or cfg1, we need to resolve the path relative to the root of the config file.

Note to the team: Lets take a look at the docs and see if this was intended behavior or a regression happened along the way.

tejal29 avatar Mar 31 '22 16:03 tejal29

Hi @tejal29, at https://skaffold.dev/docs/design/config/#configuration-dependencies it says the following:

In imported configurations, files are resolved relative to the location of imported Skaffold configuration file.

I infer from that that the paths should be resolved from cfg1/skaffold.yaml when that is the file that refers to the path, since it is that file that is being imported, and not the file that is doing the importing.

neutvd avatar Mar 31 '22 19:03 neutvd

This is still happening with [email protected]. This is really annoying as it defeat most of the dependency feature purpose. Is there any plan to tackle that @tejal29 ? @neutvd Did you find any workaround apart from symlinking files ?

toniopelo avatar Sep 06 '22 14:09 toniopelo

Hi @toniopelo, no I have not found a work around other than writing a script that goes through the sub directories and runs skaffold in each of them. The symlink solution is not really sustainable in practice, because they don't exist in the remote repository (on github or gitlab for example) from where the CI/CD deployment scripts are run. Also, any artifacts produced by the hooks end up in the wrong directory, so other scripts cannot find them without also adding code in the hooks to create symlinks to these artifacts.

neutvd avatar Sep 07 '22 06:09 neutvd

Thanks for the reply @neutvd, I guess I'll have to implement something like that as well.

toniopelo avatar Sep 07 '22 08:09 toniopelo

hey y'all. Sorry for the confusing documentation. I think it needs to be clarified that files are resolved relative to the imported config's location ONLY when resolving a field of the skaffold config that we have marked as a filepath type (ex: build.artifacts.docker.dockerfile or deploy.helm.releases.chartPath)

We currently don't support this for cases like this, as we don't parse the arbitrary strings of the command field as filepaths. This would be a good feature to have. I'll update this issue accordingly

MarlonGamez avatar Jan 09 '23 19:01 MarlonGamez

@tejal29 @MarlonGamez I'm no Go developer but I'm thinking of taking a crack at this issue as it would improve a workflow of ours. There's a couple of approaches that could be taken:

  1. The working directory of the command is set to the directory of the skaffold.yaml that defines the hook
  2. The same, but instead of setting the working directory, introduce an environment variable that has the full path configuration dependency, something like $SKAFFOLD_CONFIG_PATH.

Personally, 1 feels more in line with the ethos of the project, but it is a semver breaking change. 2 is non breaking. Any advice / opinions on which approach is more suitable here?

wtaylor avatar Mar 08 '23 15:03 wtaylor

@wtaylor option 2 sounds neat to me

ericzzzzzzz avatar May 08 '23 17:05 ericzzzzzzz

@wtaylor Option 2 would allow more flexibility where hooks could know about the "required path" and the "requiree path"

tjorvi avatar Jun 07 '23 11:06 tjorvi

Note that adding dir: . to a hook specification currently makes the hook run in the directory of the required skaffold file.

tjorvi avatar Jun 07 '23 11:06 tjorvi