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

internal/pkg/diagnostics - Fix creation of sub-directories in logs/ #2523

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions changelog/fragments/1681942680-fix-diag-zip-file.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
kind: bug-fix
summary: Fix diagnostic zip file handling of sub-directories in logs/.
component: diagnostics
pr: https://github.com/elastic/elastic-agent/pull/2523
2 changes: 1 addition & 1 deletion internal/pkg/diagnostics/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func zipLogs(zw *zip.Writer, ts time.Time) error {

if d.IsDir() {
_, err := zw.CreateHeader(&zip.FileHeader{
Name: "logs" + name + "/",
Name: "logs/" + name + "/",
Method: zip.Deflate,
Modified: ts,
})
Expand Down
56 changes: 56 additions & 0 deletions internal/pkg/diagnostics/dianostics_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package diagnostics

import (
"archive/zip"
"bytes"
"os"
"path/filepath"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
)

func TestZipLogs(t *testing.T) {
// Setup a directory structure of: logs/httpjson/log.ndjson
{
paths.SetTop(t.TempDir())
dir := filepath.Join(paths.Home(), "logs/sub-dir")
require.NoError(t, os.MkdirAll(dir, 0o700))
require.NoError(t, os.WriteFile(filepath.Join(dir, "log.ndjson"), []byte(".\n"), 0o600))
}

// Zip the logs directory.
buf := new(bytes.Buffer)
w := zip.NewWriter(buf)
require.NoError(t, zipLogs(w, time.Now()))
require.NoError(t, w.Close())

type zippedItem struct {
Name string
IsDir bool
}

// Read back the contents.
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
require.NoError(t, err)
var observed []zippedItem
for _, f := range r.File {
observed = append(observed, zippedItem{Name: f.Name, IsDir: f.FileInfo().IsDir()})
}

// Verify the results.
expected := []zippedItem{
{"logs/", true},
Copy link
Member Author

@andrewkroh andrewkroh Apr 20, 2023

Choose a reason for hiding this comment

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

@michalpristas I pulled in the latest code and this test points out a problem with the newly added elastic-agent-{commit} directory in that it is missing a directory entry within the Zip file.

These directory entries may not be required, but IIRC depending on how robust a particular unzip tool used is, this can lead to problems when unzipping. Sometimes a tool will try to write files into a directory that doesn't exist and fail (or worse, silently ignore the problem and not write any of the files under that dir).

The test is passing as is (at least on posix), so this could be followed up in a separate change. Fixed everything I found here.

{"logs/sub-dir/", true},
{"logs/sub-dir/log.ndjson", false},
}
assert.Equal(t, expected, observed)
}