The "context" parameter of Action.run() isn't filled properly for asynchronous actions

Bug #1718353 reported by Renat Akhmerov
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mistral
Fix Released
High
Dougal Matthews
tripleo
Fix Released
Medium
Dougal Matthews

Bug Description

The "context" parameter of Action.run() isn't filled properly for asynchronous actions. Asynchronous actions need things like "workflow_execution_id" but this parameter is just a MistralContext object that holds security info which is in conflict with the description of this parameter (see [1]). I think we didn't fully account for that during "mistral-lib" related refactoring.

So, "context" parameter, in my opinion, should be more structured so that we know what it contains. The suggestion is to create a class in mistral-lib called something like ActionContext that would look like:

class ActionContext(object):
  def __init__(security_ctx, execution_ctx):
    ...

class SecurityContext(object):
  # Same info as we have in MistralContext class..

class ExecutionContext(object):
  def __init__(workflow_execution_id, task_execution_id, ...)
    ...

Once this is made we need to validate that pure asynchronous actions work properly (all contextual info is available) as well as ad-hoc actions built on top of them. See [2].

links:
[1] https://github.com/openstack/mistral-lib/blob/master/mistral_lib/actions/base.py#L40
[2] https://github.com/openstack/mistral/blob/master/mistral/engine/actions.py#L338

Revision history for this message
Renat Akhmerov (rakhmerov) wrote :

WIP patch which better illustrates the problem: https://review.openstack.org/#/c/505508/

description: updated
Changed in mistral:
milestone: none → queens-1
assignee: nobody → Ryan Brady (rbrady)
importance: Undecided → High
status: New → Confirmed
Dougal Matthews (d0ugal)
tags: added: tripleo
Changed in mistral:
assignee: Ryan Brady (rbrady) → Dougal Matthews (d0ugal)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to mistral-lib (master)

Fix proposed to branch: master
Review: https://review.openstack.org/506058

Changed in mistral:
status: Confirmed → In Progress
Revision history for this message
Dougal Matthews (d0ugal) wrote :

I added tripleo as the spotted design flaw will impact tripleo and we need to migrate it over.

Dougal Matthews (d0ugal)
Changed in tripleo:
importance: Undecided → Medium
assignee: nobody → Dougal Matthews (d0ugal)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to mistral (master)

Fix proposed to branch: master
Review: https://review.openstack.org/506185

Changed in tripleo:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tripleo-common (master)

Fix proposed to branch: master
Review: https://review.openstack.org/506186

Changed in tripleo:
milestone: none → queens-1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to mistral-lib (master)

Reviewed: https://review.openstack.org/506058
Committed: https://git.openstack.org/cgit/openstack/mistral-lib/commit/?id=db09c50dbe49fdf6f730ab7b5177b3ec53abb84c
Submitter: Jenkins
Branch: master

commit db09c50dbe49fdf6f730ab7b5177b3ec53abb84c
Author: Dougal Matthews <email address hidden>
Date: Thu Sep 21 09:18:42 2017 +0100

    Add a representation of the ActionContext in mistral-lib

    This allows us to better define the expected interface and provide
    additional context information to actions.

    The existing context is deprecated with this change and will output
    deprecation warnings for the old property names.

    Change-Id: Ib879ba58d4b9a04d4f5ea668ae94d79a82758919
    Partial-Bug: #1718353

