isn't there a bool type we can use?

Otherwise looks good to me :)

isn't there a bool type we can use?

Bool, what is that? :-) I can fit that change in I suppose...

Are you sure we should not evaluate the filter at this point. the entry comes from dirsync, that should return a full entry not a MOD

Are you sure we should not evaluate the filter at this point. the entry comes from dirsync, that should return a full entry not a MOD

It's not a full entry though. On a mod update we only send a partial entry. But feel free to setup winsync and see the behavior for itself. I can send you the details on how to do it...

Are you sure we should not evaluate the filter at this point. the entry comes from dirsync, that should return a full entry not a MOD

It's not a full entry though. On a mod update we only send a partial entry. But feel free to setup winsync and see the behavior for itself. I can send you the details on how to do it...

Let me try and explain this again. I thought the description was pretty good, but it's obviously not clear (my bad). When we get the entries from dirsync we use the fitter to get those entries from AD. The bug is that we re-evaluated the same filter on the entries we just grabbed using that filter(it's redundant for one), but the entries returned only contain the attributes that are modified.

So we call

windows_dirsync_inc_run() -> send_dirsync_search() -> windows_process_dirsync_entry() ->
is_subject_of_agreement_remote()

send_dirsync_search() gets entries from AD using the winFilter, so why do we need to check it again is one issue, the other issue is that entry is partial because it is just a mod, so the filter will fail on the partial modified entry. We can not check the partial entry using the filter, but we don't need to, we already used the filter to find the entry in the first place.

I hope this helps clear this up.

@mreynolds you initial description was good and I agree that checking again the filter is useless. What was surprising is that the returned entry is partial. Googling I did not find good description but I trust you :). It could also be a question of ACI that we can search but not read the attribute.

Anyway the fix looks valid to me. ACK

@mreynolds you initial description was good and I agree that checking again the filter is useless. What was surprising is that the returned entry is partial.

I was VERY surprised by this as well. But think about it, we only get "entries" from AD, we don't get "mods". So how do you represent a MOD? Just send a partial entry with what changed. Otherwise you need to delete and re-add the entry.

As for ACI's, we don't check ACI's for replicated operations, so why would we do it for winsync replication? And we do bind as cn=administrator,cn=users,dc=AD,dc=TEST to AD so we have full access anyway. So I don't think we need to worry about ACI's in this case.

The partial entry contains only the changed attributes. That is an elegant way to reduce the work of a client to synchronize the entry. It should only do MOD_REPL on received attributes.
I think it would make sense to support that control on 389-DS for example with sync_repl.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4186

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.