From 897656b4891fdf47c7f221b4b56c563e9bfb8579 Mon Sep 17 00:00:00 2001 From: Mauricio Poppe Date: Fri, 7 Jan 2022 21:01:23 +0000 Subject: [PATCH] RmdirContents server implementation --- Makefile | 2 +- integrationtests/filesystem_v2alpha1_test.go | 99 ++++++++++++++++++- pkg/os/filesystem/api.go | 25 +++++ pkg/server/filesystem/impl/types.go | 22 +++++ .../impl/v2alpha1/conversion_generated.go | 38 +++++++ pkg/server/filesystem/server.go | 16 +++ pkg/server/filesystem/server_test.go | 3 + pkg/server/smb/server_test.go | 3 + scripts/bump-version.sh | 5 +- 9 files changed, 208 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 87ea0947..ce9c0d38 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ all: build test # include release tools for building binary and testing targets include release-tools/build.make -BUILD_PLATFORMS=windows amd64 .exe +BUILD_PLATFORMS=windows amd64 amd64 .exe GOPATH ?= $(shell go env GOPATH) REPO_ROOT = $(CURDIR) BUILD_DIR = bin diff --git a/integrationtests/filesystem_v2alpha1_test.go b/integrationtests/filesystem_v2alpha1_test.go index 950d3db4..24535659 100644 --- a/integrationtests/filesystem_v2alpha1_test.go +++ b/integrationtests/filesystem_v2alpha1_test.go @@ -2,7 +2,9 @@ package integrationtests import ( "context" + "errors" "fmt" + "io/ioutil" "math/rand" "os" "path/filepath" @@ -125,7 +127,7 @@ func v2alpha1FilesystemTests(t *testing.T) { err = os.Mkdir(targetStagePath, os.ModeDir) require.Nil(t, err) defer os.Remove(targetStagePath) - // Create a sym link + // Create a symlink err = os.Symlink(targetStagePath, lnTargetStagePath) require.Nil(t, err) defer os.Remove(lnTargetStagePath) @@ -147,4 +149,99 @@ func v2alpha1FilesystemTests(t *testing.T) { require.Nil(t, err) require.Equal(t, isSymlink.IsSymlink, false) }) + t.Run("RmdirContents", func(t *testing.T) { + client, err := v2alpha1client.NewClient() + require.Nil(t, err) + defer client.Close() + + r1 := rand.New(rand.NewSource(time.Now().UnixNano())) + rand1 := r1.Intn(100) + + rootPath := getKubeletPathForTest(fmt.Sprintf("testplugin-%d.csi.io", rand1), t) + // this line should delete the rootPath because only its content were deleted + defer os.RemoveAll(rootPath) + + paths := []string{ + filepath.Join(rootPath, "foo/goo/"), + filepath.Join(rootPath, "foo/bar/baz/"), + filepath.Join(rootPath, "alpha/beta/gamma/"), + } + for _, path := range paths { + err = os.MkdirAll(path, os.ModeDir) + require.Nil(t, err) + } + + rmdirContentsRequest := &v2alpha1.RmdirContentsRequest{ + Path: rootPath, + } + _, err = client.RmdirContents(context.Background(), rmdirContentsRequest) + require.Nil(t, err) + + // the root path should exist + exists, err := pathExists(rootPath) + assert.True(t, exists, err) + // the root path children shouldn't exist + for _, path := range paths { + exists, err = pathExists(path) + assert.False(t, exists, err) + } + }) + + t.Run("RmdirContentsNoFollowSymlink", func(t *testing.T) { + // RmdirContents should not delete the target of a symlink, only the symlink + client, err := v2alpha1client.NewClient() + require.Nil(t, err) + defer client.Close() + + r1 := rand.New(rand.NewSource(time.Now().UnixNano())) + rand1 := r1.Intn(100) + + rootPath := getKubeletPathForTest(fmt.Sprintf("testplugin-%d.csi.io", rand1), t) + // this line should delete the rootPath because only its content were deleted + defer os.RemoveAll(rootPath) + + insidePath := filepath.Join(rootPath, "inside/") + outsidePath := filepath.Join(rootPath, "outside/") + paths := []string{ + filepath.Join(insidePath, "foo/goo/"), + filepath.Join(insidePath, "foo/bar/baz/"), + filepath.Join(insidePath, "foo/beta/gamma/"), + outsidePath, + } + for _, path := range paths { + err = os.MkdirAll(path, os.ModeDir) + require.Nil(t, err) + } + + // create a temp file on the outside and make a symlink from the inside to the outside + outsideFile := filepath.Join(outsidePath, "target") + insideFile := filepath.Join(insidePath, "source") + + file, err := os.Create(outsideFile) + require.Nil(t, err) + defer file.Close() + err = os.Symlink(outsideFile, insideFile) + require.Nil(t, err) + + rmdirContentsRequest := &v2alpha1.RmdirContentsRequest{ + Path: insidePath, + } + _, err = client.RmdirContents(context.Background(), rmdirContentsRequest) + require.Nil(t, err) + + // the inside path should exist + exists, err := pathExists(insidePath) + require.Nil(t, err) + assert.True(t, exists, "The path shouldn't exist") + // it should have no children + children, err := ioutil.ReadDir(insidePath) + require.Nil(t, err) + assert.True(t, len(children) == 0, "The RmdirContents path to delete shouldn't have children") + // the symlink target should exist + _, err = os.Open(outsideFile) + if errors.Is(err, os.ErrNotExist) { + // the file should exist but it was deleted! + t.Fatalf("File outsideFile=%s doesn't exist", outsideFile) + } + }) } diff --git a/pkg/os/filesystem/api.go b/pkg/os/filesystem/api.go index adfeab0e..7a033b63 100644 --- a/pkg/os/filesystem/api.go +++ b/pkg/os/filesystem/api.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" ) @@ -18,6 +19,7 @@ type API interface { PathValid(path string) (bool, error) Mkdir(path string) error Rmdir(path string, force bool) error + RmdirContents(path string) error CreateSymlink(oldname string, newname string) error IsSymlink(path string) (bool, error) } @@ -78,6 +80,29 @@ func (filesystemAPI) Rmdir(path string, force bool) error { return os.Remove(path) } +// RmdirContents removes the contents of a directory with `os.RemoveAll` +func (filesystemAPI) RmdirContents(path string) error { + dir, err := os.Open(path) + if err != nil { + return err + } + defer dir.Close() + + files, err := dir.Readdirnames(-1) + if err != nil { + return err + } + for _, file := range files { + candidatePath := filepath.Join(path, file) + err = os.RemoveAll(candidatePath) + if err != nil { + return err + } + } + + return nil +} + // CreateSymlink creates newname as a symbolic link to oldname. func (filesystemAPI) CreateSymlink(oldname, newname string) error { return os.Symlink(oldname, newname) diff --git a/pkg/server/filesystem/impl/types.go b/pkg/server/filesystem/impl/types.go index 7b904f88..bcfade62 100644 --- a/pkg/server/filesystem/impl/types.go +++ b/pkg/server/filesystem/impl/types.go @@ -61,6 +61,28 @@ type RmdirRequest struct { type RmdirResponse struct { } +type RmdirContentsRequest struct { + // The path to remove in the host's filesystem. + // All special characters allowed by Windows in path names will be allowed + // except for restrictions noted below. For details, please check: + // https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file + // + // Restrictions: + // Only absolute path (indicated by a drive letter prefix: e.g. "C:\") is accepted. + // Depending on the context parameter of this function, the path prefix needs + // to match the paths specified either as kubelet-csi-plugins-path + // or as kubelet-pod-path parameters of csi-proxy. + // UNC paths of the form "\\server\share\path\file" are not allowed. + // All directory separators need to be backslash character: "\". + // Characters: .. / : | ? * in the path are not allowed. + // Path cannot be a file of type symlink. + // Maximum path length will be capped to 260 characters. + Path string +} + +type RmdirContentsResponse struct { +} + type CreateSymlinkRequest struct { // The path of the existing directory to be linked. // All special characters allowed by Windows in path names will be allowed diff --git a/pkg/server/filesystem/impl/v2alpha1/conversion_generated.go b/pkg/server/filesystem/impl/v2alpha1/conversion_generated.go index ceb7469a..a58977c4 100644 --- a/pkg/server/filesystem/impl/v2alpha1/conversion_generated.go +++ b/pkg/server/filesystem/impl/v2alpha1/conversion_generated.go @@ -165,6 +165,44 @@ func Convert_impl_PathExistsResponse_To_v2alpha1_PathExistsResponse(in *impl.Pat return autoConvert_impl_PathExistsResponse_To_v2alpha1_PathExistsResponse(in, out) } +func autoConvert_v2alpha1_RmdirContentsRequest_To_impl_RmdirContentsRequest(in *v2alpha1.RmdirContentsRequest, out *impl.RmdirContentsRequest) error { + out.Path = in.Path + return nil +} + +// Convert_v2alpha1_RmdirContentsRequest_To_impl_RmdirContentsRequest is an autogenerated conversion function. +func Convert_v2alpha1_RmdirContentsRequest_To_impl_RmdirContentsRequest(in *v2alpha1.RmdirContentsRequest, out *impl.RmdirContentsRequest) error { + return autoConvert_v2alpha1_RmdirContentsRequest_To_impl_RmdirContentsRequest(in, out) +} + +func autoConvert_impl_RmdirContentsRequest_To_v2alpha1_RmdirContentsRequest(in *impl.RmdirContentsRequest, out *v2alpha1.RmdirContentsRequest) error { + out.Path = in.Path + return nil +} + +// Convert_impl_RmdirContentsRequest_To_v2alpha1_RmdirContentsRequest is an autogenerated conversion function. +func Convert_impl_RmdirContentsRequest_To_v2alpha1_RmdirContentsRequest(in *impl.RmdirContentsRequest, out *v2alpha1.RmdirContentsRequest) error { + return autoConvert_impl_RmdirContentsRequest_To_v2alpha1_RmdirContentsRequest(in, out) +} + +func autoConvert_v2alpha1_RmdirContentsResponse_To_impl_RmdirContentsResponse(in *v2alpha1.RmdirContentsResponse, out *impl.RmdirContentsResponse) error { + return nil +} + +// Convert_v2alpha1_RmdirContentsResponse_To_impl_RmdirContentsResponse is an autogenerated conversion function. +func Convert_v2alpha1_RmdirContentsResponse_To_impl_RmdirContentsResponse(in *v2alpha1.RmdirContentsResponse, out *impl.RmdirContentsResponse) error { + return autoConvert_v2alpha1_RmdirContentsResponse_To_impl_RmdirContentsResponse(in, out) +} + +func autoConvert_impl_RmdirContentsResponse_To_v2alpha1_RmdirContentsResponse(in *impl.RmdirContentsResponse, out *v2alpha1.RmdirContentsResponse) error { + return nil +} + +// Convert_impl_RmdirContentsResponse_To_v2alpha1_RmdirContentsResponse is an autogenerated conversion function. +func Convert_impl_RmdirContentsResponse_To_v2alpha1_RmdirContentsResponse(in *impl.RmdirContentsResponse, out *v2alpha1.RmdirContentsResponse) error { + return autoConvert_impl_RmdirContentsResponse_To_v2alpha1_RmdirContentsResponse(in, out) +} + func autoConvert_v2alpha1_RmdirRequest_To_impl_RmdirRequest(in *v2alpha1.RmdirRequest, out *impl.RmdirRequest) error { out.Path = in.Path out.Force = in.Force diff --git a/pkg/server/filesystem/server.go b/pkg/server/filesystem/server.go index 3ddb8211..453a6394 100644 --- a/pkg/server/filesystem/server.go +++ b/pkg/server/filesystem/server.go @@ -163,6 +163,22 @@ func (s *Server) Rmdir(ctx context.Context, request *internal.RmdirRequest, vers } return nil, err } + +func (s *Server) RmdirContents(ctx context.Context, request *internal.RmdirContentsRequest, version apiversion.Version) (*internal.RmdirContentsResponse, error) { + klog.V(2).Infof("Request: RmdirContents with path=%q", request.Path) + err := s.validatePathWindows(request.Path) + if err != nil { + klog.Errorf("failed validatePathWindows %v", err) + return nil, err + } + err = s.hostAPI.RmdirContents(request.Path) + if err != nil { + klog.Errorf("failed RmdirContents %v", err) + return nil, err + } + return nil, err +} + func (s *Server) LinkPath(ctx context.Context, request *internal.LinkPathRequest, version apiversion.Version) (*internal.LinkPathResponse, error) { klog.V(2).Infof("Request: LinkPath with targetPath=%q sourcePath=%q", request.TargetPath, request.SourcePath) createSymlinkRequest := &internal.CreateSymlinkRequest{ diff --git a/pkg/server/filesystem/server_test.go b/pkg/server/filesystem/server_test.go index 9d2bf9c6..f9a9facd 100644 --- a/pkg/server/filesystem/server_test.go +++ b/pkg/server/filesystem/server_test.go @@ -25,6 +25,9 @@ func (fakeFileSystemAPI) Mkdir(path string) error { func (fakeFileSystemAPI) Rmdir(path string, force bool) error { return nil } +func (fakeFileSystemAPI) RmdirContents(path string) error { + return nil +} func (fakeFileSystemAPI) CreateSymlink(tgt string, src string) error { return nil } diff --git a/pkg/server/smb/server_test.go b/pkg/server/smb/server_test.go index ea2a1a7a..e2b2e649 100644 --- a/pkg/server/smb/server_test.go +++ b/pkg/server/smb/server_test.go @@ -47,6 +47,9 @@ func (fakeFileSystemAPI) Mkdir(path string) error { func (fakeFileSystemAPI) Rmdir(path string, force bool) error { return nil } +func (fakeFileSystemAPI) RmdirContents(path string) error { + return nil +} func (fakeFileSystemAPI) CreateSymlink(tgt string, src string) error { return nil } diff --git a/scripts/bump-version.sh b/scripts/bump-version.sh index d8e5cb38..3bf5496d 100755 --- a/scripts/bump-version.sh +++ b/scripts/bump-version.sh @@ -10,7 +10,6 @@ set -o nounset set -ex -# The bucket url of this script in Google Cloud, set in sync_scripts : "${API_GROUP?API_GROUP is not set}" : "${OLD_API_VERSION:?OLD_API_VERSION is not set, it needs the format vX}" : "${NEW_API_VERSION:?NEW_API_VERSION is not set, it needs the format vX}" @@ -40,11 +39,11 @@ function generate_client_files { rm client/api/$target/api.pb.go || true rm client/groups/$target/client_generated.go || true - # generate client_generated.go - make generate-csi-proxy-api-gen # generate api.pb.go # it's going to fail but it's expected :( make generate-protobuf || true + # generate client_generated.go + make generate-csi-proxy-api-gen # restore files from other API groups (side effect of generate-protobuf) other_leaf_client_files=$(find client/api/ -links 2 -type d -exec echo {} \; | grep -v "$target\$")