tags: added: workflows
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to mistral-lib (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/509996

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to mistral-lib (stable/pike)

Reviewed: https://review.openstack.org/509996
Committed: https://git.openstack.org/cgit/openstack/mistral-lib/commit/?id=9c2d6d922a771449002b4ad76cbccd56c0aada86
Submitter: Jenkins
Branch: stable/pike

commit 9c2d6d922a771449002b4ad76cbccd56c0aada86
Author: Dougal Matthews <email address hidden>
Date: Thu Sep 21 09:18:42 2017 +0100

    Add a representation of the ActionContext in mistral-lib

    This allows us to better define the expected interface and provide
    additional context information to actions.

    The existing context is deprecated with this change and will output
    deprecation warnings for the old property names.

    Change-Id: Ib879ba58d4b9a04d4f5ea668ae94d79a82758919
    Partial-Bug: #1718353
    (cherry picked from commit db09c50dbe49fdf6f730ab7b5177b3ec53abb84c)

tags: added: in-stable-pike
Changed in tripleo:
milestone: queens-1 → queens-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to mistral-lib (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/514675

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to mistral (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/514688

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to mistral-lib (master)

Reviewed: https://review.openstack.org/514675
Committed: https://git.openstack.org/cgit/openstack/mistral-lib/commit/?id=40e593bd2016b623ce509a3db24965997422eed2
Submitter: Zuul
Branch: master

commit 40e593bd2016b623ce509a3db24965997422eed2
Author: Dougal Matthews <email address hidden>
Date: Wed Oct 18 15:51:19 2017 +0100

    Rename task_id to task_execution_id

    The previous name isn't consistent and it is confusing, tasks don't have
    an ID, but the execution for a task does.

    Related-Bug: #1718353
    Change-Id: I049cf7312c58d83e9405aaa36a6961cca006cb16

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to mistral-lib (master)

Reviewed: https://review.openstack.org/518689
Committed: https://git.openstack.org/cgit/openstack/mistral-lib/commit/?id=817461b40c4d433e3c18879e0c47c5ef3a7281c8
Submitter: Zuul
Branch: master

commit 817461b40c4d433e3c18879e0c47c5ef3a7281c8
Author: Dougal Matthews <email address hidden>
Date: Thu Nov 9 08:58:50 2017 +0000

    Provide redirection for all security context attributes

    The previous attempt at this was to redirect as little as possible.
    However, this has proven to be problematic twice. auth_cacert wasn't
    redirected, it was used by some of the OpenStack actions and thus
    blocked Mistral [1] patch that would have Mistral start using the new
    context.

    This change redirects all properties in the SecurityContext which should
    be safer for our internal actions and for end users actions.

    [1]: https://review.openstack.org/#/c/506185/14

    Partial-Bug: #1718353
    Needed-By: Ife653558bfcda794e7f37086832f70b0ad7c28a4
    Change-Id: I6057d0ce3fe4ae23468be8fb06cb85dc5f467f6b

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to mistral-lib (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/518761

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to mistral-lib (stable/pike)

Reviewed: https://review.openstack.org/518761
Committed: https://git.openstack.org/cgit/openstack/mistral-lib/commit/?id=8986ce9529f90ed5cdb52fd54a54ccc21378b320
Submitter: Zuul
Branch: stable/pike

commit 8986ce9529f90ed5cdb52fd54a54ccc21378b320
Author: Dougal Matthews <email address hidden>
Date: Thu Nov 9 08:58:50 2017 +0000

    Provide redirection for all security context attributes

    The previous attempt at this was to redirect as little as possible.
    However, this has proven to be problematic twice. auth_cacert wasn't
    redirected, it was used by some of the OpenStack actions and thus
    blocked Mistral [1] patch that would have Mistral start using the new
    context.

    This change redirects all properties in the SecurityContext which should
    be safer for our internal actions and for end users actions.

    [1]: https://review.openstack.org/#/c/506185/14

    Partial-Bug: #1718353
    Needed-By: Ife653558bfcda794e7f37086832f70b0ad7c28a4
    Change-Id: I6057d0ce3fe4ae23468be8fb06cb85dc5f467f6b
    (cherry picked from commit 817461b40c4d433e3c18879e0c47c5ef3a7281c8)

Changed in mistral:
milestone: queens-1 → queens-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on mistral (master)

Change abandoned by Dougal Matthews (<email address hidden>) on branch: master
Review: https://review.openstack.org/505508

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to mistral (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/520348

Changed in tripleo:
milestone: queens-2 → queens-3
Changed in mistral:
milestone: queens-2 → queens-3
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to mistral (master)

Reviewed: https://review.openstack.org/506185
Committed: https://git.openstack.org/cgit/openstack/mistral/commit/?id=dd4a4bd440ba3fc14f0da782b99d64e7b338a1e7
Submitter: Zuul
Branch: master

commit dd4a4bd440ba3fc14f0da782b99d64e7b338a1e7
Author: Dougal Matthews <email address hidden>
Date: Thu Sep 21 13:41:25 2017 +0100

    Pass the new ActionContext to mistral-lib

    Partial-Bug: #1718353
    Depends-On: I6057d0ce3fe4ae23468be8fb06cb85dc5f467f6b
    Change-Id: Ife653558bfcda794e7f37086832f70b0ad7c28a4

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to mistral (master)

Reviewed: https://review.openstack.org/520348
Committed: https://git.openstack.org/cgit/openstack/mistral/commit/?id=f9457b8cc608f4ae355c1219d214057e02668f35
Submitter: Zuul
Branch: master

commit f9457b8cc608f4ae355c1219d214057e02668f35
Author: Renat Akhmerov <email address hidden>
Date: Wed Sep 20 13:51:49 2017 +0700

    Use the new action context in MistralHTTPAction

    This move allows us to remove the previous method for injecting
    context information into actions. It was only used by the
    MistralHTTPAction.

    Related-Bug: #1718353
    Co-Authored-By: Renat Akhmerov <email address hidden>
    Change-Id: Ie5c39d35e5848accb24f1cd15e88a4a2dd4b69c0

Changed in tripleo:
assignee: Dougal Matthews (d0ugal) → Adriano Petrich (apetrich)
Dougal Matthews (d0ugal)
Changed in mistral:
status: In Progress → Fix Committed
Changed in tripleo:
milestone: queens-3 → queens-rc1
Changed in mistral:
milestone: queens-3 → queens-rc1
Changed in mistral:
status: Fix Committed → Fix Released
Changed in tripleo:
milestone: queens-rc1 → rocky-1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/514688
Committed: https://git.openstack.org/cgit/openstack/mistral/commit/?id=629bdcc51ef84e31eab9d150784e301a61983fc8
Submitter: Zuul
Branch: master

commit 629bdcc51ef84e31eab9d150784e301a61983fc8
Author: Dougal Matthews <email address hidden>
Date: Tue Oct 24 15:08:08 2017 +0100

    Rename task_id to task_execution_id

    This updates Mistral to reflect the change in mistral-lib, which was
    done to clarify the property name. Tasks don't have an ID, but the
    execution of a task does. This makes it consistent with
    workflow_execution_id and action_execution_id

    Change-Id: I80feeaeff42b87c6ea832b92697603a9e6965086
    Related-Bug: #1718353
    Depends-On: I049cf7312c58d83e9405aaa36a6961cca006cb16

Brad P. Crochet (brad-9)
Changed in tripleo:
status: In Progress → Fix Released
status: Fix Released → In Progress
Changed in tripleo:
assignee: Adriano Petrich (apetrich) → Dougal Matthews (d0ugal)
Changed in tripleo:
milestone: rocky-1 → rocky-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-common (master)

Reviewed: https://review.openstack.org/506186
Committed: https://git.openstack.org/cgit/openstack/tripleo-common/commit/?id=55a78c5c0df9cf3771fb9097ba98144fd1c9a700
Submitter: Zuul
Branch: master

commit 55a78c5c0df9cf3771fb9097ba98144fd1c9a700
Author: Dougal Matthews <email address hidden>
Date: Thu Sep 21 14:52:21 2017 +0100

    Migrate to the new Mistral context class

    The context sent to actions by Mistral has changed. It is now a class
    with the potential to carry more context information - we only use the
    security context at present. This updates the context usage to make it
    use the security attribute.

    Closes-Bug: #1718353
    Change-Id: I759a1d7a2da0a978b5b784be9f0e58a549b51e06

Changed in tripleo:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/tripleo-common 9.1.0

This issue was fixed in the openstack/tripleo-common 9.1.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.