Merge AD trust installer into composite ones #479

martbab · 2017-02-17T16:49:26Z

This PR implements setup of Samba/Winbind as a part of server/replica install.
I will update installation tests in a separate PR in order not to inflate an
already sizeable amount of code touched in this one.

I also updated man pages of ipa-server/replica-install, but since the entries
are a bit chatty, it may be a good idea to write a more terse option
descriptions and provide a link to ipa-adtrust-install for more thorough
explanation.

The commits from #457 are on the bottom
of the branch in order to provide working AD trust installer in cases where
admin password is not provided.

martbab · 2017-02-22T10:50:51Z

Bump for review.

martbab · 2017-02-22T12:06:55Z

I have added a basic integration tests for the built-in AD trust installation, you can run them on 3 machines (master + 2 replicas) by running

# ipa-run-tests --verbose /usr/lib/python2.7/site-packages/ipatests/test_integration/test_installation.py -k TestADTrustInstall

and having a properly configured test config.

HonzaCholasta · 2017-02-22T15:06:18Z

ipaserver/install/server/__init__.py

@@ -331,6 +336,13 @@ def dirsrv_config_file(self, value):
    )
    pkinit_cert_name = prepare_only(pkinit_cert_name)

+    # TODO: --add-agents makes no sense on server install (since it is a first
+    # master


That's already resolved, isn't it?

HonzaCholasta · 2017-02-22T15:08:12Z

ipaserver/install/server/__init__.py

@@ -548,6 +560,10 @@ def dm_password(self, value):
    def admin_password(self, value):
        validate_admin_password(value)

+    # always run sidgen task and do not allow adding agents on first master
+    add_sids = True


AFAICT add_sids is only relevant when upgrading from an old server which did not have the sidgen plugin enabled, so it in fact should always be False on first master.

The problem is that (for some reason) the 389-ds is populated by several default groups during bootstrap. One of these groups is cn=editors which is POSIX one has no SID assigned when I hardcoded add_sids to False (cn=admins have SID assinged directly by CIFS installer since it has to have special RID of 512).

@rcritten is there any reason why are these groups created during bootstrap and not during LDAP update phase?

MartinBasti · 2017-02-24T15:02:35Z

Works for me, except, ipa-server-install --setup-adtrust works even without freeipa-server-trust-ad package. Please fix this in a new PR in way how DNS is done.

martbab · 2017-02-27T14:11:16Z

I have noticed that the check for installed dependencies is buggy, I will have to fix it before pushing.

Also we would need to move the 'editors' group addition to the LDAP update phase since it remains with missing SID during ipa-server-install when add_sids knob is set to False. @abbra @rcritten is that ok with you? Please see inline comment for more details.

abbra · 2017-02-27T14:43:26Z

If you can differentiate how the installer is being run, then for composite installer always run add_sids.

martbab · 2017-02-27T16:14:53Z

@abbra I think that I am confused by the way sidgen plugin works. During LDAP configuration I can see that sidgen/extdom plugins are activated. e.g:

...
  [43/47]: enabling compatibility plugin
  [44/47]: activating sidgen plugin
  [45/47]: activating extdom plugin
...

Yet unless I install AD trust related bits, there are no SIDs generated on entries I am added (user or groups). When the AD trust installer is run, I see that the sidgen task is activated:

...
 [13/21]: activating sidgen task
 [14/21]: configuring smbd to start on boot
...

The admin user now has SID added by installer, yet the existing POSIX groups (editors) have no SIDs associated with them, only the new user I add afterwards.

Do we have a documentation about the semantics of different sidgen-related operations somewhere? If not, can you please explain the behavior I am seeing here?

abbra · 2017-02-27T16:48:07Z

Unless you specified --add-sids to ipa-adtrust-install (or add_sids=True in ADTrustInstance.setup() call), no task would be run. 'Activating sidgen task' only adds configuration to allow the task to be run.

martbab · 2017-02-27T17:10:52Z

OK I will then hard-code add_sids=True in ipa-server-install

