Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shell Command v0.21 doesn't support Process' workingDir anymore #212

Open
yuri1969 opened this issue Feb 6, 2025 · 6 comments
Open

Shell Command v0.21 doesn't support Process' workingDir anymore #212

yuri1969 opened this issue Feb 6, 2025 · 6 comments
Assignees
Labels
area/plugin Plugin-related issue or feature request bug Something isn't working

Comments

@yuri1969
Copy link

yuri1969 commented Feb 6, 2025

Describe the issue

Using Shell Plugin version 0.21 the io.kestra.plugin.scripts.shell.Commands task fails at rendering the workingDir Pebbble variable.

Execution stacktrace

io.kestra.core.exceptions.IllegalVariableEvaluationException: Unable to find `workingDir` used in the expression `["cp {{workingDir}}/data.txt {{workingDir}}/out.txt"]` at line 1
	at io.kestra.core.runners.VariableRenderer.properPebbleException(VariableRenderer.java:59)
	at io.kestra.core.runners.VariableRenderer.renderOnce(VariableRenderer.java:120)
	at io.kestra.core.runners.VariableRenderer.render(VariableRenderer.java:91)
	at io.kestra.core.runners.VariableRenderer.render(VariableRenderer.java:76)
	at io.kestra.core.runners.VariableRenderer.render(VariableRenderer.java:68)
	at io.kestra.core.runners.DefaultRunContext.render(DefaultRunContext.java:240)
	at io.kestra.core.models.property.Property.asList(Property.java:141)
	at io.kestra.core.models.property.Property.asList(Property.java:129)
	at io.kestra.core.runners.RunContextProperty.lambda$asList$2(RunContextProperty.java:105)
	at io.kestra.core.utils.Rethrow.lambda$throwFunction$4(Rethrow.java:89)
	at java.base/java.util.Optional.map(Optional.java:260)
	at io.kestra.core.runners.RunContextProperty.asList(RunContextProperty.java:105)
	at io.kestra.plugin.scripts.shell.Commands.run(Commands.java:228)
	at io.kestra.plugin.scripts.shell.Commands.run(Commands.java:22)
	at io.kestra.core.runners.WorkerTaskCallable.doCall(WorkerTaskCallable.java:78)
	at io.kestra.core.runners.AbstractWorkerCallable.call(AbstractWorkerCallable.java:62)
	at io.kestra.core.runners.WorkerSecurityService.callInSecurityContext(WorkerSecurityService.java:10)
	at io.kestra.core.runners.Worker.lambda$callJob$19(Worker.java:837)
	at io.kestra.core.trace.NoopTracer.inCurrentContext(NoopTracer.java:15)
	at io.kestra.core.runners.Worker.callJob(Worker.java:833)
	at io.kestra.core.runners.Worker.runAttempt(Worker.java:792)
	at io.kestra.core.runners.Worker.run(Worker.java:640)
	at io.kestra.core.runners.Worker.handleTask(Worker.java:332)
	at io.kestra.core.runners.Worker.lambda$run$8(Worker.java:261)
	at io.micrometer.core.instrument.internal.TimedRunnable.run(TimedRunnable.java:49)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: io.pebbletemplates.pebble.error.RootAttributeNotFoundException: Root attribute [workingDir] does not exist or can not be accessed and strict variables is set to true. (["cp {{workingDir}}/data.txt {{workingDir}}/out.txt"]:1)
	at io.pebbletemplates.pebble.node.expression.ContextVariableExpression.evaluate(ContextVariableExpression.java:44)
	at io.pebbletemplates.pebble.node.PrintNode.render(PrintNode.java:37)
	at io.pebbletemplates.pebble.node.BodyNode.render(BodyNode.java:44)
	at io.pebbletemplates.pebble.node.RootNode.render(RootNode.java:31)
	at io.pebbletemplates.pebble.template.PebbleTemplateImpl.evaluate(PebbleTemplateImpl.java:157)
	at io.pebbletemplates.pebble.template.PebbleTemplateImpl.evaluate(PebbleTemplateImpl.java:96)
	at io.kestra.core.runners.VariableRenderer.renderOnce(VariableRenderer.java:114)
	... 26 more


Note unit tests still pass correctly.

I didn't have time to properly debug the issue but it feels like the introduction of rendering the value of commands "early" in 0699cf2 might be involved.


Example flow taken directly from io.kestra.plugin.core.runner.Process docs:

id: new_shell_with_file
namespace: company.team

inputs:
  - id: file
    type: FILE

tasks:
  - id: shell
    type: io.kestra.plugin.scripts.shell.Commands
    inputFiles:
      data.txt: "{{inputs.file}}"
    outputFiles:
      - out.txt
    taskRunner:
      type: io.kestra.plugin.core.runner.Process
    commands:
      - cp {{workingDir}}/data.txt {{workingDir}}/out.txt # FIXME

Environment

  • Kestra Version: 0.21.1
@yuri1969 yuri1969 added area/plugin Plugin-related issue or feature request bug Something isn't working labels Feb 6, 2025
@kestrabot kestrabot bot added this to Issues Feb 6, 2025
@github-project-automation github-project-automation bot moved this to Backlog in Issues Feb 6, 2025
@Ben8t
Copy link
Member

Ben8t commented Feb 10, 2025

Thanks for raising this one, I'm able to reproduce even on Docker runner.

id: new_shell_with_file
namespace: qa

inputs:
  - id: file
    type: FILE

tasks:
  - id: shell
    type: io.kestra.plugin.scripts.shell.Commands
    inputFiles:
      data.txt: "{{inputs.file}}"
    outputFiles:
      - out.txt
    commands:
      - cp {{workingDir}}/data.txt out.txt # FIXME

Removing {{workingDir}} works though. @mgabelle could it be a regression with dynamic props made recenlty?

@mgabelle
Copy link
Contributor

I am not sure if it is related to dynamic properties.
I am not aware of the use of {{workingDir}} in script task. Was it happen to work before ?

@yuri1969 Which version of Kestra were you using before upgrading ?

@yuri1969
Copy link
Author

@mgabelle I've tested the workingDir variable behavior using 0.20.15.

The variable is defined by ScriptService and used by the Process TaskRunner and CommandsWrapper.

@mgabelle
Copy link
Contributor

OK I see so with the new changes we render before getting to the ScriptService class and therefore we don't have a proper rendering.

Thank's @yuri1969 for investigating this.
I will try to find a fix and provide it for the tomorrow bugfix release 👍🏻

@yuri1969
Copy link
Author

Thanks @mgabelle. Can you, please, investigate why unit tests (e.g. CommandsTest) didn't catch this regression?

@mgabelle
Copy link
Contributor

I guess I know why. Using Property.of() was hiding the pebble.
In fact when we use this method (mainly for testing) we must not put pebble in it because it will be not be rendered.

For instance :
If we have inputs=Map.of("world", "Kestra")

  • Property.of("Hello {{ world }}") will be rendered as --> "Hello {{ world }}"
  • Using the constructor it works new Property<>("Hello {{ world }}") will be rendered as --> "Hello Kestra"

Unfortunately while refactoring I'd used the 'of' static method without knowing it would have side effect with pebble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugin Plugin-related issue or feature request bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants