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

Copy collector enhanced #237

Merged
merged 26 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f77f09f
Copy collector enhanced
manavellamnimble Aug 5, 2020
07bc145
Recursive (un)tar and redact
manavellamnimble Aug 11, 2020
829f311
Error Handling
manavellamnimble Aug 12, 2020
dba0209
Comments added
manavellamnimble Aug 12, 2020
5bfdffd
Merge pull request #1 from manavellamnimble/Copy_tar
manavellamnimble Aug 12, 2020
d5ab4e3
New command and Tar function
manavellamnimble Aug 14, 2020
03c3f07
New command and Tar function
manavellamnimble Aug 14, 2020
85976fc
Merge pull request #2 from manavellamnimble/Copy_tar
manavellamnimble Aug 14, 2020
425344a
Dirs Included in tar
manavellamnimble Aug 14, 2020
9750a79
Merge pull request #3 from manavellamnimble/Copy_tar
manavellamnimble Aug 14, 2020
6cdc808
Dirs Included in tar
manavellamnimble Aug 14, 2020
eec86f5
Merge pull request #4 from manavellamnimble/Copy_tar
manavellamnimble Aug 14, 2020
3a8126e
tar.gz and .tgz included
manavellamnimble Aug 16, 2020
2c10570
Merge pull request #5 from manavellamnimble/Copy_tar
manavellamnimble Aug 16, 2020
ffc8154
tar.gz and .tgz included
manavellamnimble Aug 16, 2020
d59e311
Merge pull request #6 from manavellamnimble/Copy_tar
manavellamnimble Aug 16, 2020
257e246
tar.gz and .tgz included
manavellamnimble Aug 17, 2020
340e0d4
Merge pull request #7 from manavellamnimble/Copy_tar
manavellamnimble Aug 17, 2020
ca6e3e4
Merge pull request #8 from manavellamnimble/Copy_tar
manavellamnimble Aug 18, 2020
af004be
tar.gz and .tgz included
manavellamnimble Aug 18, 2020
0225123
Copy into folder
manavellamnimble Aug 19, 2020
880f435
Merge pull request #9 from manavellamnimble/Copy_tar
manavellamnimble Aug 19, 2020
58c6229
Copy into folder
manavellamnimble Aug 19, 2020
f73aff0
Merge pull request #10 from manavellamnimble/Copy_tar
manavellamnimble Aug 19, 2020
b4a6a86
command modified
manavellamnimble Aug 21, 2020
6c222eb
Merge pull request #11 from manavellamnimble/Copy_tar
manavellamnimble Aug 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions pkg/collect/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"k8s.io/client-go/tools/remotecommand"
)

