Re: [Freeipa-devel] [PATCH 017] certprofile-import: do not require profileId in profile data

2015-07-31 Thread Martin Basti 

On 30/07/15 12:44, Christian Heimes wrote:

On 2015-07-24 12:41, Martin Basti wrote:

On 24/07/15 05:15, Fraser Tweedale wrote:

diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
index 
5550ed942521dbab2e783fba1570520268f9b378..fe8934690fe09499f0bacb6610d9815a2b4367a4
 100644
--- a/ipalib/plugins/certprofile.py
+++ b/ipalib/plugins/certprofile.py
@@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
  
  match = self.PROFILE_ID_PATTERN.search(options['file'])

  if match is None:
-raise errors.ValidationError(name='file',
-error=_("Profile ID is not present in profile data"))
+# no profileId found, use CLI value as profileId.
+options['file'] = u'profileId=%s\n%s' % (keys[0], options['file'])

NACK

This assignment has no external effect; `post_callback' is called
with original `options['file']' and dogtag profile import can fail
due to missing profileId.

The solution is to do the same thing in post_callback; updated patch
attached.

Thanks,
Fraser



I dont like to have the same code twice in pre and post callback.

Can you use contexmanager to store the right value in pre callback and
then use it in post callback?
(can find it in dns plugin, search for context)


Sounds good to me!

Christian

PS: Context is a fancy name for a TLS dict. ;)

ACK

Pushed to:
master: a4ade199aa594307cdd6bc43d1729cc42e92fd1e
ipa-4-2: d80e90fa5c5ad41f5f29a02c11bca7c7da269938


--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 017] certprofile-import: do not require profileId in profile data

2015-07-30 Thread Christian Heimes 
On 2015-07-24 12:41, Martin Basti wrote:
> On 24/07/15 05:15, Fraser Tweedale wrote:
>>> diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
>>> index 
>>> 5550ed942521dbab2e783fba1570520268f9b378..fe8934690fe09499f0bacb6610d9815a2b4367a4
>>>  100644
>>> --- a/ipalib/plugins/certprofile.py
>>> +++ b/ipalib/plugins/certprofile.py
>>> @@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
>>>  
>>>  match = self.PROFILE_ID_PATTERN.search(options['file'])
>>>  if match is None:
>>> -raise errors.ValidationError(name='file',
>>> -error=_("Profile ID is not present in profile data"))
>>> +# no profileId found, use CLI value as profileId.
>>> +options['file'] = u'profileId=%s\n%s' % (keys[0], 
>>> options['file'])
>> NACK
>>
>> This assignment has no external effect; `post_callback' is called
>> with original `options['file']' and dogtag profile import can fail
>> due to missing profileId.
>>
>> The solution is to do the same thing in post_callback; updated patch
>> attached.
>>
>> Thanks,
>> Fraser
>>
>>
> 
> I dont like to have the same code twice in pre and post callback.
> 
> Can you use contexmanager to store the right value in pre callback and
> then use it in post callback?
> (can find it in dns plugin, search for context)


Sounds good to me!

Christian

PS: Context is a fancy name for a TLS dict. ;)
From 1c7a67f331fb7d07f1e306e292e97b1df810958c Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Thu, 23 Jul 2015 17:48:56 +0200
Subject: [PATCH] certprofile-import: do not require profileId in profile data

certprofile-import no longer requires profileId in profile data. Instead
the profile ID from the command line is taken and added to the profile
data internally.

If profileId is set in the profile, then it still has to match the CLI
option.

https://fedorahosted.org/freeipa/ticket/5090
---
 ipalib/plugins/certprofile.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
index ae75d43d7412d0df7c09a33c16c833995d9a3fe4..658fbca3b4eb851eb5a22190c443044f6ceb8491 100644
--- a/ipalib/plugins/certprofile.py
+++ b/ipalib/plugins/certprofile.py
@@ -11,6 +11,7 @@ from ipalib.plugins.virtual import VirtualCommand
 from ipalib.plugins.baseldap import (
 LDAPObject, LDAPSearch, LDAPCreate,
 LDAPDelete, LDAPUpdate, LDAPRetrieve)
