[SSSD] [PATCH] AD: Cache gpo version; only download policy files if version changes

Yassir Elley yelley at redhat.com
Wed Jul 30 12:43:30 UTC 2014



----- Original Message -----
> On Tue, Jul 29, 2014 at 03:28:37PM -0400, Yassir Elley wrote:
> > 
> > 
> > ----- Original Message -----
> > > On Tue, Jul 29, 2014 at 10:01:03AM +0200, Jakub Hrozek wrote:
> > > > On Mon, Jul 28, 2014 at 10:19:38PM -0400, Yassir Elley wrote:
> > > > > 
> > > > > 
> > > > > ----- Original Message -----
> > > > > > On Mon, Jul 28, 2014 at 08:48:37PM +0200, Jakub Hrozek wrote:
> > > > > > > This is the contents of the file as downloaded on my machine.
> > > > > > > Note
> > > > > > > the
> > > > > > > garbage (?) at the beginning of the file:
> > > > > > 
> > > > > > Replying to myself after I looked at the contents more closely --
> > > > > > the
> > > > > > file just seems to begin with 0xFFFE which seems to indicate
> > > > > > UTF-16,
> > > > > > little endian. Sorry for the noise, I should have done my research
> > > > > > sooner. But the question about SID <-> name resolution still
> > > > > > stands.
> > > > > > _______________________________________________
> > > > > > sssd-devel mailing list
> > > > > > sssd-devel at lists.fedorahosted.org
> > > > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > > > > 
> > > > > 
> > > > > Hi Jakub,
> > > > > 
> > > > > Hmmm. I have not seen this before. The SYSVOL files I have generated
> > > > > have
> > > > > always exclusively contained user/group SIDs (not user/group names).
> > > > > However, you may have uncovered a bug. Please tell me how you
> > > > > generated
> > > > > the GptTmpl.inf file in the SYSVOL share. Did you use the Group
> > > > > Policy
> > > > > Object Editor?? Did you edit it manually?
> > > > 
> > > > I never edited the file manually, I only used the Group Policy
> > > > Management editor.
> > > > 
> > > > btw I don't want to stall these patches any longer, I will test the
> > > > patches again using SIDs only in the GPO file and if they work, I will
> > > > ack them and we can continue pursuing this issue separately.
> > > 
> > > In case the above wasn't clear -- I did verify the file is not refreshed
> > > if the version stays the same and is correctly refreshed after modifying
> > > the GPO policy on the AD side.
> > > 
> > > So it's an ACK from me for these patches, let's investigate the SID issue
> > > separately.
> > 
> > Agreed. Are you waiting for other ACKs before pushing this patch to master?
> 
> No, I wanted to find some time to re-test again with a fresh AD install,
> but didn't. Anyway, pushed to master now:
>     b47449d8596d9573ff13e74b6984d080b414715a
>     64074e584a56611d7563667e0fcdadd215b0c922
> > 
> > Regards,
> > Yassir.
> > 
> > PS: I think a previous patch that was pushed to master ("AD: Increment
> > som_index when advancing to the next GPO") actually relied on som_index
> > changes in *this* patch. So it would be good if we could push this patch
> > soon; otherwise, we should revert the previous patch.
> 
> I thought so, too, but the patch applied OK even w/o these two on
> master..so I think we're fine. I also checked that the index is still
> incremented correctly after pushing all to master before all calls to
> ad_gpo_get_som_attrs_step()

There was a run-time dependency that would yield incorrect behavior in certain error cases, but no compile-time/patch-time dependency :)

Thanks for the review!

Regards,
Yassir.



More information about the sssd-devel mailing list