[Freeipa-devel] [PATCH 0058] interactive installer does not ignore leading/trailing whitespace
Hello,
Fix for https://fedorahosted.org/freeipa/ticket/5355
Thanks,
Gabe
From 02434fc8467bbc81313d4bda0cf0e9644c151f00 Mon Sep 17 00:00:00 2001
From: Gabe
Date: Tue, 27 Oct 2015 19:17:43 -0600
Subject: [PATCH] interactive installer does not ignore leading/trailing
whitespace
https://fedorahosted.org/freeipa/ticket/5355
---
ipapython/ipautil.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index b6fd11338f5f55402d5e4502297866f3b0cc0534..34ff339800d56673f3438a3495cdf4f54d5563d3 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -763,7 +763,7 @@ def user_input(prompt, default = None, allow_empty = True):
try:
ret = input("%s: " % prompt)
if allow_empty or ret.strip():
-return ret
+return ret.strip()
except EOFError:
if allow_empty:
return ''
@@ -776,7 +776,7 @@ def user_input(prompt, default = None, allow_empty = True):
if not ret and (allow_empty or default):
return default
elif ret.strip():
-return ret
+return ret.strip()
except EOFError:
return default
--
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
Re: [Freeipa-devel] [PATCH] enable pem=True in export_pem_cert function
Tomas Babej wrote: > On 10/26/2015 08:59 PM, Niranjan wrote: > > Greetings, > > > > export_pem_cert function from ipapython/certdb should export the > > certificate > > in pem format but instead exports the cert in der format as it doesn't > > enable pem=True. > > > > This patch specifies pem=True for export_pem_cert function > > > > Regards > > Niranjan > > Hi, > > the patch looks good, however, I'm curious as to how did you find this > bug? Does it affect anything? I am part of the CS(dogtag) QE team, and as part of CS Automation, i am relying on some generic functions provided by ipapython. While using those functions for our automation, I found it. > > It seems to me that this part of the code is a dead branch which should > be removed. I did not look further ipapython, so i am not aware where else export_pem_cert is being used, but i would like the functions in certdb.py be available. > > $ git grep export_pem_cert > ipapython/certdb.py:def export_pem_cert(self, nickname, location): > ipaserver/install/certs.py:def export_pem_cert(self, nickname, > ipaserver/install/certs.py:return self.nssdb.export_pem_ce.. > > Tomas pgpjsOjWKyWMi.pgp Description: PGP 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 0327] KRA: fix check if CA is installed on replica
On 10/27/2015 10:43 AM, Martin Basti wrote: On 26.10.2015 10:35, Martin Babinsky wrote: On 10/23/2015 05:18 PM, Martin Basti wrote: On 23.10.2015 15:15, Martin Babinsky wrote: On 10/23/2015 03:12 PM, Martin Babinsky wrote: On 10/16/2015 12:41 PM, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5345 Patch attached. I have tested it on 4-2 branch and it works as expected, ACK. Obviously, master branch would require a different patch. I actually missed your check in ipaserver/install/kra.py which can break ipa-replica-install with --setup-kra, so NACK. Updated patches attached. NACK on the master-branch patch. You forgot a 'return' in this code snippet: +if self.installing_replica: +domain_level = dsinstance.get_domain_level(api) +if domain_level > DOMAIN_LEVEL_0: +self.options.promote = True +return that would make installation abort when domain level is greter than zero. Updated patch attached. ACK. -- Martin^3 Babinsky -- 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 0091] Silence of the linters
On 10/27/2015 06:26 PM, Tomas Babej wrote: > > > On 10/27/2015 06:25 PM, Martin Babinsky wrote: >> On 10/27/2015 06:21 PM, Martin Babinsky wrote: >>> this patch silences a false-positive pylint error introduced by recent >>> python 3 porting patches (commit >>> c38516eab7b82f3ba7334cdea7a422a070048b59) >>> >>> >>> >> pep8 fail. >> >> Attaching updated patch. >> >> >> > > Thanks for the PEP8 compliant version - now we're reducing the number of > errors on all fronts! > > ACK. > > Tomas > Pushed to master: 82fd4250b9b4f408174edec7c7f070dc9fc73ab0 -- 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 0091] Silence of the linters
On 10/27/2015 06:25 PM, Martin Babinsky wrote: > On 10/27/2015 06:21 PM, Martin Babinsky wrote: >> this patch silences a false-positive pylint error introduced by recent >> python 3 porting patches (commit >> c38516eab7b82f3ba7334cdea7a422a070048b59) >> >> >> > pep8 fail. > > Attaching updated patch. > > > Thanks for the PEP8 compliant version - now we're reducing the number of errors on all fronts! ACK. Tomas -- 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 0091] Silence of the linters
On 10/27/2015 06:21 PM, Martin Babinsky wrote:
this patch silences a false-positive pylint error introduced by recent
python 3 porting patches (commit c38516eab7b82f3ba7334cdea7a422a070048b59)
pep8 fail.
Attaching updated patch.
--
Martin^3 Babinsky
From c34a8697014e4c7b00f05f660114949da7023eff Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Tue, 27 Oct 2015 18:18:26 +0100
Subject: [PATCH] silence pylint in Python 3-specific portion of ipalib/rpc.py
---
ipalib/rpc.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 2accdc5f0799a1cadab995212db59c081715e29f..3664b265a1f070a198f4af640f20212f14d6c302 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -656,7 +656,7 @@ class KerbTransport(SSLTransport):
else:
connection.putrequest("POST", handler)
headers.append(("User-Agent", self.user_agent))
-self.send_headers(connection, headers)
+self.send_headers(connection, headers) # pylint: disable=E1101
self.send_content(connection, request_body)
return connection
--
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 0091] Silence of the linters
this patch silences a false-positive pylint error introduced by recent
python 3 porting patches (commit c38516eab7b82f3ba7334cdea7a422a070048b59)
--
Martin^3 Babinsky
From abd6194bb432acbab910c2de1430ce333411ea2a Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Tue, 27 Oct 2015 18:18:26 +0100
Subject: [PATCH] silence pylint in Python 3-specific portion of ipalib/rpc.py
---
ipalib/rpc.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 2accdc5f0799a1cadab995212db59c081715e29f..59e0e1578ea54727e4c3c42c86797c47ec583a60 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -656,7 +656,7 @@ class KerbTransport(SSLTransport):
else:
connection.putrequest("POST", handler)
headers.append(("User-Agent", self.user_agent))
-self.send_headers(connection, headers)
+self.send_headers(connection, headers) # pylint: disable=E1101
self.send_content(connection, request_body)
return connection
--
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
Re: [Freeipa-devel] [PATCHSET] Replica promotion patches
On 10/27/2015 04:23 PM, Martin Babinsky wrote:
On 10/22/2015 01:06 PM, Petr Vobornik wrote:
On 10/16/2015 06:41 PM, Endi Sukma Dewata wrote:
On 10/15/2015 9:54 AM, Simo Sorce wrote:
3) ipa-ca-install fails with:
Traceback (most recent call last):
File
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 445, in start_creation
run_step(full_msg, method)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 435, in run_step
method()
File
"/usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py",
line
631, in __spawn_instance
DogtagInstance.spawn_instance(self, cfg_file)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py",
line 185, in spawn_instance
self.handle_setup_error(e)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py",
line 448, in handle_setup_error
raise RuntimeError("%s configuration failed." % self.subsystem)
RuntimeError: CA configuration failed.
I guess I'm hitting the authentication bug in Dogtag. It is
supposed to
be fixed in pki-core-10.2.6-10, but is it fixed in
pki-core-10.2.7-0.2?
We might need a new 10.2.7 build.
I am not sure which version has it fixed, Endi ?
PKI ticket #1580 was fixed in pki-core-10.2.6-10 for F23 and F24. We
never released a pki-core-10.2.7. I suppose that is a custom build?
Yes it is a custom build[4].
It was advertised that #1414[1] will be in PKI 10.2.7 but it was
laterincluded into 10.2.6-5. I don't know what's a plan for 10.2.7.
Required patch for the discussed issue #1580[2] is included in 10.2.6-10
So I propose to change requires - patch attached, remove 10.2.7 custom
build from mkosek/freeipa-master repo and add new build(for f22) based
on pki-core-10.2.6-10.fc23 from koji[3]
[1] https://fedorahosted.org/pki/ticket/1414
[2] https://fedorahosted.org/pki/ticket/1580
[3] http://koji.fedoraproject.org/koji/buildinfo?buildID=689985
[4]
https://copr.fedoraproject.org/coprs/mkosek/freeipa-master/build/121544/
ACK
Pushed to master: 3f0707a199dae98d55d8e1f69b750f2d1ed4dcab
pki-core-10.2.6-11 was built for f22 and f23 in mkosek/freeipa-master
copr [1]
pki-core-10.2.7-0.2 was removed from the copr
[1] https://copr.fedoraproject.org/coprs/mkosek/freeipa-master/build/130759/
--
Petr Vobornik
--
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] Fix ipa-ca-install bug #5397
On 23.10.2015 17:26, Simo Sorce wrote: This patch moves the check to see if a CA is already installed locally early. Simo. ACK Pushed to master: 53294aa7a7db7eff527455fc35283306b37fc777 -- 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] [PATCHES] 0743-0747 Python 3 porting
On 10/27/2015 05:22 PM, Tomas Babej wrote: > > > On 10/23/2015 08:21 PM, Petr Viktorin wrote: >> Hello, >> Another batch of py3 porting patches. >> >> With these, the only thing to fix to get ipapython tests passing will be >> handling encoding/decoding for stdin/stdout/stderr for ipautil.run(). > > ACK, it's nice to see some of that old code go. > > Tomas > Pushed to master: c38516eab7b82f3ba7334cdea7a422a070048b59 -- 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] [PATCHES] 0743-0747 Python 3 porting
On 10/23/2015 08:21 PM, Petr Viktorin wrote: > Hello, > Another batch of py3 porting patches. > > With these, the only thing to fix to get ipapython tests passing will be > handling encoding/decoding for stdin/stdout/stderr for ipautil.run(). ACK, it's nice to see some of that old code go. Tomas -- 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] enable pem=True in export_pem_cert function
On 10/26/2015 08:59 PM, Niranjan wrote: > Greetings, > > export_pem_cert function from ipapython/certdb should export the certificate > in pem format but instead exports the cert in der format as it doesn't enable > pem=True. > > This patch specifies pem=True for export_pem_cert function > > Regards > Niranjan Hi, the patch looks good, however, I'm curious as to how did you find this bug? Does it affect anything? It seems to me that this part of the code is a dead branch which should be removed. $ git grep export_pem_cert ipapython/certdb.py:def export_pem_cert(self, nickname, location): ipaserver/install/certs.py:def export_pem_cert(self, nickname, ipaserver/install/certs.py:return self.nssdb.export_pem_ce.. Tomas -- 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] [draft] Fate of ipa-replica-manage and ipa-csreplica-manage tools
On 10/27/2015 03:54 PM, Petr Vobornik wrote:
Both tools serve primarily for managing replication agreements and
replicas. ipa-replica-manage also manages winsync agreements and DNA
ranges.
FreeIPA 4.3 will introduce managed topology which affects these tools.
Let's go trough all sub-commands of both tools and decide what is the
fate of them/how they should be replaced. Comments are welcome.
In text, term 'disable' means: print an error message with help what
is the new alternative.
For domain level == 0 all sub-commands should behave the same way as
before. Proposals are for domain level 1 if not stated otherwise.
== ipa-replica-manage ==
=== list ===
Lists all IPA server or replication agreements of a specific IPA
server including winsync agreements.
Server list is replaced by
ipa server-find
Replication agreements by:
ipa topologysegment-find realm
I see following paths:
1. do not change (current state)
2. list only winsync agreements - IMO it will be easier to maintain
If winsync was not in play we could 'disable' it but winsync is not
planned to be centrally managed. Mainly because the preferred
alternative is trust.
=== connect ===
Allow for winsync, disable for REALM agmts. (current state)
=== disconenct ===
Allow for winsync, disable for REALM agmts. (current state)
=== del ===
(current state)
With domain level 0:
- removes replica and repl. agmts for REALM suffix and winsync
With domain level 1:
- removes replica entry and therefore repl. agmts for all
suffices(REALM, CS)
- ensure last services, e.g. sets renewal master
- does additional cleanup
I'm not aware of any operation which needs directory manager. IMO it
can be moved to API in future release(e.g. 4.4), especially if
ipa-server-install --uninstall is modified to do most of the cleanup.
=== re-initialize ===
Not changed.
Can be disabled (long-term solution)
Same capability is in topologysegment_reinitialize API command. The
only difference is that no API command shows state of the pending
operation. Should we transform presence of 'start' and 'stop' in
nsds5beginreplicarefresh;left|right attribute into an output of
topologysegment_show, e.g.: 'initialization in progress',
'cancellation of re-initialization requested'.
yes, something like this would be possible,
maybe this can be part of the replication monitoring work, allowing to
query the state of specific agreements.
=== force-sync ===
no change yet
Currently done by setting nsDS5ReplicaUpdateSchedule attribute of
repl. agreement.
1. Is it required?
2. Should the functionality be transferred to topologysegment/topology
plugin?
3. Is current approach good?
in fact it is a hack, it uses the fact that a change in the replication
agremeent will trigger a fresh start of the protocol thread. It woul be
more clean to have "sendupdatesnow" attribute or as a value of the
refresh attribute, would require a change in DS
IMO if we want to preserve the possibility then the long-term solution
is to move it to topology plugin.
yes
=== list-ruv, clean-ruv, abort-clean-ruv, list-clean-ruv ===
Commands manages clean-all-ruv operations on REALM suffix.
ipa-csreplica-manage doesn't have these commands #4987. These
operations are meant for removal of dangling ruvs but they can also
remove "correct" RUV which is not desired.
The UX is not the best because if replica still exists it won't tell
the admin what is the correct RUV and which are the dangling one(s)
and therefore admin must get the info in
cn=replica,cn=$SUFFIX,cn=mapping tree,cn=config
We have a ticket to automate it:
https://fedorahosted.org/freeipa/ticket/5411
Is it possible to manage it in topology plugin in centralized manner?
I see $5411 as short-term solution for 4.3 or 4.4. +
{list|clean|abort-clean-list-clean}-ruv sub-commands should be
extended to work with all suffices.
Long term solution not in 4.3 is to move it to topology plugin.
=== dna(next)range-{show|set} commands
No change in 4.3.
Long term solution is to make it centrally manageable. Not sure if by
topo plugin or something else.
== ipa-csreplica-manage ==
This tool manages only CS replication agreements.
=== list ===
Not needed. We have `ipa server-find` and `ipa topologysegment-find
ipaca` commands.
Should be disabled, add to #5405
=== connect and disconnect ===
Replaced by `ipa topologysegment-{add,del}` commands.
disable #5405
=== del ===
The work is done in `ipa-replica-manage del` therefore disable #5405
=== re-initialize ===
Same as in ipa-replica-manage - can be disabled. No ticket yet.
=== force-sync ===
Same as in ipa-replica-manage - decide what to do. No ticket yet.
=== set-renewal-master ===
AFAIK it's only update in cn=masters so could be moved to API then
this could be disabled.
The change is simple enough for changing in 4.3. No ticket yet.
== Conclusion ==
ipa-csreplica-manage could be abandoned in 4.3 which plays well with
topic "simplify management of CA replica
Re: [Freeipa-devel] [PATCH 0011] Replica promotion related changes in integration tests
Hi Martin,
The updated patch is attached
On 10/27/2015 01:58 PM, Martin Basti wrote:
On 27.10.2015 13:56, Oleg Fayans wrote:
On 10/27/2015 01:22 PM, Martin Basti wrote:
On 27.10.2015 12:06, Oleg Fayans wrote:
Hi Martin,
On 10/27/2015 10:50 AM, Martin Basti wrote:
On 27.10.2015 10:22, Martin Basti wrote:
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my
comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to
this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for
which
test will be running, we were talking about using config
files,
but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which
tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would
require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file,
sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" %
host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture
needs
to know the value of domain level before we can do any dynamic
detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part
you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit
one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
Yep, right you are, it works as expected. Now the line looks like:
self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
Respectively you should use this:
self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL
Now that would definitely not work in case of domain_level is 0:
0 or 1 is always 1
Oh right.
I had discussion with Tomas, and we should add there check if it is not
None, in case that kwargs contains {'domain_level': None} (One does not
know with test when the None value appears there)
I do not get this. If we do not specify domain_level in config.yaml,
it will automatically be populated with a MAX_DOMAIN_LEVEL value. That
means domain_level will never ever possibly be None.
I do not know how pytest works inside, if you are 100% sure, that the
case written above cannot happen, I'm fine with it.
Martin
self.domain_level = kwargs.get('domain_level')
if self.domain_level is None:
self.domain_level = MAX_DOMAIN_LEVEL
As we cannot user 'or' expression with integers, so this is the right
way.
because kwargs IMO contains undefined config values as None
freeipa-te
[Freeipa-devel] [PATCHES 377-379] Hardening of ipa-adtrust-install
Hi,
this couple of patches harden the adtrust installer.
Details in the commit messages.
Fixes: https://fedorahosted.org/freeipa/ticket/5134
Tomas
From a310154f1706cc05cd6c556ec7d92ffb77f7b3fa Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Tue, 27 Oct 2015 16:05:03 +0100
Subject: [PATCH] adtrustinstance: Wait for sidgen task completion
As part of hardening of adtrust installer, we should wait until
the sidgen task is completed before continuing, as it can take
considerable amount of time for a larger deployment.
https://fedorahosted.org/freeipa/ticket/5134
---
ipaserver/install/adtrustinstance.py | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index a890f141b5ea5d79511cbd7eb3d24c73cf04f3b5..588e0648e55a989fd8ab3c5262b1146f55bf11a2 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -30,6 +30,7 @@ from ipaserver.install import service
from ipaserver.install import installutils
from ipaserver.install.bindinstance import get_rr, add_rr, del_rr, \
dns_zone_exists
+from ipaserver.install.replication import wait_for_task
from ipalib import errors, api
from ipalib.util import normalize_zone
from ipapython.dn import DN
@@ -463,13 +464,24 @@ class ADTRUSTInstance(service.Service):
def __add_sids(self):
"""
-Add SIDs for existing users and groups
+Add SIDs for existing users and groups. Make sure the task is finished
+before continuing.
"""
try:
+# Start the sidgen task
self._ldap_mod("ipa-sidgen-task-run.ldif", self.sub_dict)
-except:
-pass
+
+# Notify the user about the possible delay
+self.print_msg("This step may take considerable amount of time, please wait..")
+
+# Wait for the task to complete
+task_dn = DN('cn=sidgen,cn=ipa-sidgen-task,cn=tasks,cn=config')
+wait_for_task(self.admin_conn, task_dn)
+
+except Exception:
+root_logger.debug("Exception occured during SID generation: {0}"
+ .format(str(e)))
def __add_s4u2proxy_target(self):
"""
--
2.1.0
From 8663d83d40bb8ae44a1c1ec0106108e924b9 Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Tue, 27 Oct 2015 16:05:35 +0100
Subject: [PATCH] adtrustinstance: Restart samba service at the end of
adtrust-install
Errors related to establishing trust can occur if samba service is not
restarted after ipa-adtrust-install has been run. Restart the service at
the end of the installer to avoid such issues.
https://fedorahosted.org/freeipa/ticket/5134
---
ipaserver/install/adtrustinstance.py | 7 +++
1 file changed, 7 insertions(+)
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 588e0648e55a989fd8ab3c5262b1146f55bf11a2..9e05bdbe5c4b2e77dee3dc0d4de74a252ea6f4c1 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -743,6 +743,12 @@ class ADTRUSTInstance(service.Service):
except:
pass
+def __restart_smb(self):
+try:
+services.knownservices.smb.restart()
+except Exception:
+pass
+
def __enable(self):
self.backup_state("enabled", self.is_enabled())
# We do not let the system start IPA components on its own,
@@ -874,6 +880,7 @@ class ADTRUSTInstance(service.Service):
if self.add_sids:
self.step("adding SIDs to existing users and groups",
self.__add_sids)
+self.step("restarting smbd", self.__restart_smb)
self.start_creation(show_service_name=False)
--
2.1.0
From 1e682ad06f99723ca2b0c0a71432d13263db5dc6 Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Tue, 27 Oct 2015 16:08:10 +0100
Subject: [PATCH] adtrustinstance: Do not use bare except clauses
https://fedorahosted.org/freeipa/ticket/5134
---
ipaserver/install/adtrustinstance.py | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 9e05bdbe5c4b2e77dee3dc0d4de74a252ea6f4c1..c2b446832559bbeab67f57b45bc23df82d56a68e 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -209,13 +209,13 @@ class ADTRUSTInstance(service.Service):
try:
admin_entry = self.admin_conn.get_entry(admin_dn)
-except:
+except errors.NotFound:
self.print_msg("IPA admin object not found")
return
try:
admin_group_entry = self.admin_conn.get_entry(admin_group_dn)
-except:
+except errors.NotFound:
self.print_msg("IPA admin group object not found")
return
@@ -226,7 +226,7 @@ class AD
Re: [Freeipa-devel] [PATCHSET] Replica promotion patches
On 10/22/2015 01:06 PM, Petr Vobornik wrote:
On 10/16/2015 06:41 PM, Endi Sukma Dewata wrote:
On 10/15/2015 9:54 AM, Simo Sorce wrote:
3) ipa-ca-install fails with:
Traceback (most recent call last):
File
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 445, in start_creation
run_step(full_msg, method)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 435, in run_step
method()
File
"/usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py",
line
631, in __spawn_instance
DogtagInstance.spawn_instance(self, cfg_file)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py",
line 185, in spawn_instance
self.handle_setup_error(e)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py",
line 448, in handle_setup_error
raise RuntimeError("%s configuration failed." % self.subsystem)
RuntimeError: CA configuration failed.
I guess I'm hitting the authentication bug in Dogtag. It is supposed to
be fixed in pki-core-10.2.6-10, but is it fixed in pki-core-10.2.7-0.2?
We might need a new 10.2.7 build.
I am not sure which version has it fixed, Endi ?
PKI ticket #1580 was fixed in pki-core-10.2.6-10 for F23 and F24. We
never released a pki-core-10.2.7. I suppose that is a custom build?
Yes it is a custom build[4].
It was advertised that #1414[1] will be in PKI 10.2.7 but it was
laterincluded into 10.2.6-5. I don't know what's a plan for 10.2.7.
Required patch for the discussed issue #1580[2] is included in 10.2.6-10
So I propose to change requires - patch attached, remove 10.2.7 custom
build from mkosek/freeipa-master repo and add new build(for f22) based
on pki-core-10.2.6-10.fc23 from koji[3]
[1] https://fedorahosted.org/pki/ticket/1414
[2] https://fedorahosted.org/pki/ticket/1580
[3] http://koji.fedoraproject.org/koji/buildinfo?buildID=689985
[4]
https://copr.fedoraproject.org/coprs/mkosek/freeipa-master/build/121544/
ACK
--
Martin^3 Babinsky
--
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] DNSZone command with user friendly message
On 27.10.2015 14:46, Martin Basti wrote: On 27.10.2015 13:00, Abhijeet Kasurde wrote: Hi All, This patch fixes bug - https://fedorahosted.org/freeipa/ticket/4811 Thanks, Abhijeet Kasurde [Tue Oct 27 14:44:51.328615 2015] [wsgi:error] [pid 5556] ipa: ERROR: non-public: AttributeError: 'dnszone' object has no attribute 'handle_obj_found' [Tue Oct 27 14:44:51.328654 2015] [wsgi:error] [pid 5556] Traceback (most recent call last): [Tue Oct 27 14:44:51.328659 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, in wsgi_execute [Tue Oct 27 14:44:51.328664 2015] [wsgi:error] [pid 5556] result = self.Command[name](*args, **options) [Tue Oct 27 14:44:51.328669 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in __call__ [Tue Oct 27 14:44:51.328674 2015] [wsgi:error] [pid 5556] ret = self.run(*args, **options) [Tue Oct 27 14:44:51.328678 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 764, in run [Tue Oct 27 14:44:51.328683 2015] [wsgi:error] [pid 5556] return self.execute(*args, **options) [Tue Oct 27 14:44:51.328687 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2935, in execute [Tue Oct 27 14:44:51.328692 2015] [wsgi:error] [pid 5556] result = super(dnszone_enable, self).execute(*keys, **options) [Tue Oct 27 14:44:51.328696 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2262, in execute [Tue Oct 27 14:44:51.328701 2015] [wsgi:error] [pid 5556] self.obj.handle_obj_found(*keys) [Tue Oct 27 14:44:51.328705 2015] [wsgi:error] [pid 5556] AttributeError: 'dnszone' object has no attribute 'handle_obj_found' Thank you, ACK patch works as expected However now 2 tests are failing because error message was changed, please fix tests too. test_xmlrpc/test_dns_plugin.py <- test_xmlrpc/xmlrpc_test.py::test_forward_zones::test_command[0071: dnsforwardzone_disable: Try to disable non-existent forward zone] FAILED test_xmlrpc/test_dns_plugin.py <- test_xmlrpc/xmlrpc_test.py::test_forward_zones::test_command[0075: dnsforwardzone_enable: Try to enable non-existent forward zone] FAILED E AssertionError: assert_deepequal: expected != got. E E expected = u'no such entry' E got = u'non-existent.fwzone.test.: DNS forward zone not found' E path = () Martin -- 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] [draft] Fate of ipa-replica-manage and ipa-csreplica-manage tools
Both tools serve primarily for managing replication agreements and
replicas. ipa-replica-manage also manages winsync agreements and DNA
ranges.
FreeIPA 4.3 will introduce managed topology which affects these tools.
Let's go trough all sub-commands of both tools and decide what is the
fate of them/how they should be replaced. Comments are welcome.
In text, term 'disable' means: print an error message with help what is
the new alternative.
For domain level == 0 all sub-commands should behave the same way as
before. Proposals are for domain level 1 if not stated otherwise.
== ipa-replica-manage ==
=== list ===
Lists all IPA server or replication agreements of a specific IPA server
including winsync agreements.
Server list is replaced by
ipa server-find
Replication agreements by:
ipa topologysegment-find realm
I see following paths:
1. do not change (current state)
2. list only winsync agreements - IMO it will be easier to maintain
If winsync was not in play we could 'disable' it but winsync is not
planned to be centrally managed. Mainly because the preferred
alternative is trust.
=== connect ===
Allow for winsync, disable for REALM agmts. (current state)
=== disconenct ===
Allow for winsync, disable for REALM agmts. (current state)
=== del ===
(current state)
With domain level 0:
- removes replica and repl. agmts for REALM suffix and winsync
With domain level 1:
- removes replica entry and therefore repl. agmts for all
suffices(REALM, CS)
- ensure last services, e.g. sets renewal master
- does additional cleanup
I'm not aware of any operation which needs directory manager. IMO it can
be moved to API in future release(e.g. 4.4), especially if
ipa-server-install --uninstall is modified to do most of the cleanup.
=== re-initialize ===
Not changed.
Can be disabled (long-term solution)
Same capability is in topologysegment_reinitialize API command. The only
difference is that no API command shows state of the pending operation.
Should we transform presence of 'start' and 'stop' in
nsds5beginreplicarefresh;left|right attribute into an output of
topologysegment_show, e.g.: 'initialization in progress', 'cancellation
of re-initialization requested'.
=== force-sync ===
no change yet
Currently done by setting nsDS5ReplicaUpdateSchedule attribute of repl.
agreement.
1. Is it required?
2. Should the functionality be transferred to topologysegment/topology
plugin?
3. Is current approach good?
IMO if we want to preserve the possibility then the long-term solution
is to move it to topology plugin.
=== list-ruv, clean-ruv, abort-clean-ruv, list-clean-ruv ===
Commands manages clean-all-ruv operations on REALM suffix.
ipa-csreplica-manage doesn't have these commands #4987. These operations
are meant for removal of dangling ruvs but they can also remove
"correct" RUV which is not desired.
The UX is not the best because if replica still exists it won't tell the
admin what is the correct RUV and which are the dangling one(s) and
therefore admin must get the info in cn=replica,cn=$SUFFIX,cn=mapping
tree,cn=config
We have a ticket to automate it:
https://fedorahosted.org/freeipa/ticket/5411
Is it possible to manage it in topology plugin in centralized manner?
I see $5411 as short-term solution for 4.3 or 4.4. +
{list|clean|abort-clean-list-clean}-ruv sub-commands should be extended
to work with all suffices.
Long term solution not in 4.3 is to move it to topology plugin.
=== dna(next)range-{show|set} commands
No change in 4.3.
Long term solution is to make it centrally manageable. Not sure if by
topo plugin or something else.
== ipa-csreplica-manage ==
This tool manages only CS replication agreements.
=== list ===
Not needed. We have `ipa server-find` and `ipa topologysegment-find
ipaca` commands.
Should be disabled, add to #5405
=== connect and disconnect ===
Replaced by `ipa topologysegment-{add,del}` commands.
disable #5405
=== del ===
The work is done in `ipa-replica-manage del` therefore disable #5405
=== re-initialize ===
Same as in ipa-replica-manage - can be disabled. No ticket yet.
=== force-sync ===
Same as in ipa-replica-manage - decide what to do. No ticket yet.
=== set-renewal-master ===
AFAIK it's only update in cn=masters so could be moved to API then this
could be disabled.
The change is simple enough for changing in 4.3. No ticket yet.
== Conclusion ==
ipa-csreplica-manage could be abandoned in 4.3 which plays well with
topic "simplify management of CA replication agreements".
ipa-replica-manage is still needed for RUV handling and removal of
replicas in 4.3. This can change in a future. Same situation with DNA
ranges handling.
There is no future plan for winsync agreements and ipa-replica-manage
can remain solely for this purpose in environments with managed topology.
--
Petr Vobornik
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute t
Re: [Freeipa-devel] [PATCH 0331, 0337] User plugin: allow multiple managers per user - CLI part
On 20.10.2015 18:46, Martin Basti wrote:
On 20.10.2015 16:07, Martin Basti wrote:
On 20.10.2015 15:57, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/5344
Patch attached.
Test are failing, a fix in UserTracker has to be done (partially in
my patch 329)
SelfNACK, I forgot to add stageuser tests
Updated patch attached.
I extracted tests to the separate patch, tests do not work, I had
issues with user and stageuser trackers.
Patch to fix issues with --addattr and managers added and attached.
From 7e301a11f7ea46cff25cb0d6fa13058c69ae530c Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Tue, 27 Oct 2015 13:42:49 +0100
Subject: [PATCH] Fix --add-attr with multiple managers
Framework expects managers as unicode, but if there was a manager in
LDAP specified, it was returned as DN, which caused parameter conversion
error.
Normalize method was added to manager parameter which convert DN to
manager login.
https://fedorahosted.org/freeipa/ticket/5344
---
ipalib/plugins/baseuser.py | 18 ++
1 file changed, 18 insertions(+)
diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index da4883ccec906472ed2e82f5c61ef91c9b2049e9..4d6bf1dfca7c94aba31f1f8d0125f1065271613f 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -153,6 +153,23 @@ def normalize_principal(principal):
return unicode('%s@%s' % (user, realm))
+def _convert_manager(manager):
+# convert DN to unicode, otherwise --addattr call will not work
+# validation of manager is done later, just extract manager login from DN
+if not manager:
+return manager
+
+if isinstance(manager, DN):
+try:
+return manager['uid']
+except KeyError:
+raise errors.ConversionError(
+_("DN of the manager does not contain 'uid'")
+)
+
+
+return manager
+
def fix_addressbook_permission_bindrule(name, template, is_new,
anonymous_read_aci,
@@ -340,6 +357,7 @@ class baseuser(LDAPObject):
),
Str('manager*',
label=_('Manager'),
+normalizer=_convert_manager,
),
Str('carlicense*',
label=_('Car License'),
--
2.4.3
From 250c5d3f2f5e47b19c628115ecd9df8a71d357dc Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Tue, 20 Oct 2015 18:39:57 +0200
Subject: [PATCH] Allow multiple managers per user - CLI part
https://fedorahosted.org/freeipa/ticket/5344
---
API.txt| 12 ++--
VERSION| 4 ++--
ipalib/plugins/baseuser.py | 7 +--
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/API.txt b/API.txt
index 873c6d54221a0c1657b5457bd9dceedb4adf06b3..896df430aaa1952c0fe4af4672b78f1ad11da45e 100644
--- a/API.txt
+++ b/API.txt
@@ -4225,7 +4225,7 @@ option: Str('krbprincipalname', attribute=True, autofill=True, cli_name='princip
option: Str('l', attribute=True, cli_name='city', multivalue=False, required=False)
option: Str('loginshell', attribute=True, cli_name='shell', multivalue=False, required=False)
option: Str('mail', attribute=True, cli_name='email', multivalue=True, required=False)
-option: Str('manager', attribute=True, cli_name='manager', multivalue=False, required=False)
+option: Str('manager', attribute=True, cli_name='manager', multivalue=True, required=False)
option: Str('mobile', attribute=True, cli_name='mobile', multivalue=True, required=False)
option: Flag('no_members', autofill=True, default=False, exclude='webui')
option: Str('ou', attribute=True, cli_name='orgunit', multivalue=False, required=False)
@@ -4285,7 +4285,7 @@ option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='princi
option: Str('l', attribute=True, autofill=False, cli_name='city', multivalue=False, query=True, required=False)
option: Str('loginshell', attribute=True, autofill=False, cli_name='shell', multivalue=False, query=True, required=False)
option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue=True, query=True, required=False)
-option: Str('manager', attribute=True, autofill=False, cli_name='manager', multivalue=False, query=True, required=False)
+option: Str('manager', attribute=True, autofill=False, cli_name='manager', multivalue=True, query=True, required=False)
option: Str('mobile', attribute=True, autofill=False, cli_name='mobile', multivalue=True, query=True, required=False)
option: Flag('no_members', autofill=True, default=False, exclude='webui')
option: Str('not_in_group*', cli_name='not_in_groups', csv=True)
@@ -4342,7 +4342,7 @@ option: DateTime('krbprincipalexpiration', attribute=True, autofill=False, cli_n
option: Str('l', attribute=True, autofill=False, cli_name='city', multivalue=False, required=False)
option: Str('loginshell', attribute=True, autofill=False, cli_name='shell', multivalue=False, required=False)
option: Str('mail', attribute=True, autofill=False, cli
Re: [Freeipa-devel] [PATCH] DNSZone command with user friendly message
On 10/27/2015 07:16 PM, Martin Basti wrote: On 27.10.2015 13:00, Abhijeet Kasurde wrote: Hi All, This patch fixes bug - https://fedorahosted.org/freeipa/ticket/4811 Thanks, Abhijeet Kasurde Sorry, my bad. Please find the new patch. [Tue Oct 27 14:44:51.328615 2015] [wsgi:error] [pid 5556] ipa: ERROR: non-public: AttributeError: 'dnszone' object has no attribute 'handle_obj_found' [Tue Oct 27 14:44:51.328654 2015] [wsgi:error] [pid 5556] Traceback (most recent call last): [Tue Oct 27 14:44:51.328659 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, in wsgi_execute [Tue Oct 27 14:44:51.328664 2015] [wsgi:error] [pid 5556] result = self.Command[name](*args, **options) [Tue Oct 27 14:44:51.328669 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in __call__ [Tue Oct 27 14:44:51.328674 2015] [wsgi:error] [pid 5556] ret = self.run(*args, **options) [Tue Oct 27 14:44:51.328678 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 764, in run [Tue Oct 27 14:44:51.328683 2015] [wsgi:error] [pid 5556] return self.execute(*args, **options) [Tue Oct 27 14:44:51.328687 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2935, in execute [Tue Oct 27 14:44:51.328692 2015] [wsgi:error] [pid 5556] result = super(dnszone_enable, self).execute(*keys, **options) [Tue Oct 27 14:44:51.328696 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2262, in execute [Tue Oct 27 14:44:51.328701 2015] [wsgi:error] [pid 5556] self.obj.handle_obj_found(*keys) [Tue Oct 27 14:44:51.328705 2015] [wsgi:error] [pid 5556] AttributeError: 'dnszone' object has no attribute 'handle_obj_found' From 44dad1e1dc4113970068a766ccca9c66edaa Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Tue, 27 Oct 2015 17:21:17 +0530 Subject: [PATCH] Added user friendly error message for dnszone enable and disable Added try-except block in dns plugin in order to provide user friendly message to end user. https://fedorahosted.org/freeipa/ticket/4811 Signed-off-by: Abhijeet Kasurde --- ipalib/plugins/dns.py | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index ef282c94609df0be0aa02ec14eb2b137e7ad9dbd..48d6f740ebea06e0ae9e8d68deafd607b5ae18d8 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -2231,7 +2231,11 @@ class DNSZoneBase_disable(LDAPQuery): ldap = self.obj.backend dn = self.obj.get_dn(*keys, **options) -entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +try: +entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +except errors.NotFound: +self.obj.handle_not_found(*keys) + if not _check_entry_objectclass(entry, self.obj.object_class): self.obj.handle_not_found(*keys) @@ -2252,7 +2256,11 @@ class DNSZoneBase_enable(LDAPQuery): ldap = self.obj.backend dn = self.obj.get_dn(*keys, **options) -entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +try: +entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +except errors.NotFound: +self.obj.handle_not_found(*keys) + if not _check_entry_objectclass(entry, self.obj.object_class): self.obj.handle_not_found(*keys) -- 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
Re: [Freeipa-devel] [PATCH] DNSZone command with user friendly message
On 27.10.2015 13:00, Abhijeet Kasurde wrote: Hi All, This patch fixes bug - https://fedorahosted.org/freeipa/ticket/4811 Thanks, Abhijeet Kasurde [Tue Oct 27 14:44:51.328615 2015] [wsgi:error] [pid 5556] ipa: ERROR: non-public: AttributeError: 'dnszone' object has no attribute 'handle_obj_found' [Tue Oct 27 14:44:51.328654 2015] [wsgi:error] [pid 5556] Traceback (most recent call last): [Tue Oct 27 14:44:51.328659 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, in wsgi_execute [Tue Oct 27 14:44:51.328664 2015] [wsgi:error] [pid 5556] result = self.Command[name](*args, **options) [Tue Oct 27 14:44:51.328669 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in __call__ [Tue Oct 27 14:44:51.328674 2015] [wsgi:error] [pid 5556] ret = self.run(*args, **options) [Tue Oct 27 14:44:51.328678 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 764, in run [Tue Oct 27 14:44:51.328683 2015] [wsgi:error] [pid 5556] return self.execute(*args, **options) [Tue Oct 27 14:44:51.328687 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2935, in execute [Tue Oct 27 14:44:51.328692 2015] [wsgi:error] [pid 5556] result = super(dnszone_enable, self).execute(*keys, **options) [Tue Oct 27 14:44:51.328696 2015] [wsgi:error] [pid 5556] File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2262, in execute [Tue Oct 27 14:44:51.328701 2015] [wsgi:error] [pid 5556] self.obj.handle_obj_found(*keys) [Tue Oct 27 14:44:51.328705 2015] [wsgi:error] [pid 5556] AttributeError: 'dnszone' object has no attribute 'handle_obj_found' -- 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 0011] Replica promotion related changes in integration tests
On 27.10.2015 13:56, Oleg Fayans wrote:
On 10/27/2015 01:22 PM, Martin Basti wrote:
On 27.10.2015 12:06, Oleg Fayans wrote:
Hi Martin,
On 10/27/2015 10:50 AM, Martin Basti wrote:
On 27.10.2015 10:22, Martin Basti wrote:
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my
comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to
this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for
which
test will be running, we were talking about using config
files,
but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which
tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would
require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file,
sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" %
host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture
needs
to know the value of domain level before we can do any dynamic
detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part
you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit
one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
Yep, right you are, it works as expected. Now the line looks like:
self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
Respectively you should use this:
self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL
Now that would definitely not work in case of domain_level is 0:
0 or 1 is always 1
Oh right.
I had discussion with Tomas, and we should add there check if it is not
None, in case that kwargs contains {'domain_level': None} (One does not
know with test when the None value appears there)
I do not get this. If we do not specify domain_level in config.yaml,
it will automatically be populated with a MAX_DOMAIN_LEVEL value. That
means domain_level will never ever possibly be None.
I do not know how pytest works inside, if you are 100% sure, that the
case written above cannot happen, I'm fine with it.
Martin
self.domain_level = kwargs.get('domain_level')
if self.domain_level is None:
self.domain_level = MAX_DOMAIN_LEVEL
As we cannot user 'or' expression with integers, so this is the right
way.
because kwargs IMO contains undefined config values as None
freeipa-tests depends on freeipa-python so the constants should be
available in t
Re: [Freeipa-devel] [PATCH 0011] Replica promotion related changes in integration tests
On 10/27/2015 01:22 PM, Martin Basti wrote:
On 27.10.2015 12:06, Oleg Fayans wrote:
Hi Martin,
On 10/27/2015 10:50 AM, Martin Basti wrote:
On 27.10.2015 10:22, Martin Basti wrote:
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for
which
test will be running, we were talking about using config files,
but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would
require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file,
sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" % host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture
needs
to know the value of domain level before we can do any dynamic
detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
Yep, right you are, it works as expected. Now the line looks like:
self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
Respectively you should use this:
self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL
Now that would definitely not work in case of domain_level is 0:
0 or 1 is always 1
Oh right.
I had discussion with Tomas, and we should add there check if it is not
None, in case that kwargs contains {'domain_level': None} (One does not
know with test when the None value appears there)
I do not get this. If we do not specify domain_level in config.yaml, it
will automatically be populated with a MAX_DOMAIN_LEVEL value. That
means domain_level will never ever possibly be None.
self.domain_level = kwargs.get('domain_level')
if self.domain_level is None:
self.domain_level = MAX_DOMAIN_LEVEL
As we cannot user 'or' expression with integers, so this is the right way.
because kwargs IMO contains undefined config values as None
freeipa-tests depends on freeipa-python so the constants should be
available in tests.
So then you also need update this line
+if not host.config.domain_level != MAX_DOMAIN_LEVEL:
+args.append("--domain-level=%i" % host.config.domain_level)
This would n
Re: [Freeipa-devel] [PATCH 0011] Replica promotion related changes in integration tests
On 27.10.2015 12:06, Oleg Fayans wrote:
Hi Martin,
On 10/27/2015 10:50 AM, Martin Basti wrote:
On 27.10.2015 10:22, Martin Basti wrote:
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for
which
test will be running, we were talking about using config files,
but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would
require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file,
sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" % host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture
needs
to know the value of domain level before we can do any dynamic
detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
Yep, right you are, it works as expected. Now the line looks like:
self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
Respectively you should use this:
self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL
Now that would definitely not work in case of domain_level is 0:
0 or 1 is always 1
Oh right.
I had discussion with Tomas, and we should add there check if it is not
None, in case that kwargs contains {'domain_level': None} (One does not
know with test when the None value appears there)
self.domain_level = kwargs.get('domain_level')
if self.domain_level is None:
self.domain_level = MAX_DOMAIN_LEVEL
As we cannot user 'or' expression with integers, so this is the right way.
because kwargs IMO contains undefined config values as None
freeipa-tests depends on freeipa-python so the constants should be
available in tests.
So then you also need update this line
+if not host.config.domain_level != MAX_DOMAIN_LEVEL:
+args.append("--domain-level=%i" % host.config.domain_level)
This would not work if domainlevel is not set in config.yaml, in
which case the host.config.domain_level is None.
Domain level will never be None because self.domain_level =
kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
4)
Please add comment to f
[Freeipa-devel] [PATCH] DNSZone command with user friendly message
Hi All, This patch fixes bug - https://fedorahosted.org/freeipa/ticket/4811 Thanks, Abhijeet Kasurde From a333e7ddb830e5ff10d2602b12669b767b870dbb Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Tue, 27 Oct 2015 17:21:17 +0530 Subject: [PATCH] Added user friendly error message for dnszone enable and disable Added try-except block in dns plugin in order to provide user friendly message to end user. https://fedorahosted.org/freeipa/ticket/4811 Signed-off-by: Abhijeet Kasurde --- ipalib/plugins/dns.py | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index ef282c94609df0be0aa02ec14eb2b137e7ad9dbd..c90f3209b9f6226e89b2d2e832039187c8f21428 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -2231,7 +2231,11 @@ class DNSZoneBase_disable(LDAPQuery): ldap = self.obj.backend dn = self.obj.get_dn(*keys, **options) -entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +try: +entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +except errors.NotFound: +self.obj.handle_not_found(*keys) + if not _check_entry_objectclass(entry, self.obj.object_class): self.obj.handle_not_found(*keys) @@ -2252,7 +2256,11 @@ class DNSZoneBase_enable(LDAPQuery): ldap = self.obj.backend dn = self.obj.get_dn(*keys, **options) -entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +try: +entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass']) +except errors.NotFound: +self.obj.handle_obj_found(*keys) + if not _check_entry_objectclass(entry, self.obj.object_class): self.obj.handle_not_found(*keys) -- 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
Re: [Freeipa-devel] [PATCH 0011] Replica promotion related changes in integration tests
Hi Martin,
On 10/27/2015 10:50 AM, Martin Basti wrote:
On 27.10.2015 10:22, Martin Basti wrote:
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for which
test will be running, we were talking about using config files,
but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file, sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" % host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture
needs
to know the value of domain level before we can do any dynamic
detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
Yep, right you are, it works as expected. Now the line looks like:
self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
Respectively you should use this:
self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL
Now that would definitely not work in case of domain_level is 0:
0 or 1 is always 1
because kwargs IMO contains undefined config values as None
freeipa-tests depends on freeipa-python so the constants should be
available in tests.
So then you also need update this line
+if not host.config.domain_level != MAX_DOMAIN_LEVEL:
+args.append("--domain-level=%i" % host.config.domain_level)
This would not work if domainlevel is not set in config.yaml, in
which case the host.config.domain_level is None.
Domain level will never be None because self.domain_level =
kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
4)
Please add comment to function +def domainlevel(host): that it is
useful
for test where domain level will be raised dynamically, otherwise it
may
be lost after test refactoring as somebody may consider it as unneeded
and replace it with config dict.
Done
So summary is the 1) and 2) are replaced by 3) :)
Martin^2
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 7ebc7c0899a2437463aaf7fdc03ea93fbab057bb Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: T
Re: [Freeipa-devel] [PATCH 0011] Replica promotion related changes in integration tests
On 27.10.2015 10:22, Martin Basti wrote:
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for which
test will be running, we were talking about using config files,
but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file, sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" % host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture
needs
to know the value of domain level before we can do any dynamic
detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
Respectively you should use this:
self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL
because kwargs IMO contains undefined config values as None
freeipa-tests depends on freeipa-python so the constants should be
available in tests.
So then you also need update this line
+if not host.config.domain_level != MAX_DOMAIN_LEVEL:
+args.append("--domain-level=%i" % host.config.domain_level)
This would not work if domainlevel is not set in config.yaml, in
which case the host.config.domain_level is None.
Domain level will never be None because self.domain_level =
kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
4)
Please add comment to function +def domainlevel(host): that it is
useful
for test where domain level will be raised dynamically, otherwise it
may
be lost after test refactoring as somebody may consider it as unneeded
and replace it with config dict.
Done
So summary is the 1) and 2) are replaced by 3) :)
Martin^2
--
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 0327] KRA: fix check if CA is installed on replica
On 26.10.2015 10:35, Martin Babinsky wrote:
On 10/23/2015 05:18 PM, Martin Basti wrote:
On 23.10.2015 15:15, Martin Babinsky wrote:
On 10/23/2015 03:12 PM, Martin Babinsky wrote:
On 10/16/2015 12:41 PM, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/5345
Patch attached.
I have tested it on 4-2 branch and it works as expected, ACK.
Obviously, master branch would require a different patch.
I actually missed your check in ipaserver/install/kra.py which can
break ipa-replica-install with --setup-kra, so NACK.
Updated patches attached.
NACK on the master-branch patch.
You forgot a 'return' in this code snippet:
+if self.installing_replica:
+domain_level = dsinstance.get_domain_level(api)
+if domain_level > DOMAIN_LEVEL_0:
+self.options.promote = True
+return
that would make installation abort when domain level is greter than zero.
Updated patch attached.
From 1829177623561a239c20eb029ed84d1576a59d01 Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Fri, 23 Oct 2015 14:15:00 +0200
Subject: [PATCH] KRA: fix check that CA is installed
https://fedorahosted.org/freeipa/ticket/5345
---
ipaserver/install/ipa_kra_install.py | 42
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/ipaserver/install/ipa_kra_install.py b/ipaserver/install/ipa_kra_install.py
index 1ae361edc3df3c572a5a8d6900ba5425300443c1..add8250d4dabfa0cb88ea7c1c291955691b3f22b 100755
--- a/ipaserver/install/ipa_kra_install.py
+++ b/ipaserver/install/ipa_kra_install.py
@@ -32,6 +32,7 @@ from ipapython import dogtag
from ipapython import ipautil
from ipapython.dn import DN
from ipaserver.install import service
+from ipaserver.install import cainstance
from ipaserver.install import krainstance
from ipaserver.install import dsinstance
from ipaserver.install import installutils
@@ -134,28 +135,13 @@ class KRAInstaller(KRAInstall):
" in unattended mode"
)
-self.installing_replica = dogtaginstance.is_installing_replica("KRA")
-self.options.promote = False
-
-if self.installing_replica:
-domain_level = dsinstance.get_domain_level(api)
-if domain_level > DOMAIN_LEVEL_0:
-self.options.promote = True
-return
-
-if not self.args:
-self.option_parser.error("A replica file is required.")
-if len(self.args) > 1:
-self.option_parser.error("Too many arguments provided")
-
+if len(self.args) > 1:
+self.option_parser.error("Too many arguments provided")
+elif len(self.args) == 1:
self.replica_file = self.args[0]
if not ipautil.file_exists(self.replica_file):
self.option_parser.error(
"Replica file %s does not exist" % self.replica_file)
-else:
-if self.args:
-self.option_parser.error("Too many parameters provided. "
- "No replica file is required.")
def ask_for_options(self):
super(KRAInstaller, self).ask_for_options()
@@ -170,6 +156,26 @@ class KRAInstaller(KRAInstall):
def _run(self):
super(KRAInstaller, self).run()
+
+if not cainstance.is_ca_installed_locally():
+raise RuntimeError("Dogtag CA is not installed. "
+ "Please install the CA first")
+
+# this check can be done only when CA is installed
+self.installing_replica = dogtaginstance.is_installing_replica("KRA")
+self.options.promote = False
+
+if self.installing_replica:
+domain_level = dsinstance.get_domain_level(api)
+if domain_level > DOMAIN_LEVEL_0:
+self.options.promote = True
+elif not self.args:
+raise RuntimeError("A replica file is required.")
+else:
+if self.args:
+raise RuntimeError("Too many parameters provided. "
+ "No replica file is required.")
+
print(dedent(self.INSTALLER_START_MESSAGE))
self.options.dm_password = self.options.password
--
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
Re: [Freeipa-devel] [PATCH 0335] Freeipa domain levels naming
On 27.10.2015 10:12, Petr Spacek wrote:
On 23.10.2015 14:07, Petr Spacek wrote:
On 23.10.2015 13:53, Martin Basti wrote:
On 23.10.2015 13:53, Tomas Babej wrote:
On 10/23/2015 01:51 PM, Martin Basti wrote:
On 23.10.2015 13:49, Tomas Babej wrote:
On 10/23/2015 12:49 PM, Martin Basti wrote:
On 23.10.2015 09:34, Martin Basti wrote:
On 23.10.2015 09:31, Tomas Babej wrote:
On 10/22/2015 05:49 PM, Simo Sorce wrote:
On 22/10/15 11:29, Martin Basti wrote:
Hello all,
in current master branch we have mixed usage of literals 0, 1 and
constants MIN_DOMAIN_LEVEL, MAX_DOMAIN_LEVEL, and it is quite mess.
I suggest to use names for domain levels:
COMPAT_DOMAIN_LEVEL = 0
PROMOTION_DOMAIN_LEVEL = 1
UBER_NEW_FEATURE_DOMAIN_LEVEL = 2
MIN_DOMAIN_LEVEL = COMPAT_DOMAIN_LEVEL (=0)
MAX_DOMAIN_LEVEL = UBER_NEW_FEATURE_DOMAIN_LEVEL (=2)
Benefits:
* ability to grep it in code
Call them DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1
* better readability in code
Sure, but random names are not appropriate imo
I'm with you guys on this, it's a good idea. Let's go with the
DOMAIN_LEVEL_X naming though, it will be probably easier to remember.
One thing to add to the discussion - MIN/MAX_DOMAIN_LEVEL denotes only
the minimal/maximal domain level supported by the given IPA server,
not
the minimal/maximal domain level ever shipped by FreeIPA project.
Currently, those two coincide, but in general they might be
different if
we ever raise the minimal level a decide to deprecate, say, domain
level
0 or 1. It's a subtle but important difference.
Tomas
Thank you all for your opinion,
I will implement DOMAIN_LEVEL_X constants and send patch.
Thanks.
Martin^2
Patch attached.
O hope, I did not miss anything hardcoded.
I think we can safely hardcode domain levels as numbers in the error
messages. Substituting {dl} for constants.DOMAIN_LEVEL_0 in the error
message makes little sense to me, as the value of
constants.DOMAIN_LEVEL_0 will not ever change.
However, with substituting is easier to find occurrences of domain
levels in comments and messages in case of refactoring.
In case of refactoring of what? All the error messages containing
reference to a domain level 0? :)
Exactly! :)
Tomas, that one day we decide to drop support for domain level 0. We will have
to hunt down all references to it and using constant for help messages etc.
might be handy because it will show up in results of grep, so we do not forget
to wipe domain level 0 from texts.
Nitpick just to make you feel that I read the patch (does not warrant a NACK):
I would rather see conditions in form of (<= or >=) instead of (< or >)
because now you have to grep for domain level +- 1 to get all references to
particular domain level :-D
Nevermind, this is just a nitpick.
ACK, I did not find any breakage.
Pushed to master: beb6a3236d5c10acd990aaf92eddc74fee456909
--
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 0011] Replica promotion related changes in integration tests
On 27.10.2015 10:00, Oleg Fayans wrote:
Hi Martin,
The updated version of the patch is attached. Please, see my comments
below
My comments inline, I may be completely wrong in how the test suite
work, so feel free to correct me.
Martin
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the
default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for which
test will be running, we were talking about using config files, but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain
level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file, sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" % host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture needs
to know the value of domain level before we can do any dynamic detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit one.
wat? in case that kwargs is dict containing values from config file:
In [1]: kwargs = dict(domain_level=0)
In [2]: kwargs.get('domain_level', 123)
Out[2]: 0
freeipa-tests depends on freeipa-python so the constants should be
available in tests.
So then you also need update this line
+if not host.config.domain_level != MAX_DOMAIN_LEVEL:
+args.append("--domain-level=%i" % host.config.domain_level)
This would not work if domainlevel is not set in config.yaml, in which
case the host.config.domain_level is None.
Domain level will never be None because self.domain_level =
kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
4)
Please add comment to function +def domainlevel(host): that it is useful
for test where domain level will be raised dynamically, otherwise it may
be lost after test refactoring as somebody may consider it as unneeded
and replace it with config dict.
Done
So summary is the 1) and 2) are replaced by 3) :)
Martin^2
--
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 0335] Freeipa domain levels naming
On 23.10.2015 14:07, Petr Spacek wrote:
> On 23.10.2015 13:53, Martin Basti wrote:
>>
>>
>> On 23.10.2015 13:53, Tomas Babej wrote:
>>>
>>> On 10/23/2015 01:51 PM, Martin Basti wrote:
On 23.10.2015 13:49, Tomas Babej wrote:
> On 10/23/2015 12:49 PM, Martin Basti wrote:
>> On 23.10.2015 09:34, Martin Basti wrote:
>>> On 23.10.2015 09:31, Tomas Babej wrote:
On 10/22/2015 05:49 PM, Simo Sorce wrote:
> On 22/10/15 11:29, Martin Basti wrote:
>> Hello all,
>>
>> in current master branch we have mixed usage of literals 0, 1 and
>> constants MIN_DOMAIN_LEVEL, MAX_DOMAIN_LEVEL, and it is quite mess.
>>
>> I suggest to use names for domain levels:
>>
>> COMPAT_DOMAIN_LEVEL = 0
>> PROMOTION_DOMAIN_LEVEL = 1
>> UBER_NEW_FEATURE_DOMAIN_LEVEL = 2
>>
>> MIN_DOMAIN_LEVEL = COMPAT_DOMAIN_LEVEL (=0)
>> MAX_DOMAIN_LEVEL = UBER_NEW_FEATURE_DOMAIN_LEVEL (=2)
>>
>> Benefits:
>> * ability to grep it in code
> Call them DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1
>
>> * better readability in code
> Sure, but random names are not appropriate imo
I'm with you guys on this, it's a good idea. Let's go with the
DOMAIN_LEVEL_X naming though, it will be probably easier to remember.
One thing to add to the discussion - MIN/MAX_DOMAIN_LEVEL denotes only
the minimal/maximal domain level supported by the given IPA server,
not
the minimal/maximal domain level ever shipped by FreeIPA project.
Currently, those two coincide, but in general they might be
different if
we ever raise the minimal level a decide to deprecate, say, domain
level
0 or 1. It's a subtle but important difference.
Tomas
>>> Thank you all for your opinion,
>>>
>>> I will implement DOMAIN_LEVEL_X constants and send patch.
>>>
>>> Thanks.
>>> Martin^2
>>>
>> Patch attached.
>> O hope, I did not miss anything hardcoded.
> I think we can safely hardcode domain levels as numbers in the error
> messages. Substituting {dl} for constants.DOMAIN_LEVEL_0 in the error
> message makes little sense to me, as the value of
> constants.DOMAIN_LEVEL_0 will not ever change.
However, with substituting is easier to find occurrences of domain
levels in comments and messages in case of refactoring.
>>> In case of refactoring of what? All the error messages containing
>>> reference to a domain level 0? :)
>> Exactly! :)
>
> Tomas, that one day we decide to drop support for domain level 0. We will have
> to hunt down all references to it and using constant for help messages etc.
> might be handy because it will show up in results of grep, so we do not forget
> to wipe domain level 0 from texts.
>
>
> Nitpick just to make you feel that I read the patch (does not warrant a NACK):
> I would rather see conditions in form of (<= or >=) instead of (< or >)
> because now you have to grep for domain level +- 1 to get all references to
> particular domain level :-D
>
> Nevermind, this is just a nitpick.
ACK, I did not find any breakage.
--
Petr^2 Spacek
--
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 0011] Replica promotion related changes in integration tests
Hi Martin,
The updated version of the patch is attached. Please, see my comments below
On 10/26/2015 06:48 PM, Martin Basti wrote:
On 26.10.2015 08:59, Oleg Fayans wrote:
On 10/23/2015 03:10 PM, Martin Basti wrote:
On 23.10.2015 15:00, Oleg Fayans wrote:
Hi Martin,
Here comes the updated version.
On 10/22/2015 05:38 PM, Martin Basti wrote:
On 22.10.2015 15:23, Martin Basti wrote:
On 22.10.2015 14:13, Oleg Fayans wrote:
Hello,
thank you for the patch.
1)
please remove the added empty lines, they are unrelated to this
ticket
done
2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):
I suggest to use default domainlevel=None, which will use the default
domain level (specified in build)
done
3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.
We have to test, both domain level 0 and 1 for IPA 4.3, respectively
new IPA must support all older domain levels, domain level is
independent on IPA version, only admin can raise it up.
So you have to find out way how to pass the domain level for which
test will be running, we were talking about using config files, but
feel free to find something new and better
Fixed. Now, we declare domain level in config.yaml with the directive
domain_level
4)
Did you resolve the pytest fixtures which specifies which tests
can be
run under which domain level?
In fact, we do not seem to have any tests yet that would require it.
All the existing tests just use install_replica
method, no matter how is it done.
How about topology CI test? This can be executed only with domain level
That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file, sorry
about that).
1, right?
5)
+'--ip-address', client.ip,
why this change to client install?
Right, it found to be unnecessary.
Martin^2
6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85: [E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in function
call)
Fixed
1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" % host.config.domain_level)
always use: variable *is not None*
2)
Why there is specified level 1 as default? IIRC we agreed that the
default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"
+"DOMAINLVL": "1",
3)
However, as I read the patch 12, and I'm right, the pytest.fixture needs
to know the value of domain level before we can do any dynamic detection
on master.
So we should use the constants MAX_DOMAIN_LEVEL as default, for 2)
Done
Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)
This won't work in cases when domainlevel is explicitly set to 0 in
config.yaml. This default value will always override the explicit one.
freeipa-tests depends on freeipa-python so the constants should be
available in tests.
So then you also need update this line
+if not host.config.domain_level != MAX_DOMAIN_LEVEL:
+args.append("--domain-level=%i" % host.config.domain_level)
This would not work if domainlevel is not set in config.yaml, in which
case the host.config.domain_level is None.
4)
Please add comment to function +def domainlevel(host): that it is useful
for test where domain level will be raised dynamically, otherwise it may
be lost after test refactoring as somebody may consider it as unneeded
and replace it with config dict.
Done
So summary is the 1) and 2) are replaced by 3) :)
Martin^2
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 385ad3ca9c564e3d08a0a3256dfc65ab07374a04 Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Tue, 27 Oct 2015 09:43:33 +0100
Subject: [PATCH] Updated the tests according to the new replica installation
workflow
As of 4.3 the replica installation is performed without preparing a gpg file on
master, but rather enrolling a future replica as a client with subsequent
promotion of the client. This required the corresponding change in the
integration tests
https://fedorahosted.org/freeipa/ticket/5379
---
ipatests/test_integration/config.py | 3 +-
ipatests/test_integration/tasks.py | 41
ipatests/test_integration/test_testconfig.py | 5 +++-
3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/ipatests/test_integration/config.py b/ipatests/test_integration/config.py
index 785a9bb8c420f99980c098887e0bd31422119564..60a4bd700afe3027dfcbdf203d02373f8a7aa9f9 100644
--- a/ipatests/test_integration/config.py
+++ b/ipatests/test_integration/config.
Re: [Freeipa-devel] [PATCH 0012-0019] CA ACL tracker and functional test
On 23.10.2015 14:01, Milan Kubík wrote:
On 10/20/2015 02:19 PM, Martin Basti wrote:
NACK
1)
I still see many hardcoded passwords in the code
with change_principal(smime_user, "Secret123"):
For now changed to module variable.
2)
Also the 'alice' username can be extracted to module variable
instead hardcoding
The fixture should take the place of module variables in the tests.
Changed u'alice' into local variable.
Once we fix the problems with UserTracker, we should store the
password here as well.
3)
File alice.conf.tmpl can be generalized to be used for more users,
replace alice in template to {username} and in code replace this
variable with alice, also do not forgot rename template to something
more general
Done.
Updated patch set attached.
ACK
Pushed to master: 5ab0fcabf3e6ac7970c1803893717301a4b4cfe8
Pushed to ipa-4-2: 21fed035beab7dbee59f1e0c29d203345f0d0c7f
--
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
