Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 04/30/2014 05:11 AM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/29/2014 04:27 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/23/2014 08:52 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/09/2014 11:29 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob Works great here! The python-kerberos dependency issue still needs to be solved. Build is on the way to updates-testing if you can give it a go. The man page says: Copy ipa-smartproxy-apache.conf to /etc/httpd/conf.d/ipa-smartproxy.conf. It would be nice to put the whole path here so people don't have to search for the file. Done. The Configure Apache to use smartproxy line looks like a step to be performed. It could use some emphasis to make it look like a header. I combined it with the subsequent sentence so hopefully it is a bit clearer. I also added a bit on testing so you can confirm that things are working. Side note, cherrypy's routing makes requests like this possible: http POST :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass Should that be allowed? It is definitely ugly but AFAICT it isn't illegal. The zero content-length bothers me more than this horrible-looking URI. It definitely requires some understanding of the ordering of parameters to get this call right. rob Everything works now! Except one thing: json_encode_binary recently got a mandatory version argument. For code that's part of IPA, it should be fine to just use API_VERSION here. I tested with
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 04/30/2014 04:57 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/30/2014 05:11 AM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/29/2014 04:27 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/23/2014 08:52 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/09/2014 11:29 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob Works great here! The python-kerberos dependency issue still needs to be solved. Build is on the way to updates-testing if you can give it a go. The man page says: Copy ipa-smartproxy-apache.conf to /etc/httpd/conf.d/ipa-smartproxy.conf. It would be nice to put the whole path here so people don't have to search for the file. Done. The Configure Apache to use smartproxy line looks like a step to be performed. It could use some emphasis to make it look like a header. I combined it with the subsequent sentence so hopefully it is a bit clearer. I also added a bit on testing so you can confirm that things are working. Side note, cherrypy's routing makes requests like this possible: http POST :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass Should that be allowed? It is definitely ugly but AFAICT it isn't illegal. The zero content-length bothers me more than this horrible-looking URI. It definitely requires some understanding of the ordering of parameters to get this call right. rob Everything works now! Except one thing: json_encode_binary recently got a mandatory version argument. For code that's part
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 04/23/2014 08:52 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/09/2014 11:29 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob Works great here! The python-kerberos dependency issue still needs to be solved. Build is on the way to updates-testing if you can give it a go. The man page says: Copy ipa-smartproxy-apache.conf to /etc/httpd/conf.d/ipa-smartproxy.conf. It would be nice to put the whole path here so people don't have to search for the file. Done. The Configure Apache to use smartproxy line looks like a step to be performed. It could use some emphasis to make it look like a header. I combined it with the subsequent sentence so hopefully it is a bit clearer. I also added a bit on testing so you can confirm that things are working. Side note, cherrypy's routing makes requests like this possible: http POST :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass Should that be allowed? It is definitely ugly but AFAICT it isn't illegal. The zero content-length bothers me more than this horrible-looking URI. It definitely requires some understanding of the ordering of parameters to get this call right. rob Everything works now! Except one thing: json_encode_binary recently got a mandatory version argument. For code that's part of IPA, it should be fine to just use API_VERSION here. I tested with that added; ACK if you agree. ACK on your change. Sorry, one more set of changes, some fairly significant. This
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 04/29/2014 04:27 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/23/2014 08:52 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/09/2014 11:29 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob Works great here! The python-kerberos dependency issue still needs to be solved. Build is on the way to updates-testing if you can give it a go. The man page says: Copy ipa-smartproxy-apache.conf to /etc/httpd/conf.d/ipa-smartproxy.conf. It would be nice to put the whole path here so people don't have to search for the file. Done. The Configure Apache to use smartproxy line looks like a step to be performed. It could use some emphasis to make it look like a header. I combined it with the subsequent sentence so hopefully it is a bit clearer. I also added a bit on testing so you can confirm that things are working. Side note, cherrypy's routing makes requests like this possible: http POST :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass Should that be allowed? It is definitely ugly but AFAICT it isn't illegal. The zero content-length bothers me more than this horrible-looking URI. It definitely requires some understanding of the ordering of parameters to get this call right. rob Everything works now! Except one thing: json_encode_binary recently got a mandatory version argument. For code that's part of IPA, it should be fine to just use API_VERSION here. I tested with that added; ACK if you agree. ACK on your change. Sorry, one more
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 04/09/2014 11:29 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob Works great here! The python-kerberos dependency issue still needs to be solved. Build is on the way to updates-testing if you can give it a go. The man page says: Copy ipa-smartproxy-apache.conf to /etc/httpd/conf.d/ipa-smartproxy.conf. It would be nice to put the whole path here so people don't have to search for the file. Done. The Configure Apache to use smartproxy line looks like a step to be performed. It could use some emphasis to make it look like a header. I combined it with the subsequent sentence so hopefully it is a bit clearer. I also added a bit on testing so you can confirm that things are working. Side note, cherrypy's routing makes requests like this possible: http POST :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass Should that be allowed? It is definitely ugly but AFAICT it isn't illegal. The zero content-length bothers me more than this horrible-looking URI. It definitely requires some understanding of the ordering of parameters to get this call right. rob From 5ada61118d8e1c27aac3892fb844e770f9bbc9de Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA Foreman smartproxy server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 04/09/2014 11:29 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob Works great here! The python-kerberos dependency issue still needs to be solved. The man page says: Copy ipa-smartproxy-apache.conf to /etc/httpd/conf.d/ipa-smartproxy.conf. It would be nice to put the whole path here so people don't have to search for the file. The Configure Apache to use smartproxy line looks like a step to be performed. It could use some emphasis to make it look like a header. Side note, cherrypy's routing makes requests like this possible: http POST :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass Should that be allowed? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. rob From e0f5dffa85d22290ec9102125903dceb412f4e17 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA Foreman smartproxy server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by default requests must be local. Utilize GSS-Proxy to manage the TGT. Configuration information is in the ipa-smartproxy man page. Design: http://www.freeipa.org/page/V3/Smart_Proxy https://fedorahosted.org/freeipa/ticket/4128 --- Makefile | 5 +- freeipa.spec.in| 41 ipalib/backend.py | 3 +- ipalib/util.py | 13 +- ipatests/test_smartproxy/resttest.py | 170 +++ ipatests/test_smartproxy/test_features.py | 35 ipatests/test_smartproxy/test_host.py | 145 + ipatests/test_smartproxy/test_hostgroup.py | 97 + smartproxy/Makefile.am | 45 smartproxy/configure.ac| 75 +++ smartproxy/ipa-smartproxy-apache.conf | 27 +++ smartproxy/ipa-smartproxy.conf | 15 ++ smartproxy/ipa-smartproxy.logrotate| 11 + smartproxy/ipa-smartproxy.py | 323 + smartproxy/ipa-smartproxy.service | 12 ++ smartproxy/man/Makefile.am | 19 ++ smartproxy/man/ipa-smartproxy.1| 79 +++ smartproxy/man/ipa-smartproxy.conf.5 | 69 ++ 18 files changed, 1177 insertions(+), 7 deletions(-) create mode 100644 ipatests/test_smartproxy/resttest.py create mode 100644 ipatests/test_smartproxy/test_features.py create mode 100644 ipatests/test_smartproxy/test_host.py create mode 100644
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Rob Crittenden wrote: Petr Viktorin wrote: On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! Ok, taking a different tack on this. Rather than running it as a separate server process, run it as a WSGI app inside Apache. This required a fair bit of re-tooling and complicates the set up a little bit. I think I've got it all covered in the man page. On the python-kerberos front I've got bugs opened in Ubuntu and Debian to see if we can get the patch accepted their until (if) upstream ever takes a look. I decided to run the new WSGI app in a different process group, using the smartproxy we use for delegation. This simplifies the connection code, rather than using ldap2 like I was using, we use the RPC interface. And it provides to process separation. As a side-effect it will make running this code on platforms without GSSProxy a bit easier. rob From 62bd523303467ee8055d88ae60d7e21d406c3dc4 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA Foreman smartproxy server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by default requests must be local. Utilize GSS-Proxy to manage the TGT. Configuration information is in the ipa-smartproxy man page. Design: http://www.freeipa.org/page/V3/Smart_Proxy https://fedorahosted.org/freeipa/ticket/4128 --- Makefile | 5 +- freeipa.spec.in| 41 ipalib/util.py | 13 +- ipatests/test_smartproxy/resttest.py | 170 ipatests/test_smartproxy/test_features.py | 35 ipatests/test_smartproxy/test_host.py | 145 ++ ipatests/test_smartproxy/test_hostgroup.py | 97 + smartproxy/Makefile.am | 45 + smartproxy/configure.ac| 75 +++ smartproxy/ipa-smartproxy-apache.conf | 30 +++ smartproxy/ipa-smartproxy.conf | 15 ++ smartproxy/ipa-smartproxy.logrotate| 11 ++ smartproxy/ipa-smartproxy.py | 303 + smartproxy/ipa-smartproxy.service | 12 ++ smartproxy/man/Makefile.am | 19 ++ smartproxy/man/ipa-smartproxy.1| 77
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 03/14/2014 07:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: [...] Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. ACK on the functionality. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. Looks like you'll still need to silence pylint on f20 in that case. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob Good luck! -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 03/12/2014 07:48 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 03/10/2014 08:55 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob On my VMs, where the first request is handled properly but the server hangs on the second one. I gave you access to the machines for investigation. Sent you something out-of-band but in short, I wasn't able to reproduce on your reproducing VMs :-( Ping me tomorrow and we'll try it together. It ended up a combination of my mistake and a bug in GSSProxy. At least you found the bug. https://fedorahosted.org/gss-proxy/ticket/121 Please add the Python libraries (python-cherrypy, python-requests, python-kerberos) to BuildRequires. Otherwise the build fails due to pylint errors. Fixed. In the man page: +Create a host or user whose credentials will be used by the server to make requests and add it to the role: + + $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy + $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management' the first command should be ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy since by default the username is 'sserversmartproxy'. The problem was a missing space before smartproxy. I have the login name last in this example. Fixed. A nitpick regarding the systemd service: according to [0], Type=forking should be avoided. Is there a reason against setting Type=simple, and removing the PID file? Because I wasn't able to get this working with cherrypy daemon mode. AFAICT it forks itself and systemd wouldn't end up with the right pid so can't stop the service. And now the updated patch. The changes are super-minor. rob One last nitpick: the IPAErrors get encoded as JSON but the Content-Encoding is set to text/html. It's a one-line change so I went ahead and tested with it. ACK from me if you agree. +1 Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Not really. Having a patch file with a sequence+revision number you can refer to has its merits. Especially in a hairy thread like this one. Also one of our MUAs wrapped the lines, I had to undo that manually. Here is why I made the changes, in order: I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one. We currently completely eat GSSAPI errors, I figure we should log failures. IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy). I included your content-type change. These changes look good. I'm almost done testing but I need to call it a week. Awesome, thanks. Sorry for not catching that last time, but your patch doesn't add a *versioned* BuildRequres on python-kerberos, instead it adds a duplicate unversioned one. So lint (and thus the build) will fail if the old python-kerberos version is installed. A possible a solution to the build trouble would be to just add a lint exception now, and open a ticket to remove it later. That way the build succeeds despite the older version, and the new python-kerberos is only needed when installing freeipa-server-foreman-smartproxy. That should make everyone happy, including Martin. Unfortunately our lint exception mechanism doesn't work on modules, so this needs a somewhat nastier hack. The attaching a patch that does this (and I'm pasting a simple diff below). Does that look okay to push? I'm trying to find a better solution to all this. I may end up taking Martin's suggestion of rawhide-only to avoid this sort of thing. The deal with the smartproxy is that you can/should be able to run it on any IPA-enrolled client, so you can run it directly on the Foreman box, with the IPA server somewhere else. What this means is that someone could probably fairly easily package this up for other distributions and if we end up with a Fedora-only python-kerberos patch then smartproxy is Fedora-only as well. So I'm trying to get some movement out of upstream on this but it's been crickets for weeks. I think in the context of the calendar server PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up the nagging to get some sort of response, even if it is stop nagging us! rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 03/12/2014 12:02 PM, Petr Viktorin wrote: On 03/10/2014 08:55 PM, Rob Crittenden wrote: Rob Crittenden wrote: Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob On my VMs, where the first request is handled properly but the server hangs on the second one. I gave you access to the machines for investigation. Sent you something out-of-band but in short, I wasn't able to reproduce on your reproducing VMs :-( Ping me tomorrow and we'll try it together. It ended up a combination of my mistake and a bug in GSSProxy. At least you found the bug. https://fedorahosted.org/gss-proxy/ticket/121 Please add the Python libraries (python-cherrypy, python-requests, python-kerberos) to BuildRequires. Otherwise the build fails due to pylint errors. Fixed. In the man page: +Create a host or user whose credentials will be used by the server to make requests and add it to the role: + + $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy + $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management' the first command should be ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy since by default the username is 'sserversmartproxy'. The problem was a missing space before smartproxy. I have the login name last in this example. Fixed. A nitpick regarding the systemd service: according to [0], Type=forking should be avoided. Is there a reason against setting Type=simple, and removing the PID file? Because I wasn't able to get this working with cherrypy daemon mode. AFAICT it forks itself and systemd wouldn't end up with the right pid so can't stop the service. And now the updated patch. The changes are super-minor. rob One last nitpick: the IPAErrors get encoded as JSON but the Content-Encoding is set to text/html. It's a one-line change so I went ahead and tested with it. ACK from me if you agree. I spoke to Martin and he's still not satisfied with needing the COPR repo on f20. I think we can live with it though. Yes, he is not. I still think it is very inconvenient to unconditionally force everyone to import some COPR repo on all machines where he want to build and use FreeIPA even though they do not need freeipa-server-smartproxy at all. Here is an idea - add a conditional switch to freeipa.spec similar to CLIENT_ONLY which would control building the smartproxy and make it off by default. It would be only turned on for our F21 builds in the future. This is something I could live with. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote:
On 03/10/2014 08:55 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/27/2014 10:18 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Updated patch based on feedback from Foreman team. I added a new URI,
/features, which Foreman uses to determine what capabilities a proxy
has.
rob
On my VMs, where the first request is handled properly but the server
hangs on the second one. I gave you access to the machines for
investigation.
Sent you something out-of-band but in short, I wasn't able to reproduce
on your reproducing VMs :-( Ping me tomorrow and we'll try it together.
It ended up a combination of my mistake and a bug in GSSProxy. At least
you found the bug. https://fedorahosted.org/gss-proxy/ticket/121
Please add the Python libraries (python-cherrypy, python-requests,
python-kerberos) to BuildRequires. Otherwise the build fails due to
pylint errors.
Fixed.
In the man page:
+Create a host or user whose credentials will be used by the server to
make requests and add it to the role:
+
+ $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy
+ $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management'
the first command should be
ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy
since by default the username is 'sserversmartproxy'.
The problem was a missing space before smartproxy. I have the login name
last in this example. Fixed.
A nitpick regarding the systemd service: according to [0], Type=forking
should be avoided. Is there a reason against setting Type=simple, and
removing the PID file?
Because I wasn't able to get this working with cherrypy daemon mode.
AFAICT it forks itself and systemd wouldn't end up with the right pid so
can't stop the service.
And now the updated patch. The changes are super-minor.
rob
One last nitpick: the IPAErrors get encoded as JSON but the
Content-Encoding is set to text/html. It's a one-line change so I went
ahead and tested with it. ACK from me if you agree.
+1
Here are a couple more enhancements I'm considering, this seems simpler
than inter-diff since it is so small. Here is why I made the changes, in
order:
I doubled the calls to create the connection but one isn't in a
try/except!? Remove the obvious one.
We currently completely eat GSSAPI errors, I figure we should log failures.
IPA stores the principal in the request context so using that will save
a GSSAPI call (and as we learned, a lock in gssproxy).
I included your content-type change.
rob
diff --git a/smartproxy/ipa-smartproxy b/smartproxy/ipa-smartproxy
index d5e8f22..ba50cef 100755
--- a/smartproxy/ipa-smartproxy
+++ b/smartproxy/ipa-smartproxy
@@ -32,6 +32,7 @@ from cherrypy.process import plugins
from ipalib import api
from ipalib import errors
from ipalib import util
+from ipalib.request import context
from ipaserver.rpcserver import json_encode_binary
from ipapython.version import VERSION
@@ -83,12 +84,12 @@ def Command(command, *args, **options):
message=Not a local request
)
-if not api.Backend.rpcclient.isconnected():
-api.Backend.rpcclient.connect()
try:
if not api.Backend.rpcclient.isconnected():
api.Backend.rpcclient.connect()
except errors.CCacheError, e:
+cherrypy.log(msg=Kerberos error: %s % e,
+context='IPA', traceback=False)
raise IPAError(
status=401,
message=e
@@ -171,9 +172,11 @@ class IPAError(cherrypy.HTTPError):
error = {'message':
'Unable to handle error message type %s' %
type(self._mess
age)}
+principal = getattr(context, 'principal', None)
+response.headers[Content-Type] = application/json;charset=
utf-8
response.body = json.dumps({'error': error,
'id': 0,
-'principal': util.get_current_principal
(),
+'principal': principal,
'result': None,
'version': VERSION},
sort_keys=True, indent=2)
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob On my VMs, where the first request is handled properly but the server hangs on the second one. I gave you access to the machines for investigation. Sent you something out-of-band but in short, I wasn't able to reproduce on your reproducing VMs :-( Ping me tomorrow and we'll try it together. Please add the Python libraries (python-cherrypy, python-requests, python-kerberos) to BuildRequires. Otherwise the build fails due to pylint errors. Fixed. In the man page: +Create a host or user whose credentials will be used by the server to make requests and add it to the role: + + $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy + $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management' the first command should be ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy since by default the username is 'sserversmartproxy'. The problem was a missing space before smartproxy. I have the login name last in this example. Fixed. A nitpick regarding the systemd service: according to [0], Type=forking should be avoided. Is there a reason against setting Type=simple, and removing the PID file? Because I wasn't able to get this working with cherrypy daemon mode. AFAICT it forks itself and systemd wouldn't end up with the right pid so can't stop the service. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Rob Crittenden wrote: Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob On my VMs, where the first request is handled properly but the server hangs on the second one. I gave you access to the machines for investigation. Sent you something out-of-band but in short, I wasn't able to reproduce on your reproducing VMs :-( Ping me tomorrow and we'll try it together. Please add the Python libraries (python-cherrypy, python-requests, python-kerberos) to BuildRequires. Otherwise the build fails due to pylint errors. Fixed. In the man page: +Create a host or user whose credentials will be used by the server to make requests and add it to the role: + + $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy + $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management' the first command should be ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy since by default the username is 'sserversmartproxy'. The problem was a missing space before smartproxy. I have the login name last in this example. Fixed. A nitpick regarding the systemd service: according to [0], Type=forking should be avoided. Is there a reason against setting Type=simple, and removing the PID file? Because I wasn't able to get this working with cherrypy daemon mode. AFAICT it forks itself and systemd wouldn't end up with the right pid so can't stop the service. And now the updated patch. The changes are super-minor. rob From 1b0207e47249ce16c56012245a5be711d6cce53f Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA Foreman smartproxy server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by default requests must be local. Utilize GSS-Proxy to manage the TGT. Configuration information is in the ipa-smartproxy man page. Design: http://www.freeipa.org/page/V3/Smart_Proxy --- Makefile | 5 +- freeipa.spec.in| 39 ipalib/util.py | 13 +- ipatests/test_smartproxy/resttest.py | 170 +++ ipatests/test_smartproxy/test_features.py | 35 +++ ipatests/test_smartproxy/test_host.py | 145 + ipatests/test_smartproxy/test_hostgroup.py | 97 + smartproxy/Makefile.am | 43 smartproxy/configure.ac| 75 +++ smartproxy/ipa-smartproxy | 336 + smartproxy/ipa-smartproxy.conf | 15 ++ smartproxy/ipa-smartproxy.logrotate| 11 + smartproxy/ipa-smartproxy.service | 12 ++ smartproxy/man/Makefile.am | 19 ++ smartproxy/man/ipa-smartproxy.1| 78 +++ smartproxy/man/ipa-smartproxy.conf.5 | 72 +++ 16 files changed, 1159 insertions(+), 6 deletions(-) create mode 100644 ipatests/test_smartproxy/resttest.py create mode 100644 ipatests/test_smartproxy/test_features.py create mode 100644 ipatests/test_smartproxy/test_host.py create mode 100644 ipatests/test_smartproxy/test_hostgroup.py create mode 100644 smartproxy/Makefile.am create mode 100644 smartproxy/configure.ac create mode 100755 smartproxy/ipa-smartproxy create mode 100644 smartproxy/ipa-smartproxy.conf create mode 100644 smartproxy/ipa-smartproxy.logrotate create mode 100644 smartproxy/ipa-smartproxy.service create mode 100644 smartproxy/man/Makefile.am create mode 100644 smartproxy/man/ipa-smartproxy.1 create mode 100644 smartproxy/man/ipa-smartproxy.conf.5 diff --git a/Makefile b/Makefile index 2808f6153c1920f66dc7729f9ae6ef8798cb0e19..ee10e3aa7c5fb03e76986e8eddb96be5c5a418ce 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ include VERSION -SUBDIRS=daemons install ipapython ipa-client +SUBDIRS=daemons install ipapython ipa-client smartproxy CLIENTDIRS=ipapython ipa-client PRJ_PREFIX=freeipa @@ -74,6 +74,7 @@ bootstrap-autogen: version-update client-autogen @echo Building IPA $(IPA_VERSION) cd daemons; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR) --with-openldap; fi cd install; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi + cd smartproxy; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi client-autogen: version-update cd ipa-client; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi @@ -193,6 +194,7 @@ tarballs: local-archive cd
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: [...] Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for this would initially come out of Foreman/puppet. I'll submit a new patch with an updated spec but I think otherwise I've addressed the isuses Petr has raised. This thread has taken a lot of turns so it is very possible I missed something though :-) Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob My review is blocked because 389-ds doesn't install on Rawhide due to https://fedorahosted.org/389/ticket/47700 Noriko, do you know of a Rawhide build that includes your fix? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/28/2014 10:47 AM, Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: [...] Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for this would initially come out of Foreman/puppet. I'll submit a new patch with an updated spec but I think otherwise I've addressed the isuses Petr has raised. This thread has taken a lot of turns so it is very possible I missed something though :-) Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob My review is blocked because 389-ds doesn't install on Rawhide due to https://fedorahosted.org/389/ticket/47700 Noriko, do you know of a Rawhide build that includes your fix? Guys, if this patch still makes our master branch incompatible with F20, then it is a NACK from me. All developers run on F20, our CI runs on F20 and I do not think we can afford loosing that and forcing everyone to permanently switch to rawhide - it is too unstable. IMO the Requires and BuildRequires most be set so that RPMs are buildable and installable on F20. The only acceptable exception is when only freeipa-server-foreman-smartprox cannot be installed on F20, but otherwise everything else need to work. Thanks, Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/28/2014 12:41 PM, Martin Kosek wrote: On 02/28/2014 10:47 AM, Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: [...] Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for this would initially come out of Foreman/puppet. I'll submit a new patch with an updated spec but I think otherwise I've addressed the isuses Petr has raised. This thread has taken a lot of turns so it is very possible I missed something though :-) Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob My review is blocked because 389-ds doesn't install on Rawhide due to https://fedorahosted.org/389/ticket/47700 Noriko, do you know of a Rawhide build that includes your fix? Guys, if this patch still makes our master branch incompatible with F20, then it is a NACK from me. All developers run on F20, our CI runs on F20 and I do not think we can afford loosing that and forcing everyone to permanently switch to rawhide - it is too unstable. IMO the Requires and BuildRequires most be set so that RPMs are buildable and installable on F20. The only acceptable exception is when only freeipa-server-foreman-smartprox cannot be installed on F20, but otherwise everything else need to work. Thanks, Martin Okay, it's not a BuildRequires; IPA doesn't build because of a lint failure: ipalib/util.py - Module 'kerberos' has no 'authGSSClientInquireCred' member I guess the new get_current_principal needs to be kept out of ipalib until we move to f21. Until then we can have a lint exception; after then we need to remove it, and add BuildRequires so lint passes. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 02/28/2014 12:41 PM, Martin Kosek wrote: On 02/28/2014 10:47 AM, Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: [...] Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for this would initially come out of Foreman/puppet. I'll submit a new patch with an updated spec but I think otherwise I've addressed the isuses Petr has raised. This thread has taken a lot of turns so it is very possible I missed something though :-) Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob My review is blocked because 389-ds doesn't install on Rawhide due to https://fedorahosted.org/389/ticket/47700 Noriko, do you know of a Rawhide build that includes your fix? Guys, if this patch still makes our master branch incompatible with F20, then it is a NACK from me. All developers run on F20, our CI runs on F20 and I do not think we can afford loosing that and forcing everyone to permanently switch to rawhide - it is too unstable. IMO the Requires and BuildRequires most be set so that RPMs are buildable and installable on F20. The only acceptable exception is when only freeipa-server-foreman-smartprox cannot be installed on F20, but otherwise everything else need to work. Thanks, Martin Okay, it's not a BuildRequires; IPA doesn't build because of a lint failure: ipalib/util.py - Module 'kerberos' has no 'authGSSClientInquireCred' member I guess the new get_current_principal needs to be kept out of ipalib until we move to f21. Until then we can have a lint exception; after then we need to remove it, and add BuildRequires so lint passes. The other option is to locally rebuild python-kerberos from rawhide in F-20. Simo was a bit reluctant to put it into F-20 with the patch I added for authenticate_gss_client_inquire_cred(). We can also work on convincing him it is low risk. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On Fri, 2014-02-28 at 09:03 -0500, Rob Crittenden wrote: Petr Viktorin wrote: On 02/28/2014 12:41 PM, Martin Kosek wrote: On 02/28/2014 10:47 AM, Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: [...] Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for this would initially come out of Foreman/puppet. I'll submit a new patch with an updated spec but I think otherwise I've addressed the isuses Petr has raised. This thread has taken a lot of turns so it is very possible I missed something though :-) Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob My review is blocked because 389-ds doesn't install on Rawhide due to https://fedorahosted.org/389/ticket/47700 Noriko, do you know of a Rawhide build that includes your fix? Guys, if this patch still makes our master branch incompatible with F20, then it is a NACK from me. All developers run on F20, our CI runs on F20 and I do not think we can afford loosing that and forcing everyone to permanently switch to rawhide - it is too unstable. IMO the Requires and BuildRequires most be set so that RPMs are buildable and installable on F20. The only acceptable exception is when only freeipa-server-foreman-smartprox cannot be installed on F20, but otherwise everything else need to work. Thanks, Martin Okay, it's not a BuildRequires; IPA doesn't build because of a lint failure: ipalib/util.py - Module 'kerberos' has no 'authGSSClientInquireCred' member I guess the new get_current_principal needs to be kept out of ipalib until we move to f21. Until then we can have a lint exception; after then we need to remove it, and add BuildRequires so lint passes. The other option is to locally rebuild python-kerberos from rawhide in F-20. Simo was a bit reluctant to put it into F-20 with the patch I added for authenticate_gss_client_inquire_cred(). We can also work on convincing him it is low risk. Or you can simply provide a copr repo with the new build for f20 for the time being ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Simo Sorce wrote: On Fri, 2014-02-28 at 09:03 -0500, Rob Crittenden wrote: Petr Viktorin wrote: On 02/28/2014 12:41 PM, Martin Kosek wrote: On 02/28/2014 10:47 AM, Petr Viktorin wrote: On 02/27/2014 10:18 PM, Rob Crittenden wrote: Rob Crittenden wrote: [...] Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for this would initially come out of Foreman/puppet. I'll submit a new patch with an updated spec but I think otherwise I've addressed the isuses Petr has raised. This thread has taken a lot of turns so it is very possible I missed something though :-) Updated patch based on feedback from Foreman team. I added a new URI, /features, which Foreman uses to determine what capabilities a proxy has. rob My review is blocked because 389-ds doesn't install on Rawhide due to https://fedorahosted.org/389/ticket/47700 Noriko, do you know of a Rawhide build that includes your fix? Guys, if this patch still makes our master branch incompatible with F20, then it is a NACK from me. All developers run on F20, our CI runs on F20 and I do not think we can afford loosing that and forcing everyone to permanently switch to rawhide - it is too unstable. IMO the Requires and BuildRequires most be set so that RPMs are buildable and installable on F20. The only acceptable exception is when only freeipa-server-foreman-smartprox cannot be installed on F20, but otherwise everything else need to work. Thanks, Martin Okay, it's not a BuildRequires; IPA doesn't build because of a lint failure: ipalib/util.py - Module 'kerberos' has no 'authGSSClientInquireCred' member I guess the new get_current_principal needs to be kept out of ipalib until we move to f21. Until then we can have a lint exception; after then we need to remove it, and add BuildRequires so lint passes. The other option is to locally rebuild python-kerberos from rawhide in F-20. Simo was a bit reluctant to put it into F-20 with the patch I added for authenticate_gss_client_inquire_cred(). We can also work on convincing him it is low risk. Or you can simply provide a copr repo with the new build for f20 for the time being ? That doesn't deal with Martin's requirement that master build in F-20, and I presume by that no asterisks allowed (though we have made exceptions in the past). For a package this small, fetching from copr and rpmbuild are similar amounts of effort, IMHO. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity . Foreman otherwise supports no other authentication method, so we're blocked with this. The certs for
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/21/2014 04:37 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Ok, sounds reasonable to me. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this,
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/21/2014 11:09 AM, Martin Kosek wrote: On 02/21/2014 04:37 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Ok, sounds reasonable to me. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this, https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity .
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/21/2014 11:09 AM, Martin Kosek wrote: On 02/21/2014 04:37 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? Ok, so try to summarize this long-running thread, I'll rename the subpackage to freeipa-server-foreman-smartproxy to make it clearer what it is/does. Right now it requires manual configuration so having the package installed should have no negative impacts (other than potentially pulling in additional dependencies). I'll leave it in smartproxy for now, it's just cleaner and better integrates with ipatests IMHO. Ok, sounds reasonable to me. Foreman supports SSL client auth which is great, by cherrypy does not yet. There is a pull request to add this,
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/18/2014 07:52 AM, Martin Kosek wrote: On 02/18/2014 12:11 AM, Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob +1, this is indeed a Foreman-specific one-off. A real REST API would only look similar on the outside. (But I still see the value in making the responses simulate what a real REST API would look like.) OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? If this is a one off, specifically designed only for Foreman use case, my question is - do we really want to have this as a part of core FreeIPA repo and a part of it's core subpackages? So that when somebody installs all packages from our repo, he gets Foreman one-off installed? Installed, but definitely not started or configured. It's a few small files, I don't think it's worth it to create a whole different repo for them. Wouldn't it be better then to keep
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On Sun, 2014-02-16 at 21:54 -0500, Dmitri Pal wrote: On 02/16/2014 06:49 AM, Simo Sorce wrote: On Fri, 2014-02-14 at 16:52 -0500, Rob Crittenden wrote: - listens on port 8090, only on localhost - is unauthenticated Sorry to come late, but I am really at unease with this point. Can we do at least some form of simple authentication ? Even if it is a shared secret in a file accessible by both foreman and smartproxy ? Simo. Simo, it is such by design. The design is that foreman can connect to the local proxy in a simple way. We can do it w/o exposing completely open interfaces to the local host. The interface is local only and smart proxy explicitly checks that is it called locally byt a local process. If it were using a unix socket that can be protected by permissions I would have no qualms, but afaik this is listening on a network port on localhost. It means *any* process can connect, they are all local. The daemon by itself will then do a remote authenticate against IPA. We trust Foreman machine to make the host changes and allow it to make only these changes using access control rules on the server. I do not think we need or can change anything here. Any kind of authentication would significantly complicate integration with Foreman and I frankly do not see a value in another level of authentication. I.e. how certs or key in the file makes it more secure? By allowing only the Foreman process to successfully connect. I would rather suggest some SELInux policies that would open the REST api port to only specific labels. Sure SELinux should certainly be used, but not everybody runs SELinux. A shared file with a secret that only foreman and the proxy can access is very simple, it can even be generated on the fly at stratup, w/o requiring any special manual setup. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Simo Sorce wrote: On Thu, 2014-02-13 at 21:17 -0500, Dmitri Pal wrote: On 02/13/2014 09:02 PM, Rob Crittenden wrote: Dmitri Pal wrote: Recently we had a ticket in SSSD https://fedorahosted.org/sssd/ticket/2217 Should we have a similar one for IPA client and especially for the proxy? Proxy will be a long term running process so dealing with the stale tickets becomes important for it if replica is re-installed. Is it something that is solved in API level or on the proxy level? Should we have a separate ticket for that? Using gss-proxy so it's not our problem. We never touch tickets. rob Does gss proxy solve this problem? We do not have any special code to deal with this failure mode in GSS-Proxy, please open a ticket. Simo. https://fedorahosted.org/gss-proxy/ticket/119 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/17/2014 07:53 AM, Simo Sorce wrote: On Sun, 2014-02-16 at 21:54 -0500, Dmitri Pal wrote: On 02/16/2014 06:49 AM, Simo Sorce wrote: On Fri, 2014-02-14 at 16:52 -0500, Rob Crittenden wrote: - listens on port 8090, only on localhost - is unauthenticated Sorry to come late, but I am really at unease with this point. Can we do at least some form of simple authentication ? Even if it is a shared secret in a file accessible by both foreman and smartproxy ? Simo. Simo, it is such by design. The design is that foreman can connect to the local proxy in a simple way. We can do it w/o exposing completely open interfaces to the local host. The interface is local only and smart proxy explicitly checks that is it called locally byt a local process. If it were using a unix socket that can be protected by permissions I would have no qualms, but afaik this is listening on a network port on localhost. It means *any* process can connect, they are all local. The daemon by itself will then do a remote authenticate against IPA. We trust Foreman machine to make the host changes and allow it to make only these changes using access control rules on the server. I do not think we need or can change anything here. Any kind of authentication would significantly complicate integration with Foreman and I frankly do not see a value in another level of authentication. I.e. how certs or key in the file makes it more secure? By allowing only the Foreman process to successfully connect. I would rather suggest some SELInux policies that would open the REST api port to only specific labels. Sure SELinux should certainly be used, but not everybody runs SELinux. Right, but do we care? If you disable SELInux you open it to the whole host this is your choice. A shared file with a secret that only foreman and the proxy can access is very simple, it can even be generated on the fly at stratup, w/o requiring any special manual setup. Then you need to deal with permissions and labeling of this file and make sure that only these two can access again based on SELinux labels. And if you turn SELinux then other applications would be able to access if they can su to that user. IMO we should explore local socket path if possible first and make sure that only foreman can connect, then rely on SELInux and only if these options get exhausted start adding complexity to do the authentication of the API using shared secrets or certs. Keep in mind that the authentication was explicitly out of scope for the first round. I am generally no against it as next step. I am just against it being jammed in now. Simo. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On Mon, 2014-02-17 at 11:27 -0500, Dmitri Pal wrote: On 02/17/2014 07:53 AM, Simo Sorce wrote: On Sun, 2014-02-16 at 21:54 -0500, Dmitri Pal wrote: On 02/16/2014 06:49 AM, Simo Sorce wrote: On Fri, 2014-02-14 at 16:52 -0500, Rob Crittenden wrote: - listens on port 8090, only on localhost - is unauthenticated Sorry to come late, but I am really at unease with this point. Can we do at least some form of simple authentication ? Even if it is a shared secret in a file accessible by both foreman and smartproxy ? Simo. Simo, it is such by design. The design is that foreman can connect to the local proxy in a simple way. We can do it w/o exposing completely open interfaces to the local host. The interface is local only and smart proxy explicitly checks that is it called locally byt a local process. If it were using a unix socket that can be protected by permissions I would have no qualms, but afaik this is listening on a network port on localhost. It means *any* process can connect, they are all local. The daemon by itself will then do a remote authenticate against IPA. We trust Foreman machine to make the host changes and allow it to make only these changes using access control rules on the server. I do not think we need or can change anything here. Any kind of authentication would significantly complicate integration with Foreman and I frankly do not see a value in another level of authentication. I.e. how certs or key in the file makes it more secure? By allowing only the Foreman process to successfully connect. I would rather suggest some SELInux policies that would open the REST api port to only specific labels. Sure SELinux should certainly be used, but not everybody runs SELinux. Right, but do we care? If you disable SELInux you open it to the whole host this is your choice. SELinux is always optional, it is an add-on layer. It is ok to recommend it, but not to require it for basic protection. Sometimes we also need to put it in permissive to work around bugs, so it cannot be the only protection available. A shared file with a secret that only foreman and the proxy can access is very simple, it can even be generated on the fly at stratup, w/o requiring any special manual setup. Then you need to deal with permissions and labeling of this file and make sure that only these two can access again based on SELinux labels. And if you turn SELinux then other applications would be able to access if they can su to that user. No, this is an incorrect characterization. Although it is true we will also add SELinux protections on a file, the first thing that matters in files are permissions. Of course the file will need to be readable by the user nuder which foreman runs and the user under which the smartproxy runs. But that is doen by making the file readable by only those 2 users, either by adding an ACL or by putting the 2 users in a group and making the file readable only by the group. In either case SELinux is not the gate, it is an added protection. Aside: If an application can su to a user it is game over, but that is not the case in normal circumstances so not something we need to worry about. IMO we should explore local socket path if possible first and make sure that only foreman can connect, then rely on SELInux and only if these options get exhausted start adding complexity to do the authentication of the API using shared secrets or certs. Keep in mind that the authentication was explicitly out of scope for the first round. I am generally no against it as next step. I am just against it being jammed in now. not authenticating is ok at a prototype phase. I am bringing it up now because we are clearly getting out of prototype and we want to push upstream. I cannot sanction something for upstream that works unauthenticated when exposing a port over the network, even if we listen just to 127.0.0.1. Again SELinux is add-on, cannot be the only barrier, for the reasons stated above. Big +1 to using a socket if possible. [still has the same problems as a file (permissions and labels need to be right) but that is also why it can be instead of adding shared secret ot similar, it has protections a network port can't give]. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: On 02/14/2014 05:07 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just instantiate whatever we need. But that is for the future. rob But if it is
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: On 02/14/2014 05:07 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: On 02/14/2014 05:07 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just instantiate
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/18/2014 12:11 AM, Dmitri Pal wrote: On 02/17/2014 04:57 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 04:13 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 02:33 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/17/2014 01:21 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/14/2014 11:26 PM, Dmitri Pal wrote: +1, this was exactly my goal. It is easy to name and organize things now, difficult to do when it is live upstream and/or integrated with Foreman. I think the key for the right naming is a fact if this is really specific to Foreman or it is a truly general design usable also in other use cases. If Foreman-specific, I would go with freeipa-server-smartproxy-host or similar. If general, we can go with freeipa-server-proxy-host freeipa-server-proxy-user freeipa-server-proxy-dhcp The proxies may share the framework and only expose the requested part of the tree - which in advance gives you an option for an API separation, as compared to general freeipa-server-smartproxy. I still don't get the point of this. Are you proposing having 3 different servers, each providing a unique service? Or one service that one can turn on/off features, or something else? Do you envision this as 3 separate subpackages? There is no framework in my current patch, it is a cherrypy server that exposes parts of IPA on a given URI. It is the thinnest of layers. Then it seems to make sense to have 3 different packages that can be optionally coninstalled and would probably share the same principal, keytab and local REST API socket/port. Well, what you are designing is a central framework with plugins. What I designed is a quick-and-dirty get something up quickly. We need to pick one. The plugable method is going to require a fair bit of rework, though it will potentially pay dividends in the future. I think that we can start where you are but drive towards this architecture via refactoring. But we need to pick the right name now. Even if the architecture is not there yet we should name the package in such way that it does not preclude us from moving towards a clean architecture I described during the next iteration. Just having a hard time seeing the value. This would be like making each of the IPA plugins a separate package and somehow installing them individually. To do this you'd need at least 2 packages, a high level one with the framework and then a separate one for each data type. This could also be achieved, and likely more cleanly, without all these extra packages, as a simple configuration file. Once a package, always a package. Maybe I'm too close to the problem, but to me this is a Foreman-specific solution. The smartproxy is a Foreman creation, I don't know that anything else is using it. If you want a RESTful server, then we enable REST in IPA directly, but selectively enabling and disabling APIs is not something we've done before. It has been controlled by ACIs instead. rob We are trying to predict future here. Say we release it as you suggest. Tomorrow we point someone upstream who wants to add users to IPA from a similar proxy to this implementation and say use this as a model. And then Rich needs the same but for DNS for Designate. What would be the best? Rob if you knew that tomorrow two other developers will create similar proxies for users and DNS would you change anything? Would you provide some guidelines to them? If you are close to the problem you should be able to at least tell them: copy and paste vs. add more APIs to the same proxy. If your recommendation is copy and paste then I suspect there will be challenges of running these proxies on the same machine - they will collide on ports and sockets. If we say extend shouldn't we use a more generic name? I'd say use the existing IPA API. The only reason we have to write the smartproxy at all is to interoperate with another service that already has a well-defined remote server API which uses REST. rob OK, so you think that proxy is one off. OK I am fine with it. I was under assumption that it would be a beginning of a trend but I might be wrong as I do not have valid arguments to prove or disprove one way or another. I guess time would show. Any objections to Rob's approach? If this is a one off, specifically designed only for Foreman use case, my question is - do we really want to have this as a part of core FreeIPA repo and a part of it's core subpackages? So that when somebody installs all packages from our repo, he gets Foreman one-off installed? Wouldn't it be better then to keep the FreeIPA smartproxy in a separate package to avoid having FreeIPA core full of one-offs? In my mind, FreeIPA is a general purpose IdM solution and this part does not follow the picture... Just brainstorming here... Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On Fri, 2014-02-14 at 16:52 -0500, Rob Crittenden wrote: - listens on port 8090, only on localhost - is unauthenticated Sorry to come late, but I am really at unease with this point. Can we do at least some form of simple authentication ? Even if it is a shared secret in a file accessible by both foreman and smartproxy ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On Thu, 2014-02-13 at 21:17 -0500, Dmitri Pal wrote: On 02/13/2014 09:02 PM, Rob Crittenden wrote: Dmitri Pal wrote: Recently we had a ticket in SSSD https://fedorahosted.org/sssd/ticket/2217 Should we have a similar one for IPA client and especially for the proxy? Proxy will be a long term running process so dealing with the stale tickets becomes important for it if replica is re-installed. Is it something that is solved in API level or on the proxy level? Should we have a separate ticket for that? Using gss-proxy so it's not our problem. We never touch tickets. rob Does gss proxy solve this problem? We do not have any special code to deal with this failure mode in GSS-Proxy, please open a ticket. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/16/2014 06:49 AM, Simo Sorce wrote: On Fri, 2014-02-14 at 16:52 -0500, Rob Crittenden wrote: - listens on port 8090, only on localhost - is unauthenticated Sorry to come late, but I am really at unease with this point. Can we do at least some form of simple authentication ? Even if it is a shared secret in a file accessible by both foreman and smartproxy ? Simo. Simo, it is such by design. The interface is local only and smart proxy explicitly checks that is it called locally byt a local process. The daemon by itself will then do a remote authenticate against IPA. We trust Foreman machine to make the host changes and allow it to make only these changes using access control rules on the server. I do not think we need or can change anything here. Any kind of authentication would significantly complicate integration with Foreman and I frankly do not see a value in another level of authentication. I.e. how certs or key in the file makes it more secure? I would rather suggest some SELInux policies that would open the REST api port to only specific labels. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 11:26 PM, Dmitri Pal wrote: On 02/14/2014 05:07 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 12:07 AM, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/28/2014 09:35 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/23/2014 02:17 PM, Petr Viktorin wrote:
...
The URL endpoint /ipa/rest suggests that if we implement a complete
REST
API for IPA it would live here. Is the API supposed to be
future-compatible? (The API itself seems to be a good subset of a
complete REST API, but can we easily add an frontend with
authentication, i18n, etc. here later, and keep the limitations for
unauthenticated access?)
Perhaps /ipa/smartproxy would be a better choice?
It was future-proofing. I'm fine with changing the URI, it is
probably a good
thing to save that name.
+1 for moving to /ipa/smartproxy/rest, we will want a complete REST
interface
in ipa/rest/ in the future. I rather opened a ticket to track that:
https://fedorahosted.org/freeipa/ticket/4168
Martin
I think I've addressed most of Petr's suggestions with the exception of
the global ipa handle and I stuck with *args, **options as this is
pretty much standard in IPA calls.
Well, I can't have everything :)
Here's some quick feedback.
The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop
it if you want, I suppose it is duplication.
Please do. It's not discoverable at all. Anyone can copy it from the man
page and keep it around.
Note that I ran this past the Foreman design again and as a result added
another interface, /realm. It was my understanding that this Foreman
design wasn't set into stone but a patch is working its way through
their system that followed the spec so I went ahead and added it. It
isn't a big deal, the Host() class handles it out of the box.
I also updated the design page a bit to reflect some of the changes made.
Right now there are no plans to backport python-kerberos to F20.
Not even in a COPR?
So if this goes in, all developers would need to switch to Rawhide. I'm
not sure we're prepared for that yet.
rob
Currently we return this for errors in JSON:
{
error: {
code: 4002,
message: user with name \\admin\\ already exists,
name: DuplicateEntry
},
id: 0,
principal: ad...@idm.lab.eng.brq.redhat.com,
result: null,
version: 3.3.90GIT8e26acb
}
It would be more consitent if we used the same error dict also for REST.
The default port, 8090, is still not documented.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just instantiate whatever we need. But that is for the future. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just instantiate whatever we need. But that is for the future. rob But if it is specific for Foreman then it should have a generic name. I agree with Martin here. -- Thank you, Dmitri Pal
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just instantiate whatever we need. But that is for the future. rob But if it is specific for Foreman then it should have a generic name. I agree with Martin here. I have the feeling we're in
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 05:07 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class where we just instantiate whatever we need. But that is for the future. rob But if it is specific for Foreman then it should have a
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote:
On 02/14/2014 12:07 AM, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/28/2014 09:35 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/23/2014 02:17 PM, Petr Viktorin wrote:
...
The URL endpoint /ipa/rest suggests that if we implement a complete
REST
API for IPA it would live here. Is the API supposed to be
future-compatible? (The API itself seems to be a good subset of a
complete REST API, but can we easily add an frontend with
authentication, i18n, etc. here later, and keep the limitations for
unauthenticated access?)
Perhaps /ipa/smartproxy would be a better choice?
It was future-proofing. I'm fine with changing the URI, it is
probably a good
thing to save that name.
+1 for moving to /ipa/smartproxy/rest, we will want a complete REST
interface
in ipa/rest/ in the future. I rather opened a ticket to track that:
https://fedorahosted.org/freeipa/ticket/4168
Martin
I think I've addressed most of Petr's suggestions with the exception of
the global ipa handle and I stuck with *args, **options as this is
pretty much standard in IPA calls.
Well, I can't have everything :)
Here's some quick feedback.
The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop
it if you want, I suppose it is duplication.
Please do. It's not discoverable at all. Anyone can copy it from the man
page and keep it around.
Note that I ran this past the Foreman design again and as a result added
another interface, /realm. It was my understanding that this Foreman
design wasn't set into stone but a patch is working its way through
their system that followed the spec so I went ahead and added it. It
isn't a big deal, the Host() class handles it out of the box.
I also updated the design page a bit to reflect some of the changes made.
Right now there are no plans to backport python-kerberos to F20.
Not even in a COPR?
So if this goes in, all developers would need to switch to Rawhide. I'm
not sure we're prepared for that yet.
rob
Currently we return this for errors in JSON:
{
error: {
code: 4002,
message: user with name \\admin\\ already exists,
name: DuplicateEntry
},
id: 0,
principal: ad...@idm.lab.eng.brq.redhat.com,
result: null,
version: 3.3.90GIT8e26acb
}
It would be more consitent if we used the same error dict also for REST.
The default port, 8090, is still not documented.
Done some renaming. Still arguing about what to call this thing later in
the thread, but that's code-independent.
I fixed up the man pages a bit, re-tested everything, improved the spec
slightly and probably some other minor things I'm forgetting.
I raise a single, server-specific exception which is why I do all the
basestring/Exception handwringing in the IPAError class. I'm open to
suggestions. I'd rather not declare an error in the IPA Namespace for
this. This sort of brings us back to a previous discussion on where
errors.py should live...
rob
From 03ae1fd1b1313a96550ab95d981f468295885b3e Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Tue, 3 Dec 2013 09:14:00 -0700
Subject: [PATCH] Implement an IPA REST server
This currently server supports only host and hostgroup commands for
retrieving, adding and deleting entries.
The incoming requests are completely unauthenticated and by default
requests must be local.
Utilize GSS-Proxy to manage the TGT.
Configuration information is in the ipa-rest man page.
Design: http://www.freeipa.org/page/V3/Smart_Proxy
---
Makefile | 5 +-
freeipa.spec.in| 37 +++-
ipalib/util.py | 13 +-
ipatests/test_smartproxy/resttest.py | 170 +++
ipatests/test_smartproxy/test_host.py | 145 +
ipatests/test_smartproxy/test_hostgroup.py | 97 +
smartproxy/Makefile.am | 43
smartproxy/configure.ac| 75 +++
smartproxy/ipa-smartproxy | 324 +
smartproxy/ipa-smartproxy.conf | 15 ++
smartproxy/ipa-smartproxy.logrotate| 11 +
smartproxy/ipa-smartproxy.service | 12 ++
smartproxy/man/Makefile.am | 19 ++
smartproxy/man/ipa-smartproxy.1| 78 +++
smartproxy/man/ipa-smartproxy.conf.5 | 72 +++
15 files changed, 1109 insertions(+), 7 deletions(-)
create mode 100644 ipatests/test_smartproxy/resttest.py
create mode 100644 ipatests/test_smartproxy/test_host.py
create mode 100644 ipatests/test_smartproxy/test_hostgroup.py
create mode 100644 smartproxy/Makefile.am
create mode 100644 smartproxy/configure.ac
create mode 100755 smartproxy/ipa-smartproxy
create mode 100644 smartproxy/ipa-smartproxy.conf
create mode 100644 smartproxy/ipa-smartproxy.logrotate
create mode 100644 smartproxy/ipa-smartproxy.service
create mode 100644 smartproxy/man/Makefile.am
create mode
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/14/2014 02:26 PM, Dmitri Pal wrote: On 02/14/2014 05:07 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 04:52 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:09 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/14/2014 03:43 AM, Martin Kosek wrote: On 02/14/2014 12:07 AM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob I will leave functional testing to others, I just read the code. I am still quite concerned about the positioning, naming and branding of this feature. 1) Package name Currently, it is a host/hostgroup smartproxy, primarily for Foreman integration use case. Packaging it as freeipa-server-smartproxy may be ok, but only if we plan to use this proxy for all other projects. I.e. if we one day implement user smartproxy, it would need to be part of this otherwise it would lead to strange organization, like having freeipa-server-smartproxy and freeipa-server-smartproxy-users packages. Maybe it should be named differently? freeipa-server-foreman-smartproxy freeipa-server-smartproxy-hosts 2) ipa-rest stuff We have ipa-rest script, ipa-rest.conf, ipa-rest.service, ipa-rest man. However, I have the same concerns as above. This is too general and it may conflict with future core server needs (like when we implement core IPA REST interface - #4168). Let us name it consistently with package name: ipa-smartproxy.service ipa-smartproxy-hosts.service ipa-foreman-smartproxy.service The same for binary, man, ... 3) Man pages The same point, you brand it as IPA REST server. This is too general. To sum it up - let us chose one name/brand of this feature and let's use it consistently in FreeIPA infrastructure - subpackage name, subdirectory in git, subdirectory in ipatests, man, conf, script, names in man pages. Martin +1 I think it should be host ipa-host-smartproxy then we will be able to add other smart proxies and then combine them into one ipa-smartproxy package later if needed. This would imply we actually run separate servers for the various commands. Given that right now we're focused on just the Foreman use cases I think ipa-smartproxy is sufficient. For our purposes the smartproxy is just a thin wrapper around the IPA API. It is extensible for our needs, we just don't need to yet. But if we did, we'd do so within the cherrypy server and everything would be self-contained. rob Are you saying that we will just extend this smart proxy for other use cases like users and others? It all depends on the use case. If we're talking Foreman then yes. If something else, then we'll discuss it at the time, but it still may be able to be included. The IPA Foreman smart proxy is distinguished by a couple of things: - listens on port 8090, only on localhost - is unauthenticated - uses the /realm URI namespace I'm exposing hosts and hostgroups as well, but per the spec that Dominic came up with only /realm/:domain is required and affects only hosts. We can stick this behind Apache to get authentication, even on a specific URI if we want/need to, but the basic REST stuff can still be handled by cherrypy. We'll need to balance complexity of mixing things vs the complexity of code duplication in multiple servers, unless we come up with some sort of REST server class
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob From 9ef273d6ef13ff6fd420af6f3b673b69b20fa875 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA REST server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by default requests must be local. Utilize GSS-Proxy to manage the TGT. Configuration information is in the ipa-rest man page. Design: http://www.freeipa.org/page/V3/Smart_Proxy --- Makefile | 5 +- freeipa.spec.in| 23 +++ ipalib/util.py | 13 +- ipatests/test_smartproxy/resttest.py | 170 + ipatests/test_smartproxy/test_host.py | 145 ++ ipatests/test_smartproxy/test_hostgroup.py | 97 ++ smartproxy/Makefile.am | 43 + smartproxy/configure.ac| 75 smartproxy/gssproxy.conf.snippet | 6 + smartproxy/ipa-rest| 293 + smartproxy/ipa-rest.conf | 15 ++ smartproxy/ipa-rest.logrotate | 11 ++ smartproxy/ipa-rest.service| 12 ++ smartproxy/man/Makefile.am | 19 ++ smartproxy/man/ipa-rest.1 | 78 smartproxy/man/ipa-rest.conf.5 | 72 +++ 16 files changed, 1071 insertions(+), 6 deletions(-) create mode 100644 ipatests/test_smartproxy/resttest.py create mode 100644 ipatests/test_smartproxy/test_host.py create mode 100644 ipatests/test_smartproxy/test_hostgroup.py create mode 100644 smartproxy/Makefile.am create mode 100644 smartproxy/configure.ac create mode 100644 smartproxy/gssproxy.conf.snippet create mode 100755 smartproxy/ipa-rest create mode 100644 smartproxy/ipa-rest.conf create mode 100644 smartproxy/ipa-rest.logrotate create mode 100644 smartproxy/ipa-rest.service create mode 100644 smartproxy/man/Makefile.am create mode 100644 smartproxy/man/ipa-rest.1 create mode 100644 smartproxy/man/ipa-rest.conf.5 diff --git a/Makefile b/Makefile index 0a300b4ba3a0706f85255d74ecc7030fe99b76e2..9d6d93ded9985052c2327adee9a206723671cb82 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ include VERSION -SUBDIRS=daemons install ipapython ipa-client +SUBDIRS=daemons install ipapython ipa-client smartproxy CLIENTDIRS=ipapython ipa-client PRJ_PREFIX=freeipa @@ -74,6 +74,7 @@ bootstrap-autogen: version-update client-autogen @echo Building IPA $(IPA_VERSION) cd daemons; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR) --with-openldap; fi cd install; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi + cd smartproxy; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi client-autogen: version-update cd ipa-client; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi @@ -194,6 +195,7 @@ tarballs: local-archive cd dist/$(TARBALL_PREFIX)/ipa-client; ../autogen.sh --prefix=/usr --sysconfdir=/etc
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/13/2014 06:07 PM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Recently we had a ticket in SSSD https://fedorahosted.org/sssd/ticket/2217 Should we have a similar one for IPA client and especially for the proxy? Proxy will be a long term running process so dealing with the stale tickets becomes important for it if replica is re-installed. Is it something that is solved in API level or on the proxy level? Should we have a separate ticket for that? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 02/13/2014 06:07 PM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Recently we had a ticket in SSSD https://fedorahosted.org/sssd/ticket/2217 Should we have a similar one for IPA client and especially for the proxy? Proxy will be a long term running process so dealing with the stale tickets becomes important for it if replica is re-installed. Is it something that is solved in API level or on the proxy level? Should we have a separate ticket for that? Using gss-proxy so it's not our problem. We never touch tickets. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 02/13/2014 09:02 PM, Rob Crittenden wrote: Dmitri Pal wrote: On 02/13/2014 06:07 PM, Rob Crittenden wrote: Martin Kosek wrote: On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin I think I've addressed most of Petr's suggestions with the exception of the global ipa handle and I stuck with *args, **options as this is pretty much standard in IPA calls. The gssproxy.conf.snippet just makes it easier to copy/paste. I can drop it if you want, I suppose it is duplication. Note that I ran this past the Foreman design again and as a result added another interface, /realm. It was my understanding that this Foreman design wasn't set into stone but a patch is working its way through their system that followed the spec so I went ahead and added it. It isn't a big deal, the Host() class handles it out of the box. I also updated the design page a bit to reflect some of the changes made. Right now there are no plans to backport python-kerberos to F20. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Recently we had a ticket in SSSD https://fedorahosted.org/sssd/ticket/2217 Should we have a similar one for IPA client and especially for the proxy? Proxy will be a long term running process so dealing with the stale tickets becomes important for it if replica is re-installed. Is it something that is solved in API level or on the proxy level? Should we have a separate ticket for that? Using gss-proxy so it's not our problem. We never touch tickets. rob Does gss proxy solve this problem? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 01/28/2014 09:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/23/2014 02:17 PM, Petr Viktorin wrote: ... The URL endpoint /ipa/rest suggests that if we implement a complete REST API for IPA it would live here. Is the API supposed to be future-compatible? (The API itself seems to be a good subset of a complete REST API, but can we easily add an frontend with authentication, i18n, etc. here later, and keep the limitations for unauthenticated access?) Perhaps /ipa/smartproxy would be a better choice? It was future-proofing. I'm fine with changing the URI, it is probably a good thing to save that name. +1 for moving to /ipa/smartproxy/rest, we will want a complete REST interface in ipa/rest/ in the future. I rather opened a ticket to track that: https://fedorahosted.org/freeipa/ticket/4168 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 01/23/2014 02:17 PM, Petr Viktorin wrote:
On 01/22/2014 08:04 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/20/2014 05:21 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/17/2014 10:24 PM, Rob Crittenden wrote:
Implement an IPA RESTful Foreman-compatible smart proxy. This exposes
hosts and hostgroups via an unauthenticated REST API. The idea is
that
this service runs on the Foreman server and only listens on local
ports.
It is a CherryPy-based server and that handles the majority of REST
for us.
I included some tests, they can be executed with: nosetests -v
smartproxy/tests
Why is it not a part of ipatests?
I can move it if it's a show-stopper. It seemed specific to this one
directory so I stuck it there. It isn't relevant for most testing and
requires some manual configuration (though CI could handle it).
Not strictly a show stopper, but please move it. At the very least it
should end up in the freeipa-tests package.
Moved.
Thanks!
A lot of the tests (integration, webUI) need manual configuration, so
this would be no exception. Of course the tests should be skipped if the
configuration was not done, and the config instructions should be added
to/linked from http://www.freeipa.org/page/Testing
Hmm, maybe. There are instructions to set up the environment in the man
page. Testing beyond that consists of ./make-test tests/test_smartproxy
I can add that testing bit once the patch is approved I suppose.
rob
Please add python-kerberos = 1.1-13 to Requires and BuildRequires;
pylint fails with lower versions.
Are there plans to release python-kerberos-1.1-13.fc20, or will this be
f21+ only?
I've noticed gssproxy-0.3.1 is not available in rawhide (but it is in
f20). I've sent a mail to Günther; meanwhile I'll use the f20 package
(may the releng gods be merciful).
Here are my thoughts on the patch.
The URL endpoint /ipa/rest suggests that if we implement a complete REST
API for IPA it would live here. Is the API supposed to be
future-compatible? (The API itself seems to be a good subset of a
complete REST API, but can we easily add an frontend with
authentication, i18n, etc. here later, and keep the limitations for
unauthenticated access?)
Perhaps /ipa/smartproxy would be a better choice?
I assume we're in control of the REST API design. Is that correct?
According to RFC 2616, the POST verb should create a sub-resource of
what's at the given URI. In other words, it would be more correct to use:
/ipa/rest/hostPOST Create a host entry {'hostname': ...}
and same for hostgroup.
Foreman's DHCP and DNS APIs seem to do this right.
Is returning HTML for errors a Foreman requirement? I think it's a weird
thing to do in an API.
I don't think IPACommand should be a class. It has no state, and it
looks to me like if one of GET/POST/DELETE happens to not be overridden
in an exposed subclass, there's a potential security issue -- any IPA
command can be called. Could you use simple functions instead?
In IPACommand.Command, there's lot of duplicate error handling blocks.
You can handle more error classes at once with:
except (errors.DuplicateEntry, errors.DNSNotARecordError) as e:
Instead of `str(e)`, please report the errors as
{e.__class__.__name__}: {e} -- the message is sometimes unusable
without the exception type (e.g. a KeyError from {}['result'] would only
have 'result' as message).
Passing *args, **options to Command closes the door for any (future)
parametrization. Consider making the signature e.g. Command(self,
command, args, options, success_stastus=200).
Instead of `if fqdn == None:` use `if fqdn is None:`.
It would be great if you passed `api` around explicitly, instead of
relying on the global instance.
Some import issues in test_smartproxy/resttest.py:
resttest.py:24:8: Unused import 'sys'
resttest.py:25:8: Unused import 'socket'
resttest.py:26:8: Unused import 'nose'
`requests` is a third-party library, python-requests in Fedora; it
should be in the specfile. (Dogtag pulls it in for now, but when
ipa-server-ca is a separate package things will break.)
According to PEP8:
Imports should be grouped in the following order:
1. standard library imports
2. related third party imports
3. local application/library specific imports [ipa* in our case]
You should put a blank line between each group of imports.
Normally I'm not pedantic about this, but it does help spotting mistakes
like the above.
I'm not a fan of the test code copied from test_xmlrpc, but I guess we
have better things to do than refactoring that :(
But there should at least be a check that skips the tests if the smart
proxy is not available.
In the ipa-rest man page, Add this to the top of the file, before any
other services doesn't specify what file should be edited.
Also, maybe the page should mention that `systemd start
ipa-rest.service` starts the server on port 8090.
It's probably an error at my side, but I
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote:
On 01/23/2014 02:17 PM, Petr Viktorin wrote:
On 01/22/2014 08:04 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/20/2014 05:21 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/17/2014 10:24 PM, Rob Crittenden wrote:
Implement an IPA RESTful Foreman-compatible smart proxy. This
exposes
hosts and hostgroups via an unauthenticated REST API. The idea is
that
this service runs on the Foreman server and only listens on local
ports.
It is a CherryPy-based server and that handles the majority of REST
for us.
I included some tests, they can be executed with: nosetests -v
smartproxy/tests
Why is it not a part of ipatests?
I can move it if it's a show-stopper. It seemed specific to this one
directory so I stuck it there. It isn't relevant for most testing and
requires some manual configuration (though CI could handle it).
Not strictly a show stopper, but please move it. At the very least it
should end up in the freeipa-tests package.
Moved.
Thanks!
A lot of the tests (integration, webUI) need manual configuration, so
this would be no exception. Of course the tests should be skipped if
the
configuration was not done, and the config instructions should be added
to/linked from http://www.freeipa.org/page/Testing
Hmm, maybe. There are instructions to set up the environment in the man
page. Testing beyond that consists of ./make-test tests/test_smartproxy
I can add that testing bit once the patch is approved I suppose.
rob
Please add python-kerberos = 1.1-13 to Requires and BuildRequires;
pylint fails with lower versions.
Are there plans to release python-kerberos-1.1-13.fc20, or will this be
f21+ only?
I've noticed gssproxy-0.3.1 is not available in rawhide (but it is in
f20). I've sent a mail to Günther; meanwhile I'll use the f20 package
(may the releng gods be merciful).
Here are my thoughts on the patch.
The URL endpoint /ipa/rest suggests that if we implement a complete REST
API for IPA it would live here. Is the API supposed to be
future-compatible? (The API itself seems to be a good subset of a
complete REST API, but can we easily add an frontend with
authentication, i18n, etc. here later, and keep the limitations for
unauthenticated access?)
Perhaps /ipa/smartproxy would be a better choice?
It was future-proofing. I'm fine with changing the URI, it is probably a
good thing to save that name.
I assume we're in control of the REST API design. Is that correct?
According to RFC 2616, the POST verb should create a sub-resource of
what's at the given URI. In other words, it would be more correct to use:
/ipa/rest/hostPOST Create a host entry {'hostname': ...}
and same for hostgroup.
Foreman's DHCP and DNS APIs seem to do this right.
Sure, mistake on my part.
Is returning HTML for errors a Foreman requirement? I think it's a weird
thing to do in an API.
Yeah, beats me. I couldn't find anything defining the output format so I
figured I'd return it in something renderable by a browser. I'm open to
suggestions. I guess there is no reason it can't be json as well.
I don't think IPACommand should be a class. It has no state, and it
looks to me like if one of GET/POST/DELETE happens to not be overridden
in an exposed subclass, there's a potential security issue -- any IPA
command can be called. Could you use simple functions instead?
I guess so. I don't see it as a security risk though, as it would
require changing the module and we already specify that this is
unauthenticated, so if someone can hack the binary, they can already
call arbitrary commands, class or no class. Still, the user will be
limited to what they can do on the IPA side.
In IPACommand.Command, there's lot of duplicate error handling blocks.
You can handle more error classes at once with:
except (errors.DuplicateEntry, errors.DNSNotARecordError) as e:
Instead of `str(e)`, please report the errors as
{e.__class__.__name__}: {e} -- the message is sometimes unusable
without the exception type (e.g. a KeyError from {}['result'] would only
have 'result' as message).
I did that in an attempt of readability since there is so much overlap.
I'll take another look and see what it looks like combined again.
Passing *args, **options to Command closes the door for any (future)
parametrization. Consider making the signature e.g. Command(self,
command, args, options, success_stastus=200).
Not sure I follow but I'll take a look.
Instead of `if fqdn == None:` use `if fqdn is None:`.
Arg!
It would be great if you passed `api` around explicitly, instead of
relying on the global instance.
Sure.
Some import issues in test_smartproxy/resttest.py:
resttest.py:24:8: Unused import 'sys'
resttest.py:25:8: Unused import 'socket'
resttest.py:26:8: Unused import 'nose'
`requests` is a third-party library, python-requests in Fedora; it
should be in the specfile. (Dogtag pulls it in for now, but when
ipa-server-ca is a separate package things will break.)
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 01/22/2014 08:04 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/20/2014 05:21 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/17/2014 10:24 PM, Rob Crittenden wrote: Implement an IPA RESTful Foreman-compatible smart proxy. This exposes hosts and hostgroups via an unauthenticated REST API. The idea is that this service runs on the Foreman server and only listens on local ports. It is a CherryPy-based server and that handles the majority of REST for us. I included some tests, they can be executed with: nosetests -v smartproxy/tests Why is it not a part of ipatests? I can move it if it's a show-stopper. It seemed specific to this one directory so I stuck it there. It isn't relevant for most testing and requires some manual configuration (though CI could handle it). Not strictly a show stopper, but please move it. At the very least it should end up in the freeipa-tests package. Moved. Thanks! A lot of the tests (integration, webUI) need manual configuration, so this would be no exception. Of course the tests should be skipped if the configuration was not done, and the config instructions should be added to/linked from http://www.freeipa.org/page/Testing Hmm, maybe. There are instructions to set up the environment in the man page. Testing beyond that consists of ./make-test tests/test_smartproxy I can add that testing bit once the patch is approved I suppose. rob Please add python-kerberos = 1.1-13 to Requires and BuildRequires; pylint fails with lower versions. Are there plans to release python-kerberos-1.1-13.fc20, or will this be f21+ only? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Petr Viktorin wrote: On 01/20/2014 05:21 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/17/2014 10:24 PM, Rob Crittenden wrote: Implement an IPA RESTful Foreman-compatible smart proxy. This exposes hosts and hostgroups via an unauthenticated REST API. The idea is that this service runs on the Foreman server and only listens on local ports. It is a CherryPy-based server and that handles the majority of REST for us. I included some tests, they can be executed with: nosetests -v smartproxy/tests Why is it not a part of ipatests? I can move it if it's a show-stopper. It seemed specific to this one directory so I stuck it there. It isn't relevant for most testing and requires some manual configuration (though CI could handle it). Not strictly a show stopper, but please move it. At the very least it should end up in the freeipa-tests package. Moved. A lot of the tests (integration, webUI) need manual configuration, so this would be no exception. Of course the tests should be skipped if the configuration was not done, and the config instructions should be added to/linked from http://www.freeipa.org/page/Testing Hmm, maybe. There are instructions to set up the environment in the man page. Testing beyond that consists of ./make-test tests/test_smartproxy I can add that testing bit once the patch is approved I suppose. rob From 3f9357e4e395940512b82af05d7abeea24d4e424 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA REST server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by default requests must be local. Utilize GSS-Proxy to manage the TGT. Configuration information is in the ipa-rest man page. Design: http://www.freeipa.org/page/V3/Smart_Proxy --- Makefile | 5 +- freeipa.spec.in| 21 +++ ipalib/util.py | 13 +- ipatests/test_smartproxy/resttest.py | 156 + ipatests/test_smartproxy/test_host.py | 144 ipatests/test_smartproxy/test_hostgroup.py | 97 +++ smartproxy/Makefile.am | 43 + smartproxy/configure.ac| 75 + smartproxy/gssproxy.conf.snippet | 6 + smartproxy/ipa-rest| 260 + smartproxy/ipa-rest.conf | 15 ++ smartproxy/ipa-rest.logrotate | 11 ++ smartproxy/ipa-rest.service| 12 ++ smartproxy/man/Makefile.am | 19 +++ smartproxy/man/ipa-rest.1 | 78 + smartproxy/man/ipa-rest.conf.5 | 72 16 files changed, 1021 insertions(+), 6 deletions(-) create mode 100644 ipatests/test_smartproxy/resttest.py create mode 100644 ipatests/test_smartproxy/test_host.py create mode 100644 ipatests/test_smartproxy/test_hostgroup.py create mode 100644 smartproxy/Makefile.am create mode 100644 smartproxy/configure.ac create mode 100644 smartproxy/gssproxy.conf.snippet create mode 100755 smartproxy/ipa-rest create mode 100644 smartproxy/ipa-rest.conf create mode 100644 smartproxy/ipa-rest.logrotate create mode 100644 smartproxy/ipa-rest.service create mode 100644 smartproxy/man/Makefile.am create mode 100644 smartproxy/man/ipa-rest.1 create mode 100644 smartproxy/man/ipa-rest.conf.5 diff --git a/Makefile b/Makefile index 0a300b4ba3a0706f85255d74ecc7030fe99b76e2..9d6d93ded9985052c2327adee9a206723671cb82 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ include VERSION -SUBDIRS=daemons install ipapython ipa-client +SUBDIRS=daemons install ipapython ipa-client smartproxy CLIENTDIRS=ipapython ipa-client PRJ_PREFIX=freeipa @@ -74,6 +74,7 @@ bootstrap-autogen: version-update client-autogen @echo Building IPA $(IPA_VERSION) cd daemons; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR) --with-openldap; fi cd install; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi + cd smartproxy; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi client-autogen: version-update cd ipa-client; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi @@ -194,6 +195,7 @@ tarballs: local-archive cd dist/$(TARBALL_PREFIX)/ipa-client; ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); make distclean cd dist/$(TARBALL_PREFIX)/daemons; ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); make distclean cd dist/$(TARBALL_PREFIX)/install; ../autogen.sh --prefix=/usr --sysconfdir=/etc
[Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Implement an IPA RESTful Foreman-compatible smart proxy. This exposes hosts and hostgroups via an unauthenticated REST API. The idea is that this service runs on the Foreman server and only listens on local ports. It is a CherryPy-based server and that handles the majority of REST for us. I included some tests, they can be executed with: nosetests -v smartproxy/tests It is installable as a separate RPM but the local machine needs to be an IPA client. Configuration instructions are in the ipa-rest.1 man page. This requires an updated python-kerberos currently only available in rawhide: python-kerberos-1.1-13.fc21 http://www.freeipa.org/page/V3/Smart_Proxy rob From 0e4e6b47d96675abefa72eedc1afbeec94e05900 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 3 Dec 2013 09:14:00 -0700 Subject: [PATCH] Implement an IPA REST server This currently server supports only host and hostgroup commands for retrieving, adding and deleting entries. The incoming requests are completely unauthenticated and by default requests must be local. Utilize GSS-Proxy to manage the TGT. Configuration information is in the ipa-rest man page. Design: http://www.freeipa.org/page/V3/Smart_Proxy --- Makefile | 5 +- freeipa.spec.in| 21 +++ ipalib/util.py | 13 +- smartproxy/Makefile.am | 43 ++ smartproxy/configure.ac| 75 +++ smartproxy/gssproxy.conf.snippet | 6 + smartproxy/ipa-rest| 260 + smartproxy/ipa-rest.conf | 15 +++ smartproxy/ipa-rest.logrotate | 11 ++ smartproxy/ipa-rest.service| 12 ++ smartproxy/man/Makefile.am | 19 +++ smartproxy/man/ipa-rest.1 | 78 +++ smartproxy/man/ipa-rest.conf.5 | 72 ++ smartproxy/tests/resttest.py | 164 +++ smartproxy/tests/test_host.py | 144 smartproxy/tests/test_hostgroup.py | 97 ++ 16 files changed, 1029 insertions(+), 6 deletions(-) create mode 100644 smartproxy/Makefile.am create mode 100644 smartproxy/configure.ac create mode 100644 smartproxy/gssproxy.conf.snippet create mode 100755 smartproxy/ipa-rest create mode 100644 smartproxy/ipa-rest.conf create mode 100644 smartproxy/ipa-rest.logrotate create mode 100644 smartproxy/ipa-rest.service create mode 100644 smartproxy/man/Makefile.am create mode 100644 smartproxy/man/ipa-rest.1 create mode 100644 smartproxy/man/ipa-rest.conf.5 create mode 100644 smartproxy/tests/resttest.py create mode 100644 smartproxy/tests/test_host.py create mode 100644 smartproxy/tests/test_hostgroup.py diff --git a/Makefile b/Makefile index 0a300b4..9d6d93d 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ include VERSION -SUBDIRS=daemons install ipapython ipa-client +SUBDIRS=daemons install ipapython ipa-client smartproxy CLIENTDIRS=ipapython ipa-client PRJ_PREFIX=freeipa @@ -74,6 +74,7 @@ bootstrap-autogen: version-update client-autogen @echo Building IPA $(IPA_VERSION) cd daemons; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR) --with-openldap; fi cd install; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi + cd smartproxy; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi client-autogen: version-update cd ipa-client; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi @@ -194,6 +195,7 @@ tarballs: local-archive cd dist/$(TARBALL_PREFIX)/ipa-client; ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); make distclean cd dist/$(TARBALL_PREFIX)/daemons; ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); make distclean cd dist/$(TARBALL_PREFIX)/install; ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); make distclean + cd dist/$(TARBALL_PREFIX)/smartproxy; ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); make distclean cd dist; tar cfz sources/$(TARBALL) $(TARBALL_PREFIX) rm -rf dist/$(TARBALL_PREFIX) @@ -260,5 +262,6 @@ maintainer-clean: clean cd install $(MAKE) maintainer-clean cd ipa-client $(MAKE) maintainer-clean cd ipapython $(MAKE) maintainer-clean + cd smartproxy $(MAKE) maintainer-clean rm -f version.m4 rm -f freeipa.spec diff --git a/freeipa.spec.in b/freeipa.spec.in index 81c9672..da1e429 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -215,6 +215,17 @@ Cross-realm trusts with Active Directory in IPA require working Samba 4 installation. This package is provided for convenience to install all required dependencies at once. + +%package
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
On 01/17/2014 04:24 PM, Rob Crittenden wrote: Implement an IPA RESTful Foreman-compatible smart proxy. This exposes hosts and hostgroups via an unauthenticated REST API. The idea is that this service runs on the Foreman server and only listens on local ports. It is a CherryPy-based server and that handles the majority of REST for us. I included some tests, they can be executed with: nosetests -v smartproxy/tests It is installable as a separate RPM but the local machine needs to be an IPA client. Configuration instructions are in the ipa-rest.1 man page. This requires an updated python-kerberos currently only available in rawhide: python-kerberos-1.1-13.fc21 http://www.freeipa.org/page/V3/Smart_Proxy rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel What kind of the pre configuration it requires on IPA side. Should we setup some special permission for the host that would run this proxy? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1106 IPA REST smart proxy
Dmitri Pal wrote: On 01/17/2014 04:24 PM, Rob Crittenden wrote: Implement an IPA RESTful Foreman-compatible smart proxy. This exposes hosts and hostgroups via an unauthenticated REST API. The idea is that this service runs on the Foreman server and only listens on local ports. It is a CherryPy-based server and that handles the majority of REST for us. I included some tests, they can be executed with: nosetests -v smartproxy/tests It is installable as a separate RPM but the local machine needs to be an IPA client. Configuration instructions are in the ipa-rest.1 man page. This requires an updated python-kerberos currently only available in rawhide: python-kerberos-1.1-13.fc21 http://www.freeipa.org/page/V3/Smart_Proxy rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel What kind of the pre configuration it requires on IPA side. Should we setup some special permission for the host that would run this proxy? Nothing is required on the server. I tested this on and off a server and it is largely independent. I document how to create a role and what privileges it needs. For the time being I'm using a normal IPA user as a service user for this. If we add services to roles I'd prefer that, https://fedorahosted.org/freeipa/ticket/3164 . rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
