Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR - 2307compat (again) #4143

Closed
389-ds-bot opened this issue Sep 13, 2020 · 39 comments
Closed

PR - 2307compat (again) #4143

389-ds-bot opened this issue Sep 13, 2020 · 39 comments
Labels
merged Migration flag - PR pr Migration flag - PR

Comments

@389-ds-bot
Copy link

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/51090


Third time lucky - this is an update to 2307compat.ldif to resolve the potential conflicts with 60nis.ldif, by removing the conflicts from the 2307compat.ldif. This means that sites that rely on the nis values fro 2307bis.ldif will need to include 60nis.ldif. But it means that any site that already has 60nis.ldif will NOT need to change their config.

Given the fact we already have a difficult (if not impossible ....) process for changing 2307.ldif, it's unlikely many deployments are using 2307bis, and given the rarity of nis in reality, it is unlikely that migrations into 389 will be required to enable 60nis.ldif anyway.

Note that this is TWO commits - one for enable by default, and one for the changes, so that in the case of yet-another failure, we have an easier revert plan.

Thanks!

@389-ds-bot 389-ds-bot added merged Migration flag - PR pr Migration flag - PR labels Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-05-18 06:26:08

ping @tbordaz @mreynolds389 do you mind checking this please :)

@389-ds-bot
Copy link
Author

389-ds-bot commented Sep 13, 2020

Comment from tbordaz (@tbordaz) at 2020-05-18 16:47:51

@Firstyear the patch looks good but running rfc2307compt.py (#3986#comment-642253) it looks some definitions are still problematic.

I removed (nisNetgroupTriple, ,nisMapName, nisMapEntryb, nisNetgroup, nisMap and nisObject) the test went further but still has a problem of replication (where the added definition (objectcategory) is not propagated)

In addition to this test, I think we need to run upgrade test and also a test where an instance got standard definitions in 99user.ldif (#3986#comment-642326). This last bug can be hit with an update of the schema on a replica, the consumer should get std def in 99user.ldif

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-05-19 02:39:06

How did you test the replication in this case @tbordaz? That way I can test it myself to try and work out some of the issues.

@389-ds-bot
Copy link
Author

389-ds-bot commented Sep 13, 2020

Comment from tbordaz (@tbordaz) at 2020-05-19 08:34:01

I made a python test case in the Ticket 50933 (#3986#comment-642253). It creates 2 masters one with rfc2307 and one with rfc2307compat. Then it updates the schema (add a new definition) on one master and checks if the definition gets replicated. This is what happens if a new instance (rfc2307compat) joins a topology with rfc2307 deployed.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-05-21 01:58:59

Thanks, I'll give that a go today. Appreciate it.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-05-21 06:16:07

rebased onto 8e79400422d948bcbc98fe6a18d2e9783d2f633d

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-05-21 06:17:05

Hey @tbordaz I had to adjust your test to get it to work properly, but it ... works? I can't reproduce what's going on here.

It looks like the schema replication works, even though the instances have different rfc2307 instances (master1 compat, master2 the original). I even then extended the test to "simulate" updating the rfc2307 to compat on M2 after the first test, and running again. Again, it all works. So I don't understand why your setup is failing? I'm gonna need more info on this, and maybe you could try with my copy of the test (I've put it into the PR).

Thanks,

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-05-21 06:20:58

Saying that, something I did notice is that basically the entire schema is duplicated into 99user.ldif on master2 which seems ... like a bug. The schemas should be same except the replicated objectCategory and the rfc2307 diffs. There shouldn't be a need to replicate the whole thing :|

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-06-18 07:26:05

@tbordaz @mreynolds389 ping on this one :)

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-16 01:32:11

@tbordaz ping again, it has been a month ....

@389-ds-bot
Copy link
Author

389-ds-bot commented Sep 13, 2020

Comment from tbordaz (@tbordaz) at 2020-07-17 10:59:53

@Firstyear, sorry for the late feedback. It hits same issue (schema not pushed) with the provided testcase (#4143#comment-120258). I had to slightly update the testcase and attached the new version of the testcase in #3986#comment-665860

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-20 01:12:21

All good. I'll check the updated test case and see what's going on. Thanks for that.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-20 06:32:18

rebased onto 9a056781b95606f8705cc5e959efa9b20af636ce

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-20 07:08:46

So I was really stumped to why your test and my test are giving different results - In my test, which is very very similar it works, and schema replication has no issues, but in your test it fails.

I noticed a very odd behaviour in your test. In M1 we have 10rfc2307compat.ldif which contains definitions for automounts and gecos. But it looks like in 99user.ldif these were over-loaded with definitions from rfc2307.ldif from M2.

attributeTypes: (
  1.3.6.1.1.1.1.2 NAME 'gecos'
  DESC 'The GECOS field; the common name'
  EQUALITY caseIgnoreIA5Match
  SUBSTR caseIgnoreIA5SubstringsMatch
  SYNTAX 1.3.6.1.4.1.1466.115.121.1.26
  SINGLE-VALUE
  )

VS

attributeTypes: ( 1.3.6.1.1.1.1.2 NAME 'gecos' DESC 'Standard LDAP attribute t
 ype' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN ( 'RFC 2307'
 'user defined' ) )

I'm wondering if the difference between our tests is not a methodology, but a race condition - your test has M2 -> M1 first, where my test may have M1 -> M2 first. It could explain why my test works and yours does not, but I'm not 100% clear on why this should be different at all.

I'm going to re-run my test and check the replication logs also there to try to understand this, but I now have a reproducer of what you are seeing at least. Thanks again for your patience @tbordaz, but finally I'm making some progress!

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-07-20 08:21:30

I have to say thank you to you @Firstyear for such frustrating investigations. Schema enhancement are always looking simple until diving in various upgrade scenario. I just hope we will find a solution more flexible than asking for a full upgrade of the topology at the same time.

The definitions you are seeing in 99user.ldif (gecos/automount) are likely the result of schema learning. THe server where the schema is updated (add 'objectCategory') evaluates if it can push its schema (because it has a higher schemacsn). So comparing the definitions I guess it detected extended definitions from the other server and updated its schema (99user) with them.
Unfortunately it found some definition that it was not able to learn (incompatible) and then decided it can not push its schema to the other server.

You need to enable replication logging to get verbose explanation what it is doing and why it is failing.

I do not really understand what differs from your testcase with mine. They look similar. Thank for your patience on this. How urgent/important it that ticket for you ?

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-21 01:37:01

You need to enable replication logging to get verbose explanation what it is doing and why it is failing.

Yep I'm going to add this to both tests and compare to understand this.

I do not really understand what differs from your testcase with mine. They look similar. Thank for your patience on this. How urgent/important it that ticket for you ?

It's "important" but there is a few months before it's "urgent". So I'm happy to work on it now to understand it, so I will continue my investigation. I'll let you know what I find :)

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-21 05:29:43

1 new commit added

  • possible fix to the issue

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-21 05:36:54

@tbordaz, Okay I understand the issue now. In my test, I move all the schema into /etc which means that the schema is loaded where 10rfc2307compat.ldif is before 60autofs.ldif. But in your test you only move the 10rfc2307 files, and the system schema remains as is. This means that 10rfc2307 is loaded after which means the definition of automountInformation came from 10rfc2307compat. As automountInformation and others from autofs and compat were different, this is what caused the schema replication to fail as they were conflicting.

60autofs appears to be a "very early" copy of the rfc2307bis automount content, but early enough that it does not match 2307bis.

To resolve this I have modify 60autofs.ldif to be a "super set" combining the two. It retains the 60autofs.ldif may/must rules, and extends the "may" rules to allow a 2307bis object to be added with some slight modifications). This means that your test and my test now both pass with schema replication :)

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-21 05:37:21

