DAB Update May 23

[AlanCoding]

https://github.com/ansible/django-ansible-base/releases/tag/2024.5.23

[Alex-Izquierdo]

Seems there are two e2e tests related with roles that fail

[AlanCoding]

E           eda_api.exceptions.ApiValueError: Invalid value for `value` (shared.change_organization), must be one of ['eda.add_activation', 'eda.add_credentialtype', 'eda.add_decisionenvironment', 'eda.add_edacredential', 'eda.add_project', 'eda.add_team', 'eda.change_credentialtype', 'eda.change_decisionenvironment', 'eda.change_edacredential', 'eda.change_organization', 'eda.change_project', 'eda.change_team', 'eda.delete_activation', 'eda.delete_credentialtype', 'eda.delete_decisionenvironment', 'eda.delete_edacredential', 'eda.delete_organization', 'eda.delete_project', 'eda.delete_team', 'eda.disable_activation', 'eda.enable_activation', 'eda.member_organization', 'eda.member_team', 'eda.restart_activation', 'eda.sync_project', 'eda.view_activation', 'eda.view_auditrule', 'eda.view_credentialtype', 'eda.view_decisionenvironment', 'eda.view_edacredential', 'eda.view_organization', 'eda.view_project', 'eda.view_rulebook', 'eda.view_rulebookprocess', 'eda.view_team']

This failure relates to the resource registry configuration.

https://github.com/ansible/eda-server/blob/92bde7387858d105bd6b4645af0e70d3b2d00b69/src/aap_eda/api/resource_api.py#L49-L53

I don't see anything that could have changed related to that recently.

[Alex-Izquierdo]

E           eda_api.exceptions.ApiValueError: Invalid value for `value` (shared.change_organization), must be one of ['eda.add_activation', 'eda.add_credentialtype', 'eda.add_decisionenvironment', 'eda.add_edacredential', 'eda.add_project', 'eda.add_team', 'eda.change_credentialtype', 'eda.change_decisionenvironment', 'eda.change_edacredential', 'eda.change_organization', 'eda.change_project', 'eda.change_team', 'eda.delete_activation', 'eda.delete_credentialtype', 'eda.delete_decisionenvironment', 'eda.delete_edacredential', 'eda.delete_organization', 'eda.delete_project', 'eda.delete_team', 'eda.disable_activation', 'eda.enable_activation', 'eda.member_organization', 'eda.member_team', 'eda.restart_activation', 'eda.sync_project', 'eda.view_activation', 'eda.view_auditrule', 'eda.view_credentialtype', 'eda.view_decisionenvironment', 'eda.view_edacredential', 'eda.view_organization', 'eda.view_project', 'eda.view_rulebook', 'eda.view_rulebookprocess', 'eda.view_team']

This failure relates to the resource registry configuration.

https://github.com/ansible/eda-server/blob/92bde7387858d105bd6b4645af0e70d3b2d00b69/src/aap_eda/api/resource_api.py#L49-L53

I don't see anything that could have changed related to that recently.

Hi @AlanCoding, this test failure only happens here, not in the other PRs or the scheduled one, so I think it includes some change that might require to upgrade the openapi spec or the tests.

CC @dhaustein

[dhaustein]

I'll update the api client in the test suite.

[Alex-Izquierdo]

I'm concerned because it seems that the new openapi spec from this upgrade modifies the description of /role_definitions/{id}/team_assignments/ with a text that looks much less clear: From:

"description": "Use this endpoint to give a team permission to a resource or an organization.\nThe needed data is the user, the role definition, and the object id.\nThe object must be of the type specified in the role definition.\nThe type given in the role definition and the provided object_id are used\nto look up the resource.\n\nAfter creation, the assignment cannot be edited, but can be deleted to\nremove those permissions.",

To

"description": "Mixin used for related viewsets which contain a sub-list, like /organizations/N/teams/",

Ref: https://github.com/ansible/eda-qa/pull/376/files#diff-a9865368a7fc7fa33065e35b2343f10d08fb79d65205435403d0a163a3044713

[dhaustein]

@AlanCoding I'm assuming the changes to PermissionsEnum are wanted because of the dichotomy between EDA-only resources and shared resources coming from GW?

-'ADD_ACTIVATION': "eda.add_activation",
+'EDA.ADD_ACTIVATION': "eda.add_activation",

-'MEMBER_TEAM': "eda.member_team",
-'VIEW_ORGANIZATION': "eda.view_organization",
+'SHARED.MEMBER_TEAM': "shared.member_team",
+'SHARED.VIEW_ORGANIZATION': "shared.view_organization",

[AlanCoding]

@Alex-Izquierdo that's a good call-out, and we should improve the text. However, the original text you quote:

Use this endpoint to give a team permission to a resource or an organization

It looks nicer, but it was wrong. You can not use that endpoint to do this. It is a read-only endpoint.

Aesthetically, we should touch up the new help text.

https://github.com/ansible/django-ansible-base/blob/0395baa3ca50b1ded2352e37fae7c45a912d213d/ansible_base/lib/routers/association_resource_router.py#L111-L112

The confusion is due to the class inheritance here. There are 2 types of sub-lists we should care about. Those that allow association & disassociation and those that just list items and are read-only. The RelatedListMixin was used for the read-only case and also used as the base class for cases allowing association. That's why it's written weirdly when put onto the API help text. Makes sense describing the class structure, but doesn't make sense describing the endpoint.

@dhaustein I am concerned that we had eda.member_team somewhere, that never should have been the case. However, I am less certain about the (all capital) keys, I'll read further into that.

[AlanCoding]

I can't really find this line in the http://127.0.0.1:8000/api/eda/v1/openapi.json endpoint or the other OPTIONS data.

'SHARED.MEMBER_TEAM': "shared.member_team",

Maybe that's fine.

[dhaustein]

@AlanCoding The 'old' PermissionEnum before this upgrade, was/is https://github.com/ansible/eda-qa/blob/main/eda_api/model/permissions_enum.py#L53-L91 So yes it has:

'MEMBER_ORGANIZATION': "eda.member_organization",
'MEMBER_TEAM': "eda.member_team",

But I updated the api client and the tests to follow the, now fixed, PermissionEnum in https://github.com/ansible/eda-qa/pull/376

[AlanCoding]

For the view description, here, I put up https://github.com/ansible/django-ansible-base/pull/419 as a proposal.

[dhaustein]

The two failing tests in CI I fixed in https://github.com/ansible/eda-qa/pull/376 so we can merge. The thing is, since this PR comes from a fork, I cannot run a e2e-on-demand check to show it publicly, you'll have to take my word for it.

[Alex-Izquierdo]

I leave you to merge it @dhaustein in sync with https://github.com/ansible/eda-qa/pull/376 so we don't have false negatives.