1) I'm a little confused at the directory name. You create bakup/secfiles at the line 511, and copy savefile to bakup/tmp... One is supposed to be the other?
2) $admConf->{configdir} already has the "bakup" directory? If not, perl mkdir works recursively like "mkdir -p"?
3) Usually, directory's mode is 0755? (If no other user has a chance to go into the directory, it does not matter, though... :)
{{{
511 mkdir ('$admConf->{configdir}/bakup/secfiles', 0744);
512 foreach my $savefile (@reconfigsavefiles) {
513 copy ("$admConf->{configdir}/$savefile", "$admConf->{configdir}/bakup/tmp/");
514 }
}}}

Replying to [comment:2 nhosoi]:

1) I'm a little confused at the directory name. You create bakup/secfiles at the line 511, and copy savefile to bakup/tmp... One is supposed to be the other?

Originally it was bakup/tmp during my testing. but I decided to use a more meaningful name. I obviously didn't fully make that name change, good catch.

2) $admConf->{configdir} already has the "bakup" directory? If not, perl mkdir works recursively like "mkdir -p"?

Yes, the bakup directory already exists.

3) Usually, directory's mode is 0755? (If no other user has a chance to go into the directory, it does not matter, though... :)

It doesn't really matter, because we create and destroy this directory during the reconfiguration process.

New patch is attached.

{{{
511 mkdir ('$admConf->{configdir}/bakup/secfiles', 0744);
512 foreach my $savefile (@reconfigsavefiles) {
513 copy ("$admConf->{configdir}/$savefile", "$admConf->{configdir}/bakup/tmp/");
514 }
}}}

Thanks for the clarification and revision, Mark!

Replying to [comment:4 nhosoi]:

Thanks for the clarification and revision, Mark!

Actually I found some other issues with the fix. I need to rework it...

New patch attached

revision
0001-Ticket-47891-Admin-Server-reconfig-breaks-SSL-config.patch

To ssh://git.fedorahosted.org/git/389/admin.git
376cb98..fa79ba1 master -> master

commit fa79ba174a410571af6206568877f91ccfe9aa8e
Author: Mark Reynolds mreynolds@redhat.com
Date: Mon Sep 8 16:41:23 2014 -0400

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1173252

git patch file (master) -- Use mkdtemp to create a temporary backup dir.
0001-Ticket-47891-Admin-Server-reconfig-breaks-SSL-config.2.patch

{{{
if ( ! -d $secfile_backup_dir){
519 mkdir ($secfile_backup_dir, 0755);
522 debug(0, "Failed to create backup dir $secfile_backup_dir\n");
523 return 1;
520 524 }
}}}
I think you should use a setup $FATAL msg for this - e.g. something like this:
{{{
$setup->msg($FATAL, 'error_creating_secfile_backup', $secfile_backup_dir, $!);
}}}
where error_creating_secfile_backup is defined in setup-ds-admin.res.in like this:
{{{
error_creating_secfile_backup = Could not create temporary directory %s to\
backup security files. Error: %s
}}}
This should be a fatal error and abort the setup process. Same in restore - use a key like error_reading_secfile_backup and have the message refer to trying to access, not create, the secfile backup dir.

In createAdminServer you should check the return value from reconfig_backup_secfiles and reconfig_restore_secfiles and handle an error return like a fatal error.

Thank you, Rich!

Plus, I'm afraid this code could cause the same issue as #47991. Move between potentially separated file systems. I should change the backup dir to the local area...

{{{

523     $secfile_backup_dir = mkdtemp($my_template_backup_dir);

518 524 if ( ! -d $secfile_backup_dir){
519 mkdir ($secfile_backup_dir, 0755);
525 $setup->msg($FATAL, 'error_creating_secfile_backup', $secfile_backup_dir, $?);
}}}
Does mkdtemp fork/exec/system an external program, or does it handle it in the current process? If it is the former, then using $? is correct. If not, then you want to use $!. See http://perldoc.perl.org/perlvar.html

git patch file (master) -- Use mkdtemp to create a temporary backup dir + revised based upon the comments by Rich (Thank you!!) + replaced '$?' with '$!'
0001-Ticket-47891-Admin-Server-reconfig-breaks-SSL-config.3.patch

Reviewed by Rich (Thank you!!)

Pushed to master:
2524f08..894e5b9 master -> master
commit 894e5b9a64c2edaea6f1a1ef089015a1920169df

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1222

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.