[Freeipa-devel] [PATCH] 0001 Provide Kerberos over HTTP (MS-KKDCP)

[Nathaniel McCallum]

On Tue, 2015-06-23 at 08:56 -0400, Simo Sorce wrote:
> On Tue, 2015-06-23 at 11:37 +0200, Christian Heimes wrote:
> > Hi,
> > 
> > I've created a new patch that implements the KDC switch as a
> > ExecStartPre hook in httpd.service.
> > 
> > Testing:
> > If you are doing an upgrade of an existing installation, then you 
> > have
> > to run ipa-server-update first. The update creates the config file
> > /etc/ipa/kdcproxy/ipa-kdc-proxy.conf from a template.
> > 
> > /usr/libexec/ipa/ipa-httpd-kdcproxy creates / removes the symlink
> > /etc/httpd/conf.d/ipa-kdc-proxy.conf. The feature is enabled by 
> > default.
> > 
> > Disable KDC Proxy on the current host:
> > # ipa-ldap-updater /usr/share/ipa/kdcproxy-disable.ldif
> > # systemctl restart httpd.service
> > 
> > Enable KDC Proxy on the current host:
> > # ipa-ldap-updater /usr/share/ipa/kdcproxy-enable.ldif
> > # systemctl restart httpd.service
> > 
> > Regards,
> > Christian
> 
> A few questions.
> 
> Why are you using "#!/usr/bin/env python2.7" ?
> We do not use this idiom, as it breaks in some cases, at most in some
> sources that are v2 only we use "#!/usr/bin/python2", please change 
> it.
> 
> I am not sure you should really have a completely separate
> KDCProxyInstance, if I read it right that will cause httpd to be
> restarted twice. If you put KDCProxy enablement as one step of the
> httpdinstance then you will have much less code and httpd can be
> restarted only once.
> KDCProxy in general is not a separate service so instantiating it as 
> a
> full service seem wrong to me. IMO it should be just one of the many
> steps of the http instance.
> 
> The rest looks good.

I agree. One other small nitpick is that the python-kdcproxy dependency
is still wrong. Please make it depend on 0.3. 0.3 is already in RHEL
and Fedora. The only remaining step here is to push python-kdcproxy in
the same update as the next FreeIPA build.

Nathaniel