Merge pull request #1607 from iaguis/fix-overlay-rootfs-permissions

stage0: use rootfs image permissions with overlay
rkt · Oct 15, 2015 · 8826d83 · 8826d83
2 parents fad44b5 + b154431
commit 8826d83
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 10 deletions.
diff --git a/makelib/variables.mk.in b/makelib/variables.mk.in
@@ -89,6 +89,9 @@ GO_TEST_FUNC_ARGS ?=
 
 GO_ENV := $(strip GOPATH="$(GOPATH)" $(if $(strip $(GOROOT)),GOROOT=$(strip $(GOROOT))))
 
+INSTALL_DIRS += \
+	$(ACIROOTFSDIR):0750
+
 CREATE_DIRS += \
 	$(BUILDDIR) \
 	$(STAMPSDIR) \
@@ -98,7 +101,6 @@ CREATE_DIRS += \
 	$(DEPSDIR) \
 	$(REVSDIR) \
 	$(ACIDIR) \
-	$(ACIROOTFSDIR) \
 	$(FILELISTDIR) \
 	$(MAINTEMPDIR) \
 	$(CLEANDIR)
diff --git a/stage0/run.go b/stage0/run.go
@@ -658,12 +658,17 @@ func copyAppManifest(cdir string, appName types.ACName, dest string) error {
 // overlay filesystem.
 // It mounts an overlay filesystem from the cached tree of the image as rootfs.
 func overlayRender(cfg RunConfig, treeStoreID string, cdir string, dest string, appName string) error {
-	destRootfs := path.Join(dest, "rootfs")
-	if err := os.MkdirAll(destRootfs, defaultRegularDirPerm); err != nil {
+	cachedTreePath := cfg.Store.GetTreeStoreRootFS(treeStoreID)
+	fi, err := os.Stat(cachedTreePath)
+	if err != nil {
 		return err
 	}
+	imgMode := fi.Mode()
 
-	cachedTreePath := cfg.Store.GetTreeStoreRootFS(treeStoreID)
+	destRootfs := path.Join(dest, "rootfs")
+	if err := os.MkdirAll(destRootfs, imgMode); err != nil {
+		return err
+	}
 
 	overlayDir := path.Join(cdir, "overlay")
 	if err := os.MkdirAll(overlayDir, defaultRegularDirPerm); err != nil {
@@ -692,7 +697,7 @@ func overlayRender(cfg RunConfig, treeStoreID string, cdir string, dest string,
 	}
 
 	upperDir := path.Join(imgDir, "upper", appName)
-	if err := os.MkdirAll(upperDir, defaultRegularDirPerm); err != nil {
+	if err := os.MkdirAll(upperDir, imgMode); err != nil {
 		return err
 	}
 	if err := label.SetFileLabel(upperDir, cfg.MountLabel); err != nil {

diff --git a/stage1/usr_from_coreos/usr_from_coreos.mk b/stage1/usr_from_coreos/usr_from_coreos.mk
@@ -90,7 +90,7 @@ $(call forward-vars,$(UFC_MKBASE_STAMP), \
 $(UFC_MKBASE_STAMP): $(UFC_SQUASHFS) $(UFC_FILELIST)
 	set -e; \
 	rm -rf "$(UFC_ROOTFS)"; \
-	mkdir -p "$(UFC_ROOTFS)"; \
+	install -m 0750 -d "$(UFC_ROOTFS)"; \
 	unsquashfs -d "$(UFC_ROOTFS)/usr" -ef "$(UFC_FILELIST)" "$(UFC_SQUASHFS)"; \
 	touch "$@"
 

diff --git a/stage1/usr_from_src/usr_from_src.mk b/stage1/usr_from_src/usr_from_src.mk
@@ -78,6 +78,7 @@ $(call forward-vars,$(UFS_SYSTEMD_INSTALL_STAMP), \
 $(UFS_SYSTEMD_INSTALL_STAMP): $(UFS_SYSTEMD_BUILD_STAMP)
 	set -e; \
 	DESTDIR="$(abspath $(UFS_ROOTFSDIR))" $(MAKE) -C "$(UFS_SYSTEMD_BUILDDIR)" install-strip; \
+	chmod 0750 "$(UFS_ROOTFSDIR)"; \
 	touch "$@"
 
 # This filelist can be generated only after the installation of

diff --git a/tests/functional.mk b/tests/functional.mk
@@ -29,7 +29,8 @@ FTST_EMPTY_IMAGE_MANIFEST := $(FTST_EMPTY_IMAGE_DIR)/manifest
 
 TOPLEVEL_CHECK_STAMPS += $(FTST_FUNCTIONAL_TESTS_STAMP)
 INSTALL_FILES += $(FTST_IMAGE_MANIFEST_SRC):$(FTST_IMAGE_MANIFEST):- $(FTST_INSPECT_BINARY):$(FTST_ACI_INSPECT):- $(FTST_EMPTY_IMAGE_MANIFEST_SRC):$(FTST_EMPTY_IMAGE_MANIFEST):- $(FTST_ACE_MAIN_IMAGE_MANIFEST_SRC):$(FTST_ACE_MAIN_IMAGE_MANIFEST):- $(FTST_ACE_SIDEKICK_IMAGE_MANIFEST_SRC):$(FTST_ACE_SIDEKICK_IMAGE_MANIFEST):- $(FTST_ECHO_SERVER_BINARY):$(FTST_ACI_ECHO_SERVER):-
-CREATE_DIRS += $(FTST_IMAGE_DIR) $(FTST_IMAGE_ROOTFSDIR) $(FTST_EMPTY_IMAGE_DIR) $(FTST_EMPTY_IMAGE_ROOTFSDIR) $(FTST_IMAGE_TEST_DIRS) $(FTST_TEST_TMP)
+CREATE_DIRS += $(FTST_IMAGE_DIR) $(FTST_EMPTY_IMAGE_DIR) $(FTST_EMPTY_IMAGE_ROOTFSDIR) $(FTST_IMAGE_TEST_DIRS) $(FTST_TEST_TMP)
+INSTALL_DIRS += $(FTST_IMAGE_ROOTFSDIR):0755
 CLEAN_FILES += $(FTST_IMAGE) $(FTST_ECHO_SERVER_BINARY) $(FTST_INSPECT_BINARY) $(FTST_EMPTY_IMAGE) $(FTST_IMAGE_ROOTFSDIR)/dir1/file $(FTST_IMAGE_ROOTFSDIR)/dir2/file $(FTST_ACE_BINARY)
 CLEAN_DIRS += $(FTST_IMAGE_ROOTFSDIR)/dir1 $(FTST_IMAGE_ROOTFSDIR)/dir2
 CLEAN_SYMLINKS += $(FTST_IMAGE_ROOTFSDIR)/inspect-link

diff --git a/tests/rkt_userns_test.go b/tests/rkt_userns_test.go
@@ -31,18 +31,20 @@ var usernsTests = []struct {
 }{
 	{
 		`^RKT_BIN^ --debug --insecure-skip-verify run ^USERNS^ --no-overlay --set-env=FILE=^FILE^ --mds-register=false ^IMAGE^`,
-		"/",    // stage2 rootfs ($POD/stage1/rootfs/opt/stage2/rkt-inspect)
-		"drwx", // TODO: revisit the permissions with #1581
+		"/", // stage2 rootfs ($POD/stage1/rootfs/opt/stage2/rkt-inspect)
+		"drwxr-xr-x",
 		"0",
 		"0",
 	},
 	{
 		`^RKT_BIN^ --debug --insecure-skip-verify run ^USERNS^ --no-overlay --set-env=FILE=^FILE^ --mds-register=false ^IMAGE^`,
 		"/proc/1/root/", // stage1 rootfs ($POD/stage1/rootfs)
-		"drwx",          // TODO: revisit the permissions with #1581
+		"drwxr-x---",
 		"0",
 		"", // no check: it could be 0 but also the gid of 'rkt', see https://github.com/coreos/rkt/pull/1452
 	},
+	// TODO test with overlay fs too. We don't test it for now because
+	// Semaphore doesn't support it.
 }
 
 func TestUserns(t *testing.T) {