ansible / django-ansible-base

Apache License 2.0
11 stars 43 forks source link

fix: Skip _system user when syncing resources #521

Closed rochacbruno closed 1 month ago

rochacbruno commented 1 month ago

The endpoint /service-index/resource-types/shared.user/manifest/ must not include the _system user so it is not include on the sync task.

AlanCoding commented 1 month ago

I'll paste this here, @TheRealHaoLiu and I discussed doing this on the resource consumer side (as opposed to the resource provider).

https://github.com/ansible/awx/compare/devel...AlanCoding:awx:no_thank_you?expand=1

Doing it in the resource provider would result in fewer patches, but I consider it less robust for the long-term. What if a resource consumer does use the system user construct? If it has the user, it should sync it from the resource provider, otherwise linkages will be broken.

rochacbruno commented 1 month ago

@AlanCoding this

class AWXSyncExecutor(SyncExecutor):
    def _process_manifest_item(self, manifest_item):
        if manifest_item.resource_data.get('username', None) == '_system':
            return SyncResult(SyncStatus.NOOP, manifest_item)
        return super()._process_manifest_item(manifest_item)

Is not going to work, at that point the manifest_item will be only ansible_id, hash the resource_data is fetched after the return super()._process_manifest_item(manifest_item)

According to the conversations settings.SYSTEM_USERNAME should never sync, you are saying that there will be the case for syncing it?

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud