Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RmdirContents API in the Fileystem v2alpha1 API Group #186

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
332 changes: 255 additions & 77 deletions client/api/filesystem/v2alpha1/api.pb.go

Large diffs are not rendered by default.

27 changes: 27 additions & 0 deletions client/api/filesystem/v2alpha1/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ service Filesystem {
// This may be used for unlinking a symlink created through CreateSymlink.
rpc Rmdir(RmdirRequest) returns (RmdirResponse) {}

// RmdirContents removes the contents of a directory in the host filesystem.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a few more words to mention contents are all the files and sub dirs under the dir. The directory itself will not be deleted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added:

    // Unlike Rmdir it won't delete the requested path, it'll only delete its contents.

// Unlike Rmdir it won't delete the requested path, it'll only delete its contents.
rpc RmdirContents(RmdirContentsRequest) returns (RmdirContentsResponse) {}

// CreateSymlink creates a symbolic link called target_path that points to source_path
// in the host filesystem (target_path is the name of the symbolic link created,
// source_path is the existing path).
Expand Down Expand Up @@ -86,6 +90,29 @@ message RmdirResponse {
// Intentionally empty.
}

message RmdirContentsRequest {
// The path whose contents will be removed 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.
string path = 1;
}

message RmdirContentsResponse {
// Intentionally empty.
}

message CreateSymlinkRequest {
// The path of the existing directory to be linked.
// All special characters allowed by Windows in path names will be allowed
Expand Down
4 changes: 4 additions & 0 deletions client/groups/filesystem/v2alpha1/client_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

99 changes: 98 additions & 1 deletion integrationtests/filesystem_v2alpha1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package integrationtests

import (
"context"
"errors"
"fmt"
"io/ioutil"
"math/rand"
"os"
"path/filepath"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
})
}
25 changes: 25 additions & 0 deletions pkg/os/filesystem/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
)

Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://stackoverflow.com/a/52685448

a reference of implementation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the same implementation as LVP for linux

if err != nil {
return err
}
defer dir.Close()

files, err := dir.Readdirnames(-1)
if err != nil {
return err
}
for _, file := range files {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you didn't implement force option here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, fixed

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)
Expand Down
22 changes: 22 additions & 0 deletions pkg/server/filesystem/impl/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/server/filesystem/impl/types_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 38 additions & 0 deletions pkg/server/filesystem/impl/v2alpha1/conversion_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions pkg/server/filesystem/impl/v2alpha1/server_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions pkg/server/filesystem/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
3 changes: 3 additions & 0 deletions pkg/server/filesystem/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/server/smb/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions scripts/bump-version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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\$")
Expand Down
Loading