+from ipalib.request import context
 from ipalib import ngettext
 from ipalib.text import _
 from ipapython.version import API_VERSION
@@ -230,11 +231,12 @@ class certprofile_import(LDAPCreate):
 
 def pre_callback(self, ldap, dn, entry, entry_attrs, *keys, **options):
 ca_enabled_check()
+context.profile = options['file']
 
 match = self.PROFILE_ID_PATTERN.search(options['file'])
 if match is None:
-raise errors.ValidationError(name='file',
-error=_("Profile ID is not present in profile data"))
+# no profileId found, use CLI value as profileId.
+context.profile = u'profileId=%s\n%s' % (keys[0], context.profile)
 elif keys[0] != match.group(1):
 raise errors.ValidationError(name='file',
 error=_("Profile ID '%(cli_value)s' does not match profile data '%(file_value)s'")
@@ -250,7 +252,7 @@ class certprofile_import(LDAPCreate):
 """
 try:
 with self.api.Backend.ra_certprofile as profile_api:
-profile_api.create_profile(options['file'])
+profile_api.create_profile(context.profile)
 profile_api.enable_profile(keys[0])
 except:
 # something went wrong ; delete entry
-- 
2.4.3



signature.asc
Description: OpenPGP digital signature
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 017] certprofile-import: do not require profileId in profile data

2015-07-24 Thread Martin Basti 

On 24/07/15 05:15, Fraser Tweedale wrote:

diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
index 
5550ed942521dbab2e783fba1570520268f9b378..fe8934690fe09499f0bacb6610d9815a2b4367a4
 100644
--- a/ipalib/plugins/certprofile.py
+++ b/ipalib/plugins/certprofile.py
@@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
  
  match = self.PROFILE_ID_PATTERN.search(options['file'])

  if match is None:
-raise errors.ValidationError(name='file',
-error=_("Profile ID is not present in profile data"))
+# no profileId found, use CLI value as profileId.
+options['file'] = u'profileId=%s\n%s' % (keys[0], options['file'])

NACK

This assignment has no external effect; `post_callback' is called
with original `options['file']' and dogtag profile import can fail
due to missing profileId.

The solution is to do the same thing in post_callback; updated patch
attached.

Thanks,
Fraser




I dont like to have the same code twice in pre and post callback.

Can you use contexmanager to store the right value in pre callback and 
then use it in post callback?

(can find it in dns plugin, search for context)

Martin^2

--
Martin Basti

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 017] certprofile-import: do not require profileId in profile data

2015-07-24 Thread Christian Heimes 
On 2015-07-24 05:15, Fraser Tweedale wrote:
>> diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
>> index 
>> 5550ed942521dbab2e783fba1570520268f9b378..fe8934690fe09499f0bacb6610d9815a2b4367a4
>>  100644
>> --- a/ipalib/plugins/certprofile.py
>> +++ b/ipalib/plugins/certprofile.py
>> @@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
>>  
>>  match = self.PROFILE_ID_PATTERN.search(options['file'])
>>  if match is None:
>> -raise errors.ValidationError(name='file',
>> -error=_("Profile ID is not present in profile data"))
>> +# no profileId found, use CLI value as profileId.
>> +options['file'] = u'profileId=%s\n%s' % (keys[0], 
>> options['file'])
> 
> NACK
> 
> This assignment has no external effect; `post_callback' is called
> with original `options['file']' and dogtag profile import can fail
> due to missing profileId.
> 
> The solution is to do the same thing in post_callback; updated patch
> attached.

Oh, I should have noticed that myself. The options parameter is passed
in as **kwargs. The keyword arguments dict is always a flat copy.

Thanks!
Christian



signature.asc
Description: OpenPGP digital signature
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 017] certprofile-import: do not require profileId in profile data

2015-07-23 Thread Fraser Tweedale 
> diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
> index 
> 5550ed942521dbab2e783fba1570520268f9b378..fe8934690fe09499f0bacb6610d9815a2b4367a4
>  100644
> --- a/ipalib/plugins/certprofile.py
> +++ b/ipalib/plugins/certprofile.py
> @@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
>  
>  match = self.PROFILE_ID_PATTERN.search(options['file'])
>  if match is None:
> -raise errors.ValidationError(name='file',
> -error=_("Profile ID is not present in profile data"))
> +# no profileId found, use CLI value as profileId.
> +options['file'] = u'profileId=%s\n%s' % (keys[0], 
> options['file'])

NACK

This assignment has no external effect; `post_callback' is called
with original `options['file']' and dogtag profile import can fail
due to missing profileId.

The solution is to do the same thing in post_callback; updated patch
attached.

Thanks,
Fraser
From 98b422098ace7d8a405facf17b7399b07ed9362c Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Thu, 23 Jul 2015 17:48:56 +0200
Subject: [PATCH] certprofile-import: do not require profileId in profile data

certprofile-import no longer requires profileId in profile data. Instead
the profile ID from the command line is taken and added to the profile
data internally.

If profileId is set in the profile, then it still has to match the CLI
option.

https://fedorahosted.org/freeipa/ticket/5090
---
 ipalib/plugins/certprofile.py | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
index 
5550ed942521dbab2e783fba1570520268f9b378..8c24192fc9204a412c5357571eb5a274074bad39
 100644
--- a/ipalib/plugins/certprofile.py
+++ b/ipalib/plugins/certprofile.py
@@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
 
 match = self.PROFILE_ID_PATTERN.search(options['file'])
 if match is None:
-raise errors.ValidationError(name='file',
-error=_("Profile ID is not present in profile data"))
+# no profileId found, use CLI value as profileId.
+options['file'] = u'profileId=%s\n%s' % (keys[0], options['file'])
 elif keys[0] != match.group(1):
 raise errors.ValidationError(name='file',
 error=_("Profile ID '%(cli_value)s' does not match profile 
data '%(file_value)s'")
@@ -248,6 +248,11 @@ class certprofile_import(LDAPCreate):
 
 If the operation fails, remove the LDAP entry.
 """
+match = self.PROFILE_ID_PATTERN.search(options['file'])
+if match is None:
+# no profileId found, use CLI value as profileId.
+options['file'] = u'profileId=%s\n%s' % (keys[0], options['file'])
+
 try:
 with self.api.Backend.ra_certprofile as profile_api:
 profile_api.create_profile(options['file'])
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 017] certprofile-import: do not require profileId in profile data

