Tim thanks for review.
I have attached the patch for CR 7120902.
Could you or anyone else please integrate it.
Abhi.
On 03/20/12 09:45, Tim Foster wrote:
On 03/20/12 12:33 AM, Abhinandan Ekande wrote:
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7120902-rev5/webrev/
Thanks for helping me to resolve the test issue when it was run as
non-root.
No problem, that change looks good to me. Thanks for fixing it.
cheers,
tim
Abhi.
On 03/09/12 10:02, Tim Foster wrote:
Hi there,
On 03/ 6/12 11:45 PM, Abhinandan Ekande wrote:
Tim, as per discussion with you over IM I have made the changes.
The updated webrev is located at :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7120902-rev4/webrev/
This looks fine, with the one addition, that it'd be nice if
sysrepo.py still ran as non-root. You could check $PKG5_TEST_ENV when
trying to chown to pkg5srv - see line 502 for an example.
cheers,
tim
Please let me know your comments.
Thanks,
Abhi.
On 03/05/12 14:06, Abhinandan Ekande wrote:
On 03/05/12 03:49, Tim Foster wrote:
You're right, sorry - I was led astray by the creative use of
'dir.find' there. Any reason why you didn't use 'dir == foo' rather
than 'dir.find(foo)'?
I will do check as you have suggested.
Also, rather than relying on the user and group of the parent
directories of the cache_dir being set correctly, it's probably
better to use the 'SYSREPO_USER' and 'SYSREPO_GROUP' globals - see
line 497 for an example.
The SYSREPO_GROUP is initialized as 'pkg5srv'. As per manifest the
gid
for
cache directory is 'bin'. Therefore I took the route of setting user
and group as per
parent directory of cache directory. Is it okay to set group as
'pkg5srv' for cache
directory ? Or is initializing of SYSREPO_GROUP incorrect ? Please
let
me know.
Thanks,
Abhi.
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
--
Oracle
Abhinandan Ekande
Solaris Install,
Revenue Product Engineering (RPE), Systems
Phone: +91 8041847267 | Fax: +91 80 22231794 | Mobile: +91 9632144088
ORACLE India | Off Langford Road | Bangalore | 560025
Green Oracle<http://www.oracle.com/commitment> Oracle is committed to
developing practices and products that help protect the environment
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
--
Oracle
Abhinandan Ekande
Solaris Install,
Revenue Product Engineering (RPE), Systems
Phone: +91 8041847267 | Fax: +91 80 22231794 | Mobile: +91 9632144088
ORACLE India | Off Langford Road | Bangalore | 560025
Green Oracle <http://www.oracle.com/commitment> Oracle is committed to
developing practices and products that help protect the environment
# HG changeset patch
# User [email protected]
# Date 1332138369 -19800
# Node ID 1f469f27e92d98a478f76dedd587bf1da6d13e69
# Parent 5b92d3ae1b76bbee38f49a8cd972c1f6c64c28df
7120902 var/cache/pkg/sysrepo recreated with broken perms
diff -r 5b92d3ae1b76 -r 1f469f27e92d src/sysrepo.py
--- a/src/sysrepo.py Fri Mar 09 12:23:53 2012 +1300
+++ b/src/sysrepo.py Mon Mar 19 11:56:09 2012 +0530
@@ -19,7 +19,7 @@
#
# CDDL HEADER END
#
-# Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
import atexit
import errno
@@ -286,6 +286,21 @@
no_uri_pubs.append(prefix)
return uri_pub_map, no_uri_pubs
+def _chown_cache_dir(dir):
+ """Sets ownership for cache directory as pkg5srv:bin"""
+
+ uid = portable.get_user_by_name(SYSREPO_USER, None, False)
+ gid = portable.get_group_by_name("bin", None, False)
+ try:
+ os.chown(dir, uid, gid)
+ except OSError, err:
+ if not os.environ.get("PKG5_TEST_ENV", None):
+ raise SysrepoException(
+ _("Unable to chown to %(user)s:%(group)s: "
+ "%(err)s") %
+ {"user": SYSREPO_USER, "group": "bin",
+ "err": err})
+
def _write_httpd_conf(runtime_dir, log_dir, template_dir, host, port,
cache_dir,
cache_size, uri_pub_map, http_proxy, https_proxy):
"""Writes the apache configuration for the system repository."""
@@ -302,9 +317,14 @@
if os.path.exists(dir) and not os.path.isdir(dir):
raise SysrepoException(
_("%s is not a directory") % dir)
+
for dir in dirs:
try:
- os.makedirs(dir, 0700)
+ os.makedirs(dir, 0755)
+ # set pkg5srv:bin as ownership for cache
+ # directory.
+ if dir == cache_dir:
+ _chown_cache_dir(dir)
except OSError, err:
if err.errno != errno.EEXIST:
raise
diff -r 5b92d3ae1b76 -r 1f469f27e92d src/tests/cli/t_sysrepo.py
--- a/src/tests/cli/t_sysrepo.py Fri Mar 09 12:23:53 2012 +1300
+++ b/src/tests/cli/t_sysrepo.py Mon Mar 19 11:56:09 2012 +0530
@@ -20,19 +20,26 @@
# CDDL HEADER END
#
-# Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
import testutils
if __name__ == "__main__":
testutils.setup_environment("../../../proto")
import pkg5unittest
+import errno
import hashlib
import os
import os.path
import unittest
import urllib2
import shutil
+import stat
+import time
+
+import pkg.portable as portable
+
+SYSREPO_USER = "pkg5srv"
class TestBasicSysrepoCli(pkg5unittest.CliTestCase):
"""Some basic tests checking that we can deal with all of our arguments
@@ -456,5 +463,61 @@
for d in directives:
self.file_contains(self.default_sc_conf, d)
+ def test_12_cache_dir_permissions(self):
+ """Our cache_dir permissions and ownership are verified"""
+
+ exp_uid = portable.get_user_by_name(SYSREPO_USER, None, False)
+ self.image_create(prefix="test1", repourl=self.durl1)
+
+ cache_dir = os.path.join(self.test_root, "t_sysrepo_cache")
+ # first verify that the user running the test has permissions
+ try:
+ os.mkdir(cache_dir)
+ os.chown(cache_dir, exp_uid, 1)
+ os.rmdir(cache_dir)
+ except OSError, e:
+ if e.errno == errno.EPERM:
+ raise pkg5unittest.TestSkippedException(
+ "User running test does not have "
+ "permissions to chown to uid %s" % exp_uid)
+ raise
+
+ # Run sysrepo to create cache directory
+ port = self.next_free_port
+ self.sysrepo("-R %s -c %s -p %s" % (self.get_img_path(),
+ cache_dir, port))
+
+ self._start_sysrepo()
+ self.sc.stop()
+
+ # Remove cache directory
+ os.rmdir(cache_dir)
+
+ # Again run sysrepo and then verify permissions
+ cache_dir = os.path.join(self.test_root, "t_sysrepo_cache")
+ port = self.next_free_port
+ self.sysrepo("-R %s -c %s -p %s" % (self.get_img_path(),
+ cache_dir, port))
+ self._start_sysrepo()
+
+ # Wait for service to come online. Try for 30 seconds.
+ count = 0
+ while (count < 10):
+ time.sleep(3)
+ count = count + 1
+ if (os.access(cache_dir, os.F_OK)):
+ break
+
+ # Verify cache directory exists.
+ self.assertTrue(os.access(cache_dir, os.F_OK))
+
+ filemode = stat.S_IMODE(os.stat(cache_dir).st_mode)
+ self.assertEqualDiff(0755, filemode)
+ uid = os.stat(cache_dir)[4]
+ exp_uid = portable.get_user_by_name(SYSREPO_USER, None, False)
+ self.assertEqualDiff(exp_uid, uid)
+
+ self.sc.stop()
+
if __name__ == "__main__":
unittest.main()
diff -r 5b92d3ae1b76 -r 1f469f27e92d src/tests/pkg5unittest.py
--- a/src/tests/pkg5unittest.py Fri Mar 09 12:23:53 2012 +1300
+++ b/src/tests/pkg5unittest.py Mon Mar 19 11:56:09 2012 +0530
@@ -1206,7 +1206,7 @@
For now, we'll record this as a success, but also save the
reason why we wanted to skip this test"""
self.addSuccess(test)
- self.skips.append((test, err))
+ self.skips.append((str(test), err))
def addPersistentSetupError(self, test, err):
errtype, errval = err[:2]
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
