search

ansible / awx

AWX provides a web-based user interface, REST API, and task engine built on top of Ansible. It is one of the upstream projects for Red Hat Ansible Automation Platform.
Other
13.87k stars 3.4k forks source link

Workflow node alias wiped after node edit #13581

Closed dylanbob closed 1 year ago

dylanbob commented 1 year ago

Please confirm the following

Bug Summary

Hello and thank you for your work. It seems that the bug previously fixed by this PR merged in 21.5.0 is back. In a job template visualiser, editing a node wipes out its alias.

AWX version

21.10.2

Select the relevant components

Installation method

kubernetes

Modifications

no

Ansible version

2.12.5.post0

Operating system

Debian GNU/Linux 11 (bullseye)

Web browser

Firefox, Chrome, Edge

Steps to reproduce

From AWX web GUI:

  1. create a job template
  2. create a workflow template
  3. in that template visualiser, create a node using the job template from step 1. and give it an alias. Save.

at this point everything is fine. Then:

  1. edit this node. You will see that the alias is still there. Click next a few times then save the node. You will still see the alias from the visualiser.
  2. save then exit the template
  3. go back to the visualiser

you will see that the alias got wiped out, it is now empty

Expected results

The node alias should not be wiped every time the node is edited, it should only happen if the form field is cleared by the user.

Actual results

The node alias is wiped out (made empty)

Additional information

The bug does NOT produce on AWX version 21.5.0 with ansible version 2.12.5.post0 (same ansible version in both AWX on my environment)

mabashian commented 1 year ago

@AlanCoding it looks like the UI is making a PATCH request in this scenario and we're not sending an alias in the payload. Is it expected that this would clear the alias out? For example, if we have a node that looks like:

{
            "id": 1,
            "type": "workflow_job_template_node",
            "url": "[/api/v2/workflow_job_template_nodes/1/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/)",
            "related": {
                "labels": "[/api/v2/workflow_job_template_nodes/1/labels/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/labels/)",
                "credentials": "[/api/v2/workflow_job_template_nodes/1/credentials/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/credentials/)",
                "instance_groups": "[/api/v2/workflow_job_template_nodes/1/instance_groups/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/instance_groups/)",
                "create_approval_template": "[/api/v2/workflow_job_template_nodes/1/create_approval_template/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/create_approval_template/)",
                "success_nodes": "[/api/v2/workflow_job_template_nodes/1/success_nodes/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/success_nodes/)",
                "failure_nodes": "[/api/v2/workflow_job_template_nodes/1/failure_nodes/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/failure_nodes/)",
                "always_nodes": "[/api/v2/workflow_job_template_nodes/1/always_nodes/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/always_nodes/)",
                "unified_job_template": "[/api/v2/workflow_approval_templates/9/](https://localhost:8043/api/v2/workflow_approval_templates/9/)",
                "workflow_job_template": "[/api/v2/workflow_job_templates/8/](https://localhost:8043/api/v2/workflow_job_templates/8/)"
            },
            "summary_fields": {
                "workflow_job_template": {
                    "id": 8,
                    "name": "foo",
                    "description": ""
                },
                "unified_job_template": {
                    "id": 9,
                    "name": "abc123",
                    "description": "",
                    "unified_job_type": "workflow_approval",
                    "timeout": 0
                }
            },
            "created": "2023-02-17T19:29:48.304771Z",
            "modified": "2023-02-22T18:44:05.172239Z",
            "extra_data": {},
            "inventory": null,
            "scm_branch": null,
            "job_type": null,
            "job_tags": null,
            "skip_tags": null,
            "limit": null,
            "diff_mode": null,
            "verbosity": null,
            "execution_environment": null,
            "forks": null,
            "job_slice_count": null,
            "timeout": null,
            "workflow_job_template": 8,
            "unified_job_template": 9,
            "success_nodes": [],
            "failure_nodes": [],
            "always_nodes": [],
            "all_parents_must_converge": false,
            "identifier": "bbb"
        }

and if we make a PATCH request with a payload that looks like:

{
     "name":  "abc",
     "description":  "",
     "timeout":0
}

the resulting object looks like:

