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

pkg/archive: Use map of long names avoiding max path limits #8449

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions changelogs/unreleased/8449-kaovilai
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pkg/archive: Backup extractor avoids max path limits
4 changes: 2 additions & 2 deletions internal/delete/delete_item_action_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func InvokeDeleteActions(ctx *Context) error {
}

// get items out of backup tarball into a temp directory
dir, err := archive.NewExtractor(ctx.Log, ctx.Filesystem).UnzipAndExtractBackup(ctx.BackupReader)
dir, longNames, err := archive.NewExtractor(ctx.Log, ctx.Filesystem).UnzipAndExtractBackup(ctx.BackupReader)
if err != nil {
return errors.Wrapf(err, "error extracting backup")
}
Expand All @@ -71,7 +71,7 @@ func InvokeDeleteActions(ctx *Context) error {

ctx.Log.Debugf("Downloaded and extracted the backup file to: %s", dir)

backupResources, err := archive.NewParser(ctx.Log, ctx.Filesystem).Parse(dir)
backupResources, err := archive.NewParser(ctx.Log, ctx.Filesystem).Parse(dir, longNames)
if existErr := errors.Is(err, archive.ErrNotExist); existErr {
ctx.Log.Debug("ignore invoking delete item actions: ", err)
return nil
Expand Down
36 changes: 24 additions & 12 deletions pkg/archive/extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import (
"archive/tar"
"compress/gzip"
"crypto/sha256"
"fmt"
"io"
"path/filepath"

Expand All @@ -42,11 +44,11 @@
}

// UnzipAndExtractBackup extracts a reader on a gzipped tarball to a local temp directory
func (e *Extractor) UnzipAndExtractBackup(src io.Reader) (string, error) {
func (e *Extractor) UnzipAndExtractBackup(src io.Reader) (string, map[string]string, error) {
gzr, err := gzip.NewReader(src)
if err != nil {
e.log.Infof("error creating gzip reader: %v", err)
return "", err
return "", nil, err
}
defer gzr.Close()

Expand All @@ -66,13 +68,18 @@
return nil
}

func (e *Extractor) readBackup(tarRdr *tar.Reader) (string, error) {
// 255 is common limit for file names.
// -5 to account for .json extension
const maxPathLength = 250

// returns tempfir containing backup contents, map[sha256]longNames, error
func (e *Extractor) readBackup(tarRdr *tar.Reader) (string, map[string]string, error) {
dir, err := e.fs.TempDir("", "")
if err != nil {
e.log.Infof("error creating temp dir: %v", err)
return "", err
return "", nil, err

Check warning on line 80 in pkg/archive/extractor.go

View check run for this annotation

Codecov / codecov/patch

pkg/archive/extractor.go#L80

Added line #L80 was not covered by tests
}

longNames := make(map[string]string)
for {
header, err := tarRdr.Next()

Expand All @@ -81,34 +88,39 @@
}
if err != nil {
e.log.Infof("error reading tar: %v", err)
return "", err
return "", nil, err

Check warning on line 91 in pkg/archive/extractor.go

View check run for this annotation

Codecov / codecov/patch

pkg/archive/extractor.go#L91

Added line #L91 was not covered by tests
}

target := filepath.Join(dir, header.Name) //nolint:gosec // Internal usage. No need to check.

// If the target is longer than maxPathLength, we'll use the sha256 of the header name as the filename.
// https://github.com/vmware-tanzu/velero/issues/8434
if len(target) > maxPathLength {
shortSha256Name := fmt.Sprintf("%x", sha256.Sum256([]byte(header.Name))) // sha256 name is 64 characters
longNames[shortSha256Name] = header.Name
target = filepath.Join(dir, shortSha256Name)
}
switch header.Typeflag {
case tar.TypeDir:
err := e.fs.MkdirAll(target, header.FileInfo().Mode())
if err != nil {
e.log.Infof("mkdirall error: %v", err)
return "", err
return "", nil, err

Check warning on line 107 in pkg/archive/extractor.go

View check run for this annotation

Codecov / codecov/patch

pkg/archive/extractor.go#L107

Added line #L107 was not covered by tests
}

case tar.TypeReg:
// make sure we have the directory created
err := e.fs.MkdirAll(filepath.Dir(target), header.FileInfo().Mode())
if err != nil {
e.log.Infof("mkdirall error: %v", err)
return "", err
return "", nil, err

Check warning on line 115 in pkg/archive/extractor.go

View check run for this annotation

Codecov / codecov/patch

pkg/archive/extractor.go#L115

Added line #L115 was not covered by tests
}

// create the file
if err := e.writeFile(target, tarRdr); err != nil {
e.log.Infof("error copying: %v", err)
return "", err
return "", nil, err

Check warning on line 121 in pkg/archive/extractor.go

View check run for this annotation

Codecov / codecov/patch

pkg/archive/extractor.go#L121

Added line #L121 was not covered by tests
}
}
}

return dir, nil
return dir, longNames, nil
}
54 changes: 53 additions & 1 deletion pkg/archive/extractor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ package archive

import (
"archive/tar"
"bytes"
"compress/gzip"
"io"
"os"
"path/filepath"
"strings"
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"

"github.com/vmware-tanzu/velero/pkg/test"
Expand Down Expand Up @@ -75,7 +79,7 @@ func TestUnzipAndExtractBackup(t *testing.T) {
file, err := ext.fs.OpenFile(fileName, os.O_RDWR|os.O_CREATE, 0644)
require.NoError(t, err)

_, err = ext.UnzipAndExtractBackup(file.(io.Reader))
_, _, err = ext.UnzipAndExtractBackup(file.(io.Reader))
if tc.wantErr && (err == nil) {
t.Errorf("%s: wanted error but got nil", tc.name)
}
Expand Down Expand Up @@ -149,3 +153,51 @@ func createRegular(fs filesystem.Interface) (string, error) {

return outName, nil
}

func TestReadBackupWithLongFilenames(t *testing.T) {
log := logrus.New()
fs := test.NewFakeFileSystem()
e := NewExtractor(log, fs)

// Create a tar reader with a file that has a very long name
var buf bytes.Buffer
tw := tar.NewWriter(&buf)

// Create a filename longer than maxPathLength
longFilename := strings.Repeat("a", maxPathLength+10) + ".txt"
content := []byte("test content")

hdr := &tar.Header{
Name: longFilename,
Mode: 0600,
Size: int64(len(content)),
Typeflag: tar.TypeReg,
}

if err := tw.WriteHeader(hdr); err != nil {
t.Fatal(err)
}
if _, err := tw.Write(content); err != nil {
t.Fatal(err)
}
tw.Close()

// Read the backup
dir, longNames, err := e.readBackup(tar.NewReader(&buf))

// Verify results
require.NoError(t, err)
require.NotEmpty(t, dir)
require.NotEmpty(t, longNames)

// Verify that a shortened SHA256 name was created and mapped correctly
found := false
for shortName, originalName := range longNames {
if originalName == longFilename {
found = true
// Verify the short name length is within limits
require.LessOrEqual(t, len(filepath.Join(dir, shortName)), maxPathLength)
}
}
require.True(t, found, "Long filename mapping not found")
}
34 changes: 22 additions & 12 deletions pkg/archive/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func NewParser(log logrus.FieldLogger, fs filesystem.Interface) *Parser {

// Parse reads an extracted backup on the file system and returns
// a structured catalog of the resources and items contained within it.
func (p *Parser) Parse(dir string) (map[string]*ResourceItems, error) {
func (p *Parser) Parse(dir string, longNames map[string]string) (map[string]*ResourceItems, error) {
// ensure top-level "resources" directory exists, and read subdirectories
// of it, where each one is expected to correspond to a resource.
resourcesDir := filepath.Join(dir, velerov1api.ResourcesDir)
Expand All @@ -81,9 +81,8 @@ func (p *Parser) Parse(dir string) (map[string]*ResourceItems, error) {
p.log.Warnf("Ignoring unexpected file %q in directory %q", resourceDir.Name(), strings.TrimPrefix(resourcesDir, dir+"/"))
continue
}

resourceItems := &ResourceItems{
GroupResource: resourceDir.Name(),
GroupResource: itemNameIfHasLongName(resourceDir.Name(), longNames),
ItemsByNamespace: map[string][]string{},
}

Expand All @@ -95,7 +94,7 @@ func (p *Parser) Parse(dir string) (map[string]*ResourceItems, error) {
return nil, errors.Wrapf(err, "error checking for existence of directory %q", strings.TrimPrefix(clusterScopedDir, dir+"/"))
}
if exists {
items, err := p.getResourceItemsForScope(clusterScopedDir, dir)
items, err := p.getResourceItemsForScope(clusterScopedDir, dir, longNames)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -124,7 +123,7 @@ func (p *Parser) Parse(dir string) (map[string]*ResourceItems, error) {
continue
}

items, err := p.getResourceItemsForScope(filepath.Join(namespaceScopedDir, namespaceDir.Name()), dir)
items, err := p.getResourceItemsForScope(filepath.Join(namespaceScopedDir, namespaceDir.Name()), dir, longNames)
if err != nil {
return nil, err
}
Expand All @@ -135,15 +134,15 @@ func (p *Parser) Parse(dir string) (map[string]*ResourceItems, error) {
}
}

resources[resourceDir.Name()] = resourceItems
resources[itemNameIfHasLongName(resourceDir.Name(), longNames)] = resourceItems
}

return resources, nil
}

// getResourceItemsForScope returns the list of items with a namespace or
// cluster-scoped subdirectory for a specific resource.
func (p *Parser) getResourceItemsForScope(dir, archiveRootDir string) ([]string, error) {
func (p *Parser) getResourceItemsForScope(dir, archiveRootDir string, longNames map[string]string) ([]string, error) {
files, err := p.fs.ReadDir(dir)
if err != nil {
return nil, errors.Wrapf(err, "error reading contents of directory %q", strings.TrimPrefix(dir, archiveRootDir+"/"))
Expand All @@ -156,12 +155,22 @@ func (p *Parser) getResourceItemsForScope(dir, archiveRootDir string) ([]string,
continue
}

items = append(items, strings.TrimSuffix(file.Name(), ".json"))
items = append(items, itemNameIfHasLongName(strings.TrimSuffix(file.Name(), ".json"), longNames))
}

return items, nil
}

func itemNameIfHasLongName(itemName string, longNames map[string]string) string {
if longNames == nil {
return itemName
}
if longName, ok := longNames[itemName]; ok {
return longName
}
return itemName
}

// checkAndReadDir is a wrapper around fs.DirExists and fs.ReadDir that does checks
// and returns errors if directory cannot be read.
func (p *Parser) checkAndReadDir(dir string) ([]os.FileInfo, error) {
Expand All @@ -183,7 +192,7 @@ func (p *Parser) checkAndReadDir(dir string) ([]os.FileInfo, error) {

// ParseGroupVersions extracts the versions for each API Group from the backup
// directory names and stores them in a metav1 APIGroup object.
func (p *Parser) ParseGroupVersions(dir string) (map[string]metav1.APIGroup, error) {
func (p *Parser) ParseGroupVersions(dir string, longNames map[string]string) (map[string]metav1.APIGroup, error) {
resourcesDir := filepath.Join(dir, velerov1api.ResourcesDir)

// Get the subdirectories inside the "resources" directory. The subdirectories
Expand All @@ -197,8 +206,9 @@ func (p *Parser) ParseGroupVersions(dir string) (map[string]metav1.APIGroup, err

// Loop through the resource.group directory names.
for _, rgd := range rgDirs {
rgdName := itemNameIfHasLongName(rgd.Name(), longNames)
group := metav1.APIGroup{
Name: extractGroupName(rgd.Name()),
Name: extractGroupName(rgdName),
}

rgdPath := filepath.Join(resourcesDir, rgd.Name())
Expand All @@ -213,7 +223,7 @@ func (p *Parser) ParseGroupVersions(dir string) (map[string]metav1.APIGroup, err
var supportedVersions []metav1.GroupVersionForDiscovery

for _, gvd := range gvDirs {
gvdName := gvd.Name()
gvdName := itemNameIfHasLongName(gvd.Name(), longNames)

// Don't save the namespaces or clusters directories in list of
// supported API Group Versions.
Expand Down Expand Up @@ -241,7 +251,7 @@ func (p *Parser) ParseGroupVersions(dir string) (map[string]metav1.APIGroup, err

group.Versions = supportedVersions

resourceAGs[rgd.Name()] = group
resourceAGs[rgdName] = group
}

return resourceAGs, nil
Expand Down
Loading
Loading