Skip to content

Conversation

@nmaludy
Copy link
Member

@nmaludy nmaludy commented May 1, 2020

Closes #4932

This is a fix for the bug found in #4932 .

I had to revert 3x fields in the workflow DB model:

  1. WorkflowExecutionDB.spec for any fields/dict keys that were statically defined in the workflow spec that include a . in their name:
    Example:
vars:
  - vars.key.with.periods: "vars.value.with.periods"
  - vars.nested.with.periods:
      nested.periods: "vars.nested.value.with.periods"

output:
  - wf.hostname.with.periods: "{{ ctx()['published.hosts'] }}"
  - wf.output.with.periods: "{{ ctx()['published.field'] }}"
  1. WorkflowExecutionDB.context for any dicts/objects that is present in the "context", ie. from a published variable. This can occur, for example, when running core.remote. The resulting dictionary has hostnames/IPs as keys in the returned dict.
    Example:
  run:
    action: core.remote
    input:
      cmd: date
      hosts: "hostname.domain.tld,hostname2.domain.tld"
    next:
      - when: "{{ succeeded() }}"
        publish:
          - published.hosts: "{{ result().result }}"
          - published.field: "{{ result().result['hostname2.domain.tld']['stdout'] }}"
  1. TaskExecutionDB.task_spec for any inputs passed into the task that may have a dict with a key contain a ..
    Example:

  bolt_plan_run:
    action: bolt.plan_run
    input:
      plan: "{{ ctx().plan }}"
      targets: "{{ ctx().targets }}"
      env:
        ST2KV_CONFIG:
          ssh.username: system.linux.username
          ssh.password: system.linux.password

I've added a single integration test case examples.orequest-test-field-escaping that invokes these three corner cases.

@nmaludy nmaludy requested review from arm4b, blag, m4dcoder and punkrokk May 1, 2020 19:35
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label May 1, 2020
@nmaludy nmaludy self-assigned this May 1, 2020
@nmaludy nmaludy added this to the 3.2.1 milestone May 1, 2020
Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@winem
Copy link
Contributor

winem commented May 1, 2020

LGTM!

And I successfully tested the fix on one of my dev nodes. I just added an additional variable some.value to the packs/actions/workflows/install.yaml

Without the fix:
Selection_107

With the fix applied:
Selection_109

@nmaludy
Copy link
Member Author

nmaludy commented May 4, 2020

FYI build failures were caused by poisoned Travis cache where pip==20.1 was installed and cached. Travis uses a hierarchical cache where if it can't find a cache for the current PR, it uses the cache from master. Since pip==20.1 breaks our fixate-requirements.py this caused both the master build and this PRs build to fail.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR definitely needs a Changelog describing the bugfix.

@nmaludy
Copy link
Member Author

nmaludy commented May 4, 2020

@armab Added a changelog. Thank you for noticing this, i was in such a rush to get it fixed i forgot!

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks!

@arm4b arm4b modified the milestone: 3.2.1 May 4, 2020
@nmaludy nmaludy merged commit 9ea4173 into StackStorm:master May 5, 2020
@nmaludy nmaludy deleted the hotfix/4932-orquesta-field-escaping branch May 5, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug regression size/L PR that changes 100-499 lines. Requires some effort to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Orquesta vars can no longer contain '.' in 3.2

6 participants