I really should have made the "retainperms" argument to copyfile.send
optional. This small patch does so by assigning it a default value of
"False". The patch should be applied after my previous patch of September
18th.
--
Marcus
On Tuesday, September 18, 2012 08:01:41 PM Marcus Lauer wrote:
> Attached is an updated version of this patch which makes two more
> changes to the CopyFile minion.
>
> One major change is that it now tries to use the pyxattr library to
> copy extended attributes (including SELinux contexts) from the target file
> to the tempfile. Pyxattr is not a core library but it is packaged for
> RHEL5 and 6. If this library is not available then it falls back to
> calling chcon.
>
> The other major change is that it explicitly looks for a SELinux
> config file. If one is present then it will consider a failure to copy the
> SELinux contexts from the target file to the tempfile to be an error and
> will exit. Previously it just kept going when this failed.
>
> The remaining changes are organizational, e.g. moving the "from
> subprocess import..." to the top.
>
> --
> Marcus
>
> On Monday, September 03, 2012 04:29:28 PM Marcus Lauer wrote:
> > Since part #1 of my three-part plan is proving
> > troublesome I moved>
> > ahead on #2.
> >
> > Although part #3 of my original plan borrows some code
> > from #1 it
> >
> > does not require #1. If this patch is accepted then I will move ahead
> > on
> > #3.
> >
> > This patch changes copyfile in two ways:
> > 1. Copyfile now opens a tempfile on the minion and appends data to this
> > tempfile, then moves the tempfile over the target file after all of the
> > data has been appended.
> >
> > The advantage of this is that updating the target file
> > is atomic, or>
> > as close to it as possible in python. This minimizes the chance of some
> > program accessing an empty or half-complete file. If the file in
> > question were a server config file... surely you can imagine how this
> > could be a problem.
> >
> > The tempfiles are created in the same directory as the
> > target file>
> > (for security) and have the same name as the target file but with the
> > extension ".functmp". Permissions are set on this file during the
> > "open"
> > stage, while it is still zero-size.
> >
> > 2. Copyfile has a new option called "retainperms" ("--retainperms" at
> > the
> > command line). It is "False" by default.
> >
> > Currently func tries to give the new file on the minion
> > the same
> >
> > attributes (permissions and uid/gid) as the file on the overlord. The
> > SELinux context of the new file is not set (and does not change). This
> > overrides this behavior to make the permissions, flags, uid, gid and
> > SELinux contexts of the new file identical to the existing file (if any)
> > on the minion.
> >
> > This is intended to deal with the situation in which the
> > file on the>
> > overlord has different permissions than the files on the minions and the
> > user does not want to overwrite them. Perhaps the overlord computer
> > does
> > not have the same users and groups as the minions? I figured this might
> > be useful for someone.
> >
> > On a related note, does anyone think it would be
> > worthwhile to have>
> > copyfile copy the SELinux contexts of the source file to the target?
> > How
> > about making command-line options to allow the user to specify the
> > desired mode, uid, gid, contexts, etc.?
> >
> > --
> > Marcus
> >
> > On Saturday, August 25, 2012 04:55:09 PM Marcus Lauer wrote:
> > > Attached are my original patch and a git-patch which
> > > contains the
> > >
> > > same code. These implement changes I called #1 in my previous
> > > e-mail.
> > >
> > > #2 doesn't seem too hard. CopyFile.open will have
> > > to
> > > open a
> > >
> > > tempfile rather than the target file. It already returns the
> > > filepath
> > > on
> > > success. Append already uses the return value of CopyFile.open as
> > > the
> > > file to append to. After appending has finished there needs to be a
> > > CopyFile.move function called by the overlord which takes both paths
> > > and moves the tempfile over the source file. Making sure the
> > > ownership/permissions/SELinux attributes of the target file do not
> > > change
> > > could be is important so had better be careful about that.
> > >
> > > Once this is in place #3 is almost trivial. The
> > > overlord needs to>
> > >
> > > calculate the MD5 of the source file. The code for doing this is
> > > also
> > > needed by #1 and is included in my patches. That MD5 should be
> > > passed
> > > to
> > > the CopyFile.move function. It should calculate the MD5 of the
> > > tempfile and if they don't match just delete the tempfile and
> > > return an error instead of moving it. I figure that this MD5 check
> > > should be optional so I will also create a flag to enable it.
> > >
> > > Can anyone see any issues with this general design?
> > > If
> > > not I'll get>
> > >
> > > working on it when I have the time.
> > >
> > > --
> > > Marcus
> > >
> > > On Friday, August 24, 2012 07:32:14 PM you wrote:
> > > > Copyfile could most certainly use some TLC, so let me know if I
> > > > can
> > > > help,
> > > > that would certainly be of help if you'd like to work on that.
> > > > The
> > > > list has indeed changed to lists.fedorahosted.com, so definitely
> > > > use that one.
> > > >
> > > > Would you mind shooting me your original patch as well? Thanks
> > > > very
> > > > much!>
> > > > On Fri, Aug 24, 2012 at 7:22 PM, Marcus Lauer
> >
> > <marcuslauer@xxxxxxxxxx>wrote:
> > > > > It seems to me that there are at least
> > > > > three
> > > > > improvements which
> > > > >
> > > > > should be made to copyfile:
> > > > >
> > > > > 1. If the file on the overlord and minion are the same, that
> > > > > file
> > > > > should not
> > > > > be copied. This saves bandwidth/processing time.
> > > > >
> > > > > 2. Overwriting the file on the minion should be atomic, e.g.
> > > > > the
> > > > > file on the
> > > > > overlord should be copied to a tempfile on the minion then
> > > > > moved
> > > > > to
> > > > > overwrite
> > > > > the target file as a final step. Right now if funcd dies or
> > > > > something
> > > > > goes wrong mid-copy you end up with half a file on the
> > > > > minion.
> > > > >
> > > > > 3. There should be an option (off by default) such that if
> > > > > the
> > > > > file
> > > > > which
> > > > > has
> > > > > been copied over to the minion is not identical to the file
> > > > > on
> > > > > the
> > > > > overlord it
> > > > > is re-copied, or at least does not overwrite the file on the
> > > > > minion.
> > > > > This won't work for some files (busy logfiles) but for most
> > > > > files it
> > > > > seems like an
> > > > > extra bit of reliability which most people would want in
> > > > > most
> > > > > situations.
> > > > >
> > > > > I submitted a patch (though not a
> > > > > git-patch...
> > > > > my
> > > > > bad!)
> > > > > back in
> > > > >
> > > > > January 2011 which implements #1, but imperfectly. The
> > > > > overlord
> > > > > still
> > > > > has to
> > > > > send a (small) packet to the minion for each 60KB chunk of
> > > > > target
> > > > > file.
> > > > >
> > > > > Would anyone be bothered terribly if I worked
> > > > > on
> > > > > #2
> > > > > and
> > > > > #3? While
> > > > >
> > > > > I am
> > > > > at it I will try to find a way to fix up my patch for #1 as
> > > > > well.
> > > > > There is some overlap between these patches. Both will
> > > > > involve
> > > > > calculating an MD5 hash
> > > > > of the file on the overlord, for example.
> > > > >
> > > > > P.S. Is the new mailing list on
> > > > > fedorahosted.org
> > > > > live
> > > > > yet? I am
> > > > >
> > > > > sending this message to both just in case.
> > > > >
> > > > > --
> > > > > Marcus
> > > > > _______________________________________________
> > > > > func mailing list
> > > > > func@xxxxxxxxxxxxxxxxxxxxxx
> > > > > https://lists.fedorahosted.org/mailman/listinfo/func
> > > >
> > > > --
> > > > Steve Salevan
> > > > steve@xxxxxxxxxx>From b421285ec76b27aeebe1c2bbb9299ad2db540b86 Mon Sep 17 00:00:00 2001
From: Marcus Lauer <marcuslauer@xxxxxxxxxx>
Date: Sat, 29 Sep 2012 13:53:44 -0400
Subject: [PATCH] CopyFile: Make the restoreperms argument to copyfile.send
optional
---
func/overlord/modules/copyfile.py | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/func/overlord/modules/copyfile.py b/func/overlord/modules/copyfile.py
index e9c0a1d..bee00ac 100644
--- a/func/overlord/modules/copyfile.py
+++ b/func/overlord/modules/copyfile.py
@@ -8,7 +8,7 @@ from func.overlord import overlord_module
import func.minion.modules.copyfile as CopyFile
class copyfile(overlord_module.BaseModule):
- def send(self, localpath, remotepath, retainperms, bufsize=60000):
+ def send(self, localpath, remotepath, retainperms=False, bufsize=60000):
try:
f = open(localpath, "r")
except IOError, e:
--
1.7.7
_______________________________________________
func mailing list
func@xxxxxxxxxxxxxxxxxxxxxx
https://lists.fedorahosted.org/mailman/listinfo/func