2015-07-23 Thread Christian Heimes 
certprofile-import no longer requires profileId in profile data. Instead
the profile ID from the command line is taken and added to the profile
data internally.

If profileId is set in the profile, then it still has to match the CLI
option.

https://fedorahosted.org/freeipa/ticket/5090
From 44212c91336f2dfbfdc1b6cefea3f928ba9074e9 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Thu, 23 Jul 2015 17:48:56 +0200
Subject: [PATCH] certprofile-import: do not require profileId in profile data

certprofile-import no longer requires profileId in profile data. Instead
the profile ID from the command line is taken and added to the profile
data internally.

If profileId is set in the profile, then it still has to match the CLI
option.

https://fedorahosted.org/freeipa/ticket/5090
---
 ipalib/plugins/certprofile.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
index 5550ed942521dbab2e783fba1570520268f9b378..fe8934690fe09499f0bacb6610d9815a2b4367a4 100644
--- a/ipalib/plugins/certprofile.py
+++ b/ipalib/plugins/certprofile.py
@@ -233,8 +233,8 @@ class certprofile_import(LDAPCreate):
 
 match = self.PROFILE_ID_PATTERN.search(options['file'])
 if match is None:
-raise errors.ValidationError(name='file',
-error=_("Profile ID is not present in profile data"))
+# no profileId found, use CLI value as profileId.
+options['file'] = u'profileId=%s\n%s' % (keys[0], options['file'])
 elif keys[0] != match.group(1):
 raise errors.ValidationError(name='file',
 error=_("Profile ID '%(cli_value)s' does not match profile data '%(file_value)s'")
-- 
2.4.3



signature.asc
Description: OpenPGP digital signature
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code