{
            "id": 1,
            "type": "workflow_job_template_node",
            "url": "[/api/v2/workflow_job_template_nodes/1/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/)",
            "related": {
                "labels": "[/api/v2/workflow_job_template_nodes/1/labels/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/labels/)",
                "credentials": "[/api/v2/workflow_job_template_nodes/1/credentials/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/credentials/)",
                "instance_groups": "[/api/v2/workflow_job_template_nodes/1/instance_groups/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/instance_groups/)",
                "create_approval_template": "[/api/v2/workflow_job_template_nodes/1/create_approval_template/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/create_approval_template/)",
                "success_nodes": "[/api/v2/workflow_job_template_nodes/1/success_nodes/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/success_nodes/)",
                "failure_nodes": "[/api/v2/workflow_job_template_nodes/1/failure_nodes/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/failure_nodes/)",
                "always_nodes": "[/api/v2/workflow_job_template_nodes/1/always_nodes/](https://localhost:8043/api/v2/workflow_job_template_nodes/1/always_nodes/)",
                "unified_job_template": "[/api/v2/workflow_approval_templates/9/](https://localhost:8043/api/v2/workflow_approval_templates/9/)",
                "workflow_job_template": "[/api/v2/workflow_job_templates/8/](https://localhost:8043/api/v2/workflow_job_templates/8/)"
            },
            "summary_fields": {
                "workflow_job_template": {
                    "id": 8,
                    "name": "foo",
                    "description": ""
                },
                "unified_job_template": {
                    "id": 9,
                    "name": "abc",
                    "description": "",
                    "unified_job_type": "workflow_approval",
                    "timeout": 0
                }
            },
            "created": "2023-02-17T19:29:48.304771Z",
            "modified": "2023-02-22T18:48:04.573170Z",
            "extra_data": {},
            "inventory": null,
            "scm_branch": null,
            "job_type": null,
            "job_tags": null,
            "skip_tags": null,
            "limit": null,
            "diff_mode": null,
            "verbosity": null,
            "execution_environment": null,
            "forks": null,
            "job_slice_count": null,
            "timeout": null,
            "workflow_job_template": 8,
            "unified_job_template": 9,
            "success_nodes": [],
            "failure_nodes": [],
            "always_nodes": [],
            "all_parents_must_converge": false,
            "identifier": "53cbf1b5-ac21-4f9d-b38f-17ea69ede830"
        }

Is this expected? It seems a little odd to me. cc @gamuniz since you ran into this as well.

AlanCoding commented 1 year ago

the uuid4 is implemented as the model field default.

https://github.com/ansible/awx/blob/4499a500194e96e7abb6c8307536fac671f563f7/awx/main/models/workflow.py#L182

Obviously it shouldn't reset on unrelated updates. I have no idea why it might be doing this.

gamuniz commented 1 year ago

I just tested this with dev tools open and it does a put with just the changed field:

  1. Request URL: https://fedora.local:8043/api/v2/workflow_job_template_nodes/1/

  2. Request Method: PUT

  3. Status Code: 200 OK

  4. Remote Address: 172.16.136.128:8043

  5. Referrer Policy: strict-origin-when-cross-origin

  6. {"all_parents_must_converge":false}

I think that this is the actual problem. If I do a patch the identifier remains.

AlanCoding commented 1 year ago

Let me be sure I understand what @gamuniz said...

The UI does a PUT, but does not provide all of the fields for the node. Since the idea of PUT is that it sets all the fields, it's unclear what the expectations are here, it's even unclear if the API should allow that.

As I test with an unrelated object, if I do a PUT and exclude, say, description then it resets the description to empty string. In that case, empty string is the default value. Running the uuid4 method is the "default" value for the workflow node identifier field. So surprising as this is, that appears to be the canonically accepted correct API behavior as per the Django rest framework.

gamuniz commented 1 year ago

Yep, that is what I was saying. I'd say that the correct fix here is to send a patch so that only the field being passed would be changed via the api.

On Thu, Feb 23, 2023 at 9:49 AM Alan Rominger @.***> wrote:

Let me be sure I understand what @gamuniz https://github.com/gamuniz said...

The UI does a PUT, but does not provide all of the fields for the node. Since the idea of PUT is that it sets all the fields, it's unclear what the expectations are here, it's even unclear if the API should allow that.

As I test with an unrelated object, if I do a PUT and exclude, say, description then it resets the description to empty string. In that case, empty string is the default value. Running the uuid4 method is the "default" value for the workflow node identifier field. So surprising as this is, that appears to be the canonically accepted correct API behavior as per the Django rest framework.

— Reply to this email directly, view it on GitHub https://github.com/ansible/awx/issues/13581#issuecomment-1441919757, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJSTWJG2TUGWQ65GS6KR3LWY52GDANCNFSM6AAAAAAU562R6M . You are receiving this because you were mentioned.Message ID: @.***>

-- Gabe Muniz Senior Software Maintenance Engineer