From 02d2f97747ea1da5480954043f9f76f77fb44dcc Mon Sep 17 00:00:00 2001
From: "Pradipta Kr. Banerjee" <pradipta.banerjee@gmail.com>
Date: Thu, 18 Jun 2020 13:34:54 +0000
Subject: [PATCH] oci: Fix running of OCI hooks

OCI hooks fails to run since the code was writing the config.json
to the read-only path. This patch fixes it

Fixes: #2763

Signed-off-by: Pradipta Kr. Banerjee <pradipta.banerjee@gmail.com>
---
 grpc.go     |  7 +++++--
 oci.go      | 22 ++++++++++++++--------
 oci_test.go | 19 +++++++++++++++----
 3 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/grpc.go b/grpc.go
index 829881f12f..69e73bd9c6 100644
--- a/grpc.go
+++ b/grpc.go
@@ -676,14 +676,14 @@ func (a *agentGRPC) CreateContainer(ctx context.Context, req *pb.CreateContainer
 		a.sandbox.addGuestHooks(ociSpec)
 
 		// write the OCI spec to a file so that hooks can read it
-		err = writeSpecToFile(ociSpec)
+		err = writeSpecToFile(ociSpec, req.ContainerId)
 		if err != nil {
 			return emptyResp, err
 		}
 
 		// Change cwd because libcontainer assumes the bundle path is the cwd:
 		// https://github.com/opencontainers/runc/blob/v1.0.0-rc5/libcontainer/specconv/spec_linux.go#L157
-		oldcwd, err := changeToBundlePath(ociSpec)
+		oldcwd, err := changeToBundlePath(ociSpec, req.ContainerId)
 		if err != nil {
 			return emptyResp, err
 		}
@@ -1305,6 +1305,9 @@ func (a *agentGRPC) RemoveContainer(ctx context.Context, req *pb.RemoveContainer
 		}
 	}
 
+	configJsonDir := filepath.Join("/run/libcontainer/", req.ContainerId)
+	//Best effort.. Ignore any errors in the deletion of the directory
+	_ = os.RemoveAll(configJsonDir)
 	delete(a.sandbox.containers, ctr.id)
 
 	return emptyResp, nil
diff --git a/oci.go b/oci.go
index 7dd849e876..96c8ee8ff1 100644
--- a/oci.go
+++ b/oci.go
@@ -22,13 +22,18 @@ import (
 const (
 	ociConfigFile     string      = "config.json"
 	ociConfigFileMode os.FileMode = 0444
+	ociConfigBasePath string      = "/run/libcontainer"
 )
 
-// writeSpecToFile writes the container's OCI spec to "dirname(spec.Root.Path)/config.json"
-// This effectively makes the parent directory a valid OCI bundle.
-func writeSpecToFile(spec *specs.Spec) error {
-	bundlePath := filepath.Dir(spec.Root.Path)
-	configPath := filepath.Join(bundlePath, ociConfigFile)
+// writeSpecToFile writes the container's OCI spec to "/run/libcontainer/<container-id>/config.json"
+// Note that the OCI bundle (rootfs) is at a different path
+func writeSpecToFile(spec *specs.Spec, containerId string) error {
+	configJsonDir := filepath.Join(ociConfigBasePath, containerId)
+	err := os.MkdirAll(configJsonDir, 0700)
+	if err != nil {
+		return err
+	}
+	configPath := filepath.Join(configJsonDir, ociConfigFile)
 	f, err := os.OpenFile(configPath, os.O_WRONLY|os.O_CREATE, ociConfigFileMode)
 	if err != nil {
 		return err
@@ -40,7 +45,7 @@ func writeSpecToFile(spec *specs.Spec) error {
 
 // changeToBundlePath changes the cwd to the OCI bundle path defined as
 // dirname(spec.Root.Path) and returns the old cwd.
-func changeToBundlePath(spec *specs.Spec) (string, error) {
+func changeToBundlePath(spec *specs.Spec, containerId string) (string, error) {
 	cwd, err := os.Getwd()
 	if err != nil {
 		return cwd, err
@@ -51,9 +56,10 @@ func changeToBundlePath(spec *specs.Spec) (string, error) {
 	}
 
 	bundlePath := filepath.Dir(spec.Root.Path)
-	configPath := filepath.Join(bundlePath, ociConfigFile)
+	configPath := filepath.Join(ociConfigBasePath, containerId, ociConfigFile)
 
-	// Verify that config.json is present at the root of the bundle path.
+	// config.json is at "/run/libcontainer/<container-id>/"
+	// Actual bundle (rootfs) is at dirname(spec.Root.Path)
 	if _, err := os.Stat(configPath); err != nil {
 		return cwd, errors.New("invalid OCI bundle")
 	}
diff --git a/oci_test.go b/oci_test.go
index e0ccc37027..2952ccc335 100644
--- a/oci_test.go
+++ b/oci_test.go
@@ -17,6 +17,8 @@ import (
 )
 
 func TestChangeToBundlePath(t *testing.T) {
+	skipUnlessRoot(t)
+	containerId := "1"
 	assert := assert.New(t)
 
 	originalCwd, err := os.Getwd()
@@ -37,15 +39,15 @@ func TestChangeToBundlePath(t *testing.T) {
 		Readonly: false,
 	}
 
-	_, err = changeToBundlePath(spec)
+	_, err = changeToBundlePath(spec, containerId)
 	assert.Error(err)
 
 	// Write the spec file to create a valid OCI bundle
 	spec.Root.Path = rootfsPath
-	err = writeSpecToFile(spec)
+	err = writeSpecToFile(spec, containerId)
 	assert.NoError(err)
 
-	cwd, err := changeToBundlePath(spec)
+	cwd, err := changeToBundlePath(spec, containerId)
 	assert.NoError(err)
 	assert.Equal(cwd, originalCwd)
 
@@ -55,6 +57,8 @@ func TestChangeToBundlePath(t *testing.T) {
 }
 
 func TestWriteSpecToFile(t *testing.T) {
+	skipUnlessRoot(t)
+	containerId := "1"
 	assert := assert.New(t)
 
 	bundlePath, err := ioutil.TempDir("", "bundle")
@@ -75,10 +79,17 @@ func TestWriteSpecToFile(t *testing.T) {
 			Readonly: false,
 		},
 	}
-	err = writeSpecToFile(spec)
+	err = writeSpecToFile(spec, containerId)
 	assert.NoError(err)
 
 	file, err := os.Open(path.Join(bundlePath, ociConfigFile))
+	assert.Error(err)
+	defer file.Close()
+
+	_, err = file.Stat()
+	assert.Error(err)
+
+	file, err = os.Open(path.Join("/run/libcontainer/", containerId, ociConfigFile))
 	assert.NoError(err)
 	defer file.Close()