Ps: I haven't adjust the commit messages yet, I'll let you review it first.

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-07-21 09:10:02

https://tools.ietf.org/html/draft-howard-rfc2307bis-01 says it is dirstring not IA5. What RFC do you want do you want to follow ?

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-07-21 09:21:15

idem for the MR. I agree that coming from rfc2307compat it was IA5 but https://tools.ietf.org/html/draft-howard-rfc2307bis-01 says it is dirstring.
dirstring including IA5 and if we want to follow rfcbis we could define it as dirstring.

@389-ds-bot
Copy link
Author

389-ds-bot commented Sep 13, 2020

Comment from tbordaz (@tbordaz) at 2020-07-21 09:59:10

Just for recording Ticket is #3986

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-07-21 10:26:15

https://tools.ietf.org/html/draft-howard-rfc2307bis-01 says automountKey MUST but I agree with your change because automoukey was neither required/allowed making it allowed makes sense
RFC does not mention 'cn' shouldn't we relax the requirement of 'cn' moving it to MAY ?

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-07-21 10:29:48

RFC does not mention 'ou' shouldn't we relax the requirement of 'ou' moving it to MAY ?

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-07-21 10:49:03

@Firstyear thanks for the patch, IMHO remains MR and attributes to move(or not) must/may. Did you try your patch with freeipa ipa-server/replica-install ?

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-22 06:37:11

@tbordaz I thought about changing the types in 60autofs to match what is in rfc2307bis, but I think i'd rather leave them as they are. They may already be in use, so I don't want to disturb things too much.

You're right about the possibility of moving ou to may as well. It allows more configurations to be valid, but it also then removes a "strictness". Saying this, I think automount may not be a very popular schema, so I think moving this to MAY is the correct move to "allow many configs".

So I think you're right that we should be as "forgiving" as possible here. Honestly, this whole processhas been a mess and really highlights how poor-past decisions can be hard to undo in the future :(

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-22 07:00:37

rebased onto 295de699b938ab1c7c7e7528d873959c4928df91

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-22 07:01:05