martbab · 2017-02-28T13:03:21Z

I have added a commit that fixes the choeck for missing dependencies in composite installers.

MartinBasti · 2017-03-01T12:57:08Z

Please rebase

Decompose the individual sub-tasks into separate functions. Also perform the lookup only when LDAP is connected. https://fedorahosted.org/freeipa/ticket/6630

This is to prevent errors due to non-existent LDAP connection such as when installing first IPA master. https://fedorahosted.org/freeipa/ticket/6630

Use newly implemented APIs for searching and presenting potential trust agents. https://fedorahosted.org/freeipa/ticket/6639

Plain print messages are a) not logged into files and b) get lost in the output from composite installer. https://fedorahosted.org/freeipa/ticket/6630

There is no point in emitting this message during server/replica install. https://fedorahosted.org/freeipa/ticket/6630

The condition that controls when to check for samba dependencies was misformulated. The check should be run when the installer is *not* run as standalone. In standalone mode the check is already made in different place so the original code triggered it twice. https://fedorahosted.org/freeipa/ticket/6630

This interface is to be used to provide AD trust-related options in server and replica installer. https://fedorahosted.org/freeipa/ticket/6630

ipa-server-install is now able to configure Samba and winbind services and manage trusts to Active Directory right off the bat with following alterations from standalone installer: * sidgen task is always triggered since there are only a few entries to tag in the beginning * the `--add-agents` option is hardcoded to False, as there are no potential agents to resolve and addd when setting up the first master in topology https://fedorahosted.org/freeipa/ticket/6630

`ipa-replica-install` is now able to configure Samba and winbind services in order to manage Active Directory trusts. `--add-agents` option is exposed in replica installer, while `--add-sids` now defaults to `False` since adding a first AD trust controller to an existing sizeable deployment can result in stuck installation as sidgen tasks can take a long time to complete. That's why adding SIDs should be a conscious decision in this case. https://fedorahosted.org/freeipa/ticket/6630

`--rid-base` and `--secondary-rid-base` had `-U` option assigned by error in the man page. Remove it as these options have not short alias. https://fedorahosted.org/freeipa/ticket/6630

Since AD trust installer is now a part of composite installers, their man pages were updated with separate section documenting relevant AD trust-related option descriptions. https://fedorahosted.org/freeipa/ticket/6630

A couple of tests were added to server/replica install integration suite to test AD trust install w/ various combinations of other optional components. https://fedorahosted.org/freeipa/ticket/6630

MartinBasti · 2017-03-01T14:56:29Z

master:

4ba6b96 Refactor the code checking for missing SIDs
c5bae57 only check for netbios name when LDAP backend is connected
9348cfa Refactor the code searching and presenting missing trust agents
c17215e adtrust.py: Use logging to emit error messages
ef37c42 print the installation info only in standalone mode
289060d check for installed dependencies when not in standalone mode
77857ea Add AD trust installer interface for composite installer
13b5821 expose AD trust related knobs in composite installers
aa353c5 Merge AD trust configurator into server installer
eee319d Merge AD trust configurator into replica installer
f62f0b7 Fix erroneous short name options in ipa-adtrust-install man page
23cebe1 Update server/replica installer man pages
612ea7f Provide basic integration tests for built-in AD trust installer

martbab requested review from abbra and HonzaCholasta February 17, 2017 16:53

HonzaCholasta suggested changes Feb 22, 2017

View reviewed changes

martbab force-pushed the adtrust-as-part-of-server-replica-install branch from aae365b to e936f49 Compare February 23, 2017 17:28

HonzaCholasta approved these changes Feb 24, 2017

View reviewed changes

MartinBasti self-assigned this Feb 24, 2017

MartinBasti added the ack Pull Request approved, can be merged label Feb 24, 2017

martbab removed the ack Pull Request approved, can be merged label Feb 27, 2017

martbab force-pushed the adtrust-as-part-of-server-replica-install branch from e936f49 to 936d286 Compare February 28, 2017 13:01

Martin Babinsky added 11 commits March 1, 2017 14:44