//Copy function gets a file or folder from a container specified in the specs.
func Copy(c *Collector, copyCollector *troubleshootv1beta1.Copy) (map[string][]byte, error) {
client, err := kubernetes.NewForConfig(c.ClientConfig)
if err != nil {
Expand Down Expand Up @@ -47,7 +48,7 @@ func Copy(c *Collector, copyCollector *troubleshootv1beta1.Copy) (map[string][]b
}

for k, v := range files {
copyOutput[filepath.Join(bundlePath, k)] = v
copyOutput[filepath.Join(bundlePath, filepath.Dir(copyCollector.ContainerPath), k)] = v
}
}
}
Expand All @@ -60,9 +61,7 @@ func copyFiles(c *Collector, client *kubernetes.Clientset, pod corev1.Pod, copyC
if copyCollector.ContainerName != "" {
container = copyCollector.ContainerName
}

command := []string{"cat", copyCollector.ContainerPath}

command := []string{"sh", "-c", fmt.Sprintf("tar -C %s -cf - %s | cat ", filepath.Dir(copyCollector.ContainerPath), filepath.Base(copyCollector.ContainerPath))}
Copy link
Member

Choose a reason for hiding this comment

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

Why does it need to pipe into cat? Can the pipe be removed and then sh is not needed, so it's just the tar command by itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Dimitriy! In some cases, depending on the version of tar installed in the container, we may get the error:
tar: Refusing to write archive contents to terminal (missing -f option?)
tar: Error is not recoverable: exiting now

Talking about this matter with Andrew and Ethan we came to the conclusion that piping it would be the best way to avoid the error.

Copy link
Member

Choose a reason for hiding this comment

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

@manavellamnimble can you add a comment in the code explaining this. I can see someone changing this and breaking things inadvertently.

req := client.CoreV1().RESTClient().Post().Resource("pods").Name(pod.Name).Namespace(pod.Namespace).SubResource("exec")
scheme := runtime.NewScheme()
if err := corev1.AddToScheme(scheme); err != nil {
Expand Down Expand Up @@ -110,7 +109,7 @@ func copyFiles(c *Collector, client *kubernetes.Clientset, pod corev1.Pod, copyC
}

return map[string][]byte{
copyCollector.ContainerPath: output.Bytes(),
filepath.Base(copyCollector.ContainerPath) + ".tar": output.Bytes(),
}, nil
}

Expand Down
87 changes: 84 additions & 3 deletions pkg/collect/redact.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,101 @@
package collect

import (
"archive/tar"
"bytes"
"encoding/binary"
"io"
"path/filepath"

troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1"
"github.com/replicatedhq/troubleshoot/pkg/redact"
)

func redactMap(input map[string][]byte, additionalRedactors []*troubleshootv1beta1.Redact) (map[string][]byte, error) {
result := make(map[string][]byte)
for k, v := range input {
if v != nil {
redacted, err := redact.Redact(v, k, additionalRedactors)
if v == nil {
continue
}
//If the file is a .tar file, it must not be redacted. Instead it is decompressed and each file inside the
//tar is decompressed, redacted and compressed back into the tar.
if filepath.Ext(k) == ".tar" {
tarFile := bytes.NewBuffer(v)
unRedacted, tarHeaders, err := untarFile(tarFile)
if err != nil {
return nil, err
}
redacted, err := redactMap(unRedacted, additionalRedactors)
if err != nil {
return nil, err
}
result[k], err = tarFiles(redacted, tarHeaders)
if err != nil {
return nil, err
}
result[k] = redacted
//Content of the tar file was redacted. Continue to next file.
continue
}
redacted, err := redact.Redact(v, k, additionalRedactors)
Copy link
Member

Choose a reason for hiding this comment

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

This can corrupt tgz and tar.gz files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Only when the file is inside the folder. I suggested a possible upgrade to the code to decompress .tgz and .tar.gz files too. If you think it would be useful I can work in this functionality!

Copy link
Member

Choose a reason for hiding this comment

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

we should address this in a separate pr if we choose to

if err != nil {
return nil, err
}
result[k] = redacted
}
return result, nil
}

func tarFiles(tarContent map[string][]byte, tarHeaders map[string]*tar.Header) ([]byte, error) {
buff := new(bytes.Buffer)
tw := tar.NewWriter(buff)
Copy link
Member

Choose a reason for hiding this comment

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

can you defer tw.Close() here to make sure its closed even on an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, definitely. It is way more robust in the way you suggest

defer tw.Close()
for p, f := range tarContent {
if tarHeaders[p].FileInfo().IsDir() {
err := tw.WriteHeader(tarHeaders[p])
if err != nil {
return nil, err
}
continue
}
//File size must be recalculated in case the redactor added some bytes when redacting.
tarHeaders[p].Size = int64(binary.Size(f))
err := tw.WriteHeader(tarHeaders[p])
if err != nil {
return nil, err
}
_, err = tw.Write(f)
if err != nil {
return nil, err
}
}
err := tw.Close()
emosbaugh marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
return buff.Bytes(), nil

}

func untarFile(tarFile *bytes.Buffer) (map[string][]byte, map[string]*tar.Header, error) {
tarReader := tar.NewReader(tarFile)
Copy link
Member

Choose a reason for hiding this comment

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

defer close

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you refering to the tar reader? I think it has not a Close() function. I've used defer for the tarWriter

tarHeaders := make(map[string]*tar.Header)
tarContent := make(map[string][]byte)
for {
header, err := tarReader.Next()
if err != nil {
if err != io.EOF {
return nil, nil, err
}
break
}
file := new(bytes.Buffer)
_, err = io.Copy(file, tarReader)
if err != nil {
return nil, nil, err
}
tarContent[header.Name] = file.Bytes()
tarHeaders[header.Name] = header

}
return tarContent, tarHeaders, nil
}