@mreynolds389 I've updated this based @tbordaz's suggestions, so I think it's good to go through a freeipa test now. Thanks!

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-07-28 04:00:30

Hey @mreynolds389 did we ever get this tested by FreeIPA?

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2020-08-04 00:41:15

Hey @mreynolds389 did we ever get this tested by FreeIPA?

They are testing it now, I should have the results tomorrow!

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-08-04 02:41:31

Thank you!

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2020-08-04 18:06:58

Hey @mreynolds389 did we ever get this tested by FreeIPA?

They are testing it now, I should have the results tomorrow!

IPA tests passed! Ack

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-08-05 04:06:06

rebased onto 79d5f2c

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-08-05 04:06:56

Thank you very much @mreynolds389 and @tbordaz ! This is going to make it a lot easier to allow openldap migrations into 389 so I really appreciate your patience with this!

As an FYI @mreynolds389 I'm leaving this as two commits, as there is a an "enable" commit we can quickly revert if we find (yet another) problem here.

Thanks!

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-08-05 04:07:49

Pull-Request has been merged by Firstyear

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2020-08-07 16:09:36

Well the IPA tests passed, but I just ran a test with two different versions of DS with IPA (one with your fix and one without) , and it broke IPA replica install:

The ipa-replica-install command failed, exception: NameNotUnique: NAME not unique for b"( 1.3.6.1.1.1.2.13 NAME 'nisMap' DESC 'Standard LDAP objectclass' SUP top STRUCTURAL MUST nisMapName MAY description X-ORIGIN ( 'RFC 2307' 'user defined' ) )"

I do not know if this is specific to IPA or not. There needs to be more testing. But we need this to work with mixed versions of DS or else we need to look at other options. Maybe we do need to support different/auxiliary schema directories? I'll do some more testing later today...

@389-ds-bot
Copy link
Author

Comment from abbra at 2020-08-07 16:20:13

I don't see nisMap or OID 1.3.6.1.1.1.2.13 in IPA at all, so this is something coming from the 389-ds.

If you look at various 10rfc23097*.ldif files, you can see that there are two different OIDs associated with nisMap:

$ git grep -i nismap
ldap/schema/10rfc2307.ldif:attributeTypes: ( 1.3.6.1.1.1.1.26 NAME 'nisMapName' DESC 'Standard LDAP attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'RFC 2307' )
ldap/schema/10rfc2307.ldif:attributeTypes: ( 1.3.6.1.1.1.1.27 NAME 'nisMapEntry' DESC 'Standard LDAP attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE X-ORIGIN 'RFC 2307' )
ldap/schema/10rfc2307.ldif:objectClasses: ( 1.3.6.1.1.1.2.10 NAME 'nisObject' DESC 'Standard LDAP objectclass' SUP top STRUCTURAL MUST ( cn $ nisMapEntry $ nisMapName ) MAY ( description ) X-ORIGIN 'RFC 2307' )
ldap/schema/10rfc2307.ldif:objectClasses: ( 1.3.6.1.1.1.2.13 NAME 'nisMap' DESC 'Standard LDAP objectclass' SUP top STRUCTURAL MUST ( nisMapName ) MAY ( description ) X-ORIGIN 'RFC 2307' )
ldap/schema/10rfc2307bis.ldif:  1.3.6.1.1.1.1.26 NAME 'nisMapName'
ldap/schema/10rfc2307bis.ldif:  1.3.6.1.1.1.1.27 NAME 'nisMapEntry'
ldap/schema/10rfc2307bis.ldif:  1.3.6.1.1.1.2.9 NAME 'nisMap' SUP top STRUCTURAL
ldap/schema/10rfc2307bis.ldif:  MUST nisMapName
ldap/schema/10rfc2307bis.ldif:  MUST ( cn $ nisMapEntry $ nisMapName )
ldap/schema/10rfc2307compat.ldif:  1.3.6.1.1.1.1.26 NAME 'nisMapName'
ldap/schema/10rfc2307compat.ldif:  1.3.6.1.1.1.1.27 NAME 'nisMapEntry'
ldap/schema/10rfc2307compat.ldif:  1.3.6.1.1.1.2.9 NAME 'nisMap' SUP top STRUCTURAL
ldap/schema/10rfc2307compat.ldif:  MUST nisMapName
ldap/schema/10rfc2307compat.ldif:  MUST ( cn $ nisMapEntry $ nisMapName )

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2020-08-07 17:19:24

1.3.6.1.1.1.2.13 oid comes from rfc2307.ldif. With this patch rfc2307.ldif is no longer a default schema, the new default schema is rfc2307compat.ldif that delivers the same objectclass but with a different OID (IIRC 1.3.6.1.1.1.2.9)

With mixed DS version, it creates an issue having same objectclass with different OID.

IIRC, it was decided to remove nis* definitions and to move them in data/60nis.ldif

@389-ds-bot
Copy link
Author

Patch
51090.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Migration flag - PR pr Migration flag - PR
Projects
None yet
Development

No branches or pull requests

1 participant