Refactor the code checking for missing SIDs

88fa344

Decompose the individual sub-tasks into separate functions. Also perform the lookup only when LDAP is connected. https://fedorahosted.org/freeipa/ticket/6630

only check for netbios name when LDAP backend is connected

7bdf40f

This is to prevent errors due to non-existent LDAP connection such as when installing first IPA master. https://fedorahosted.org/freeipa/ticket/6630

Refactor the code searching and presenting missing trust agents

acd3bd8

Use newly implemented APIs for searching and presenting potential trust agents. https://fedorahosted.org/freeipa/ticket/6639

adtrust.py: Use logging to emit error messages

aa7394a

Plain print messages are a) not logged into files and b) get lost in the output from composite installer. https://fedorahosted.org/freeipa/ticket/6630

print the installation info only in standalone mode

a78069d

There is no point in emitting this message during server/replica install. https://fedorahosted.org/freeipa/ticket/6630

Add AD trust installer interface for composite installer

64012cf

This interface is to be used to provide AD trust-related options in server and replica installer. https://fedorahosted.org/freeipa/ticket/6630

expose AD trust related knobs in composite installers

e9095b7

Fix erroneous short name options in ipa-adtrust-install man page

56b2b81

`--rid-base` and `--secondary-rid-base` had `-U` option assigned by error in the man page. Remove it as these options have not short alias. https://fedorahosted.org/freeipa/ticket/6630

Martin Babinsky added 2 commits March 1, 2017 14:47

Update server/replica installer man pages

fe15f7f

Since AD trust installer is now a part of composite installers, their man pages were updated with separate section documenting relevant AD trust-related option descriptions. https://fedorahosted.org/freeipa/ticket/6630

Provide basic integration tests for built-in AD trust installer

301f8e1

A couple of tests were added to server/replica install integration suite to test AD trust install w/ various combinations of other optional components. https://fedorahosted.org/freeipa/ticket/6630

martbab force-pushed the adtrust-as-part-of-server-replica-install branch from 936d286 to 301f8e1 Compare March 1, 2017 13:47

MartinBasti added the ack Pull Request approved, can be merged label Mar 1, 2017

MartinBasti added the pushed Pull Request has already been pushed label Mar 1, 2017

MartinBasti closed this Mar 1, 2017

martbab deleted the adtrust-as-part-of-server-replica-install branch March 22, 2017 08:46

Merge AD trust installer into composite ones #479

Merge AD trust installer into composite ones #479

martbab commented Feb 17, 2017 •

edited by MartinBasti

martbab commented Feb 22, 2017

martbab commented Feb 22, 2017

HonzaCholasta Feb 22, 2017

HonzaCholasta Feb 22, 2017

martbab Feb 23, 2017

MartinBasti commented Feb 24, 2017

martbab commented Feb 27, 2017

abbra commented Feb 27, 2017

martbab commented Feb 27, 2017

abbra commented Feb 27, 2017

martbab commented Feb 27, 2017

martbab commented Feb 28, 2017

MartinBasti commented Mar 1, 2017

MartinBasti commented Mar 1, 2017

Merge AD trust installer into composite ones #479

Merge AD trust installer into composite ones #479

Conversation

martbab commented Feb 17, 2017 • edited by MartinBasti

martbab commented Feb 22, 2017

martbab commented Feb 22, 2017

HonzaCholasta Feb 22, 2017

Choose a reason for hiding this comment

HonzaCholasta Feb 22, 2017

Choose a reason for hiding this comment

martbab Feb 23, 2017

Choose a reason for hiding this comment

MartinBasti commented Feb 24, 2017

martbab commented Feb 27, 2017

abbra commented Feb 27, 2017

martbab commented Feb 27, 2017

abbra commented Feb 27, 2017

martbab commented Feb 27, 2017

martbab commented Feb 28, 2017

MartinBasti commented Mar 1, 2017

MartinBasti commented Mar 1, 2017

martbab commented Feb 17, 2017 •

edited by MartinBasti