Dne 26.5.2015 v 13:54 Tomas Babej napsal(a):
On 05/26/2015 01:51 PM, Tomas Babej wrote:
On 05/26/2015 12:39 PM, Tomas Babej wrote:
On 05/26/2015 11:57 AM, Jan Cholasta wrote:
Dne 25.5.2015 v 17:15 Tomas Babej napsal(a):
On 05/25/2015 12:42 PM, Tomas Babej wrote:
On 05/25/2015 07:30 AM, Jan Cholasta wrote:
Dne 22.5.2015 v 12:36 Petr Vobornik napsal(a):
On 05/22/2015 07:08 AM, Jan Cholasta wrote:
Dne 21.5.2015 v 18:18 Tomas Babej napsal(a):
On 05/19/2015 04:07 PM, Tomas Babej wrote:
On 05/19/2015 03:59 PM, Martin Kosek wrote:
On 05/19/2015 03:56 PM, Tomas Babej wrote:
On 05/19/2015 03:51 PM, Martin Kosek wrote:
On 05/19/2015 03:49 PM, Ludwig Krispenz wrote:
On 05/19/2015 03:36 PM, Martin Kosek wrote:
On 05/19/2015 03:22 PM, Tomas Babej wrote:
...
3) Domain level is just a single integer and it should be
treated as such,
there's no need for an LDAPObject plugin and other
unnecessary
complexities.
The implemetation could be as simple as (from top of my
head,
untested):
That's right, I also considered this approach, but as far
as I
know you do
not
get the permission handling for the global DomainLevel entry
otherwise.
Ludwig, I changed the path for the global entry to
cn=DomainLevel.
I know this particular DN was added to the design by Simo, but
why do we want
to use CamelCase with LDAP object?
Wouldn't "cn=Domain Level,cn=ipa,cn=etc,SUFFIX" be a better
place
for it? This
is the last time we can change it, so I am asking now.
Then, we
will be stuck
with this DN forever.
I don't mind using ""cn=Domain Level" ,
but where does the entry live, here you say
cn=Domain Level,cn=ipa,cn=etc,SUFFIX"
and in the design page it is:
cn=DomainLevel,cn=etc,SUFFIX
The current version of the topology plugin is looking for
cn=DomainLevel,cn=ipa,cn=etc,SUFFIX"
but I want to change it to do a search on
objectclass=ipaDomainLevelConfig
I see - we all need to unify the location apparently. I
updated the
design page
to use "cn=Domain Level,cn=ipa,cn=etc,SUFFIX". Tomas, please
send
the updated
patch set, it should be an extremely simple change :-)
I prefer the ipa parent and the space in the name, so I'm glad we
could agree
on this without much bikeshedding.
Updated patch attaced.
Tomas
I still see
+# Create default Domain Level entry if it does not exist
+dn: cn=DomainLevel,cn=ipa,cn=etc,$SUFFIX
+default: objectClass: top
+default: objectClass: nsContainer
+default: objectClass: ipaDomainLevelConfig
+default: ipaDomainLevel: 0
...
Right, the space eluded me there, thanks for the catch.
Tomas
A new iteration of the patch, including the server-side checks
for the
installers.
Tomas
1)
https://www.redhat.com/archives/freeipa-devel/2015-May/msg00228.html
- I still don't agree that the plugin should be based on LDAPObject.
On the other hand, with LDAPObject base, Web UI for this feature is
much
more simpler because it can rely on existing conventions.
Following this logic, should *everything* be based on LDAPObject,
because it would satisfy the convetion? I don't think so. The convetion
should not apply here, because domain level is conceptually *not* an
object, it is a property. IMHO having a clean API should be preferred
over implementation convenience.
I do not have strong opinions over this. Attached version implements
a lightweight approach to the domainlevel related commands.
Tomas
Fixes a slight schema glitch.
Thanks for the patch!
1)
+ # Detect the current domain level
+ try:
+ current = remote_api.Command['domainlevel_show']['result']
+ except KeyError:
+ # If we're joining an older master, domainlevel_show is
not
+ # available
+ current = 0
KeyError? That does not look right. remote_api differs from api only in
that it sets up ldap2 to connect to the remote server, but it uses local
plugins and everything, so domainlevel_show should always be available.
2) Could you also set supported domain levels in
install/share/master-entry.ldif? I think it makes sense to have them
there right from the beginning of server install.
3) I think you should use the per-plugin api object instead of
ipalib.api when constructing DNs (domainlevel_dn, container_masters).
4) I would say the opposite of "domainlevel-set" should be
"domainlevel-get", not "domainlevel-show". IMO it's OK since property
commands are an uncharted territory and don't have to (maybe even
shouldn't) use the same convention as objects.
5)
+ 'System: Read Domain Level': {
+ 'ipapermlocation': DN('cn=masters,cn=ipa,cn=etc', api.env.basedn),
+ 'ipapermtargetfilter': {'(objectclass=ipadomainlevelconfig)'},
+ 'ipapermbindruletype': 'all',
+ 'ipapermright': {'read', 'search', 'compare'},
+ 'ipapermdefaultattr': {
+ 'ipadomainlevel', 'objectclass',
+ },
+ },
Shouldn't ipapermlocation say "cn=Domain Level,cn=ipa,cn=etc"?
Thanks for the review, I fixed all the issues raised.
Tomas
Added a small fix for replca install, to avoid duplicated creation of
the domainlevel entry.
Tomas
Aand the correct patch.
Thanks, ACK.
Pushed to master: f3010498af2a4b98512d219b8e09101176c172fe
--
Jan Cholasta
--
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
