diff --git a/pkg/kubelet/kubelet_server_journal.go b/pkg/kubelet/kubelet_server_journal.go index cd3a02649b2..0159910803e 100644 --- a/pkg/kubelet/kubelet_server_journal.go +++ b/pkg/kubelet/kubelet_server_journal.go @@ -343,7 +343,7 @@ func copyFileLogs(ctx context.Context, w io.Writer, services []string) { } for _, service := range services { - heuristicsCopyFileLogs(ctx, w, service) + heuristicsCopyFileLogs(ctx, w, nodeLogDir, service) } } @@ -352,7 +352,7 @@ func copyFileLogs(ctx context.Context, w io.Writer, services []string) { // /var/log/service.log or // /var/log/service/service.log or // in that order stopping on first success. -func heuristicsCopyFileLogs(ctx context.Context, w io.Writer, service string) { +func heuristicsCopyFileLogs(ctx context.Context, w io.Writer, logDir, service string) { logFileNames := [3]string{ service, fmt.Sprintf("%s.log", service), @@ -361,12 +361,7 @@ func heuristicsCopyFileLogs(ctx context.Context, w io.Writer, service string) { var err error for _, logFileName := range logFileNames { - var logFile string - logFile, err = securejoin.SecureJoin(nodeLogDir, logFileName) - if err != nil { - break - } - err = heuristicsCopyFileLog(ctx, w, logFile) + err = heuristicsCopyFileLog(ctx, w, logDir, logFileName) if err == nil { break } else if errors.Is(err, os.ErrNotExist) { @@ -407,30 +402,6 @@ func newReaderCtx(ctx context.Context, r io.Reader) io.Reader { } } -// heuristicsCopyFileLog returns the contents of the given logFile -func heuristicsCopyFileLog(ctx context.Context, w io.Writer, logFile string) error { - fInfo, err := os.Stat(logFile) - if err != nil { - return err - } - // This is to account for the heuristics where logs for service foo - // could be in /var/log/foo/ - if fInfo.IsDir() { - return os.ErrNotExist - } - - f, err := os.Open(logFile) - if err != nil { - return err - } - defer f.Close() - - if _, err := io.Copy(w, newReaderCtx(ctx, f)); err != nil { - return err - } - return nil -} - func safeServiceName(s string) error { // Max length of a service name is 256 across supported OSes if len(s) > maxServiceLength { diff --git a/pkg/kubelet/kubelet_server_journal_linux.go b/pkg/kubelet/kubelet_server_journal_linux.go index 3e24f3c5fcb..0265d08fccb 100644 --- a/pkg/kubelet/kubelet_server_journal_linux.go +++ b/pkg/kubelet/kubelet_server_journal_linux.go @@ -21,8 +21,13 @@ package kubelet import ( "context" "fmt" + "io" + "os" "os/exec" "strings" + + securejoin "github.com/cyphar/filepath-securejoin" + unix "golang.org/x/sys/unix" ) // getLoggingCmd returns the journalctl cmd and arguments for the given nodeLogQuery and boot. Note that @@ -72,3 +77,32 @@ func checkForNativeLogger(ctx context.Context, service string) bool { // hence we search for it in the list of services known to journalctl return strings.Contains(string(output), service+".service") } + +// heuristicsCopyFileLog returns the contents of the given logFile +func heuristicsCopyFileLog(ctx context.Context, w io.Writer, logDir, logFileName string) error { + f, err := securejoin.OpenInRoot(logDir, logFileName) + if err != nil { + return err + } + // Ignoring errors when closing a file opened read-only doesn't cause data loss + defer func() { _ = f.Close() }() + fInfo, err := f.Stat() + if err != nil { + return err + } + // This is to account for the heuristics where logs for service foo + // could be in /var/log/foo/ + if fInfo.IsDir() { + return os.ErrNotExist + } + rf, err := securejoin.Reopen(f, unix.O_RDONLY) + if err != nil { + return err + } + defer func() { _ = rf.Close() }() + + if _, err := io.Copy(w, newReaderCtx(ctx, rf)); err != nil { + return err + } + return nil +} diff --git a/pkg/kubelet/kubelet_server_journal_nonlinux.go b/pkg/kubelet/kubelet_server_journal_nonlinux.go new file mode 100644 index 00000000000..a53c197a1e7 --- /dev/null +++ b/pkg/kubelet/kubelet_server_journal_nonlinux.go @@ -0,0 +1,56 @@ +//go:build !linux + +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubelet + +import ( + "context" + "io" + "os" + + securejoin "github.com/cyphar/filepath-securejoin" +) + +// heuristicsCopyFileLog returns the contents of the given logFile +func heuristicsCopyFileLog(ctx context.Context, w io.Writer, logDir, logFileName string) error { + logFile, err := securejoin.SecureJoin(logDir, logFileName) + if err != nil { + return err + } + fInfo, err := os.Stat(logFile) + if err != nil { + return err + } + // This is to account for the heuristics where logs for service foo + // could be in /var/log/foo/ + if fInfo.IsDir() { + return os.ErrNotExist + } + + f, err := os.Open(logFile) + if err != nil { + return err + } + // Ignoring errors when closing a file opened read-only doesn't cause data loss + defer func() { _ = f.Close() }() + + if _, err := io.Copy(w, newReaderCtx(ctx, f)); err != nil { + return err + } + return nil +} diff --git a/pkg/kubelet/kubelet_server_journal_others.go b/pkg/kubelet/kubelet_server_journal_others.go index 2f9e0ecb1a5..562b0414409 100644 --- a/pkg/kubelet/kubelet_server_journal_others.go +++ b/pkg/kubelet/kubelet_server_journal_others.go @@ -24,11 +24,11 @@ import ( ) // getLoggingCmd on unsupported operating systems returns the echo command and a warning message (as strings) -func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error) { +func getLoggingCmd(_ *nodeLogQuery, _ []string) (string, []string, error) { return "", []string{}, errors.New("Operating System Not Supported") } // checkForNativeLogger on unsupported operating systems returns false -func checkForNativeLogger(ctx context.Context, service string) bool { +func checkForNativeLogger(_ context.Context, _ string) bool { return false } diff --git a/pkg/kubelet/kubelet_server_journal_test.go b/pkg/kubelet/kubelet_server_journal_test.go index 430a73fc27c..7e8c13c1f84 100644 --- a/pkg/kubelet/kubelet_server_journal_test.go +++ b/pkg/kubelet/kubelet_server_journal_test.go @@ -17,7 +17,11 @@ limitations under the License. package kubelet import ( + "bytes" + "context" "net/url" + "os" + "path/filepath" "reflect" "runtime" "strings" @@ -27,6 +31,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" + "k8s.io/utils/ptr" ) func Test_getLoggingCmd(t *testing.T) { @@ -92,15 +97,15 @@ func Test_newNodeLogQuery(t *testing.T) { {query: url.Values{"untilTime": []string{validTimeValue}}, want: &nodeLogQuery{options: options{UntilTime: &validT}}}, - {query: url.Values{"tailLines": []string{"100"}}, want: &nodeLogQuery{options: options{TailLines: intPtr(100)}}}, + {query: url.Values{"tailLines": []string{"100"}}, want: &nodeLogQuery{options: options{TailLines: ptr.To(100)}}}, {query: url.Values{"tailLines": []string{"foo"}}, wantErr: true}, {query: url.Values{"tailLines": []string{" "}}, wantErr: true}, {query: url.Values{"pattern": []string{"foo"}}, want: &nodeLogQuery{options: options{Pattern: "foo"}}}, {query: url.Values{"boot": []string{""}}, want: nil}, - {query: url.Values{"boot": []string{"0"}}, want: &nodeLogQuery{options: options{Boot: intPtr(0)}}}, - {query: url.Values{"boot": []string{"-23"}}, want: &nodeLogQuery{options: options{Boot: intPtr(-23)}}}, + {query: url.Values{"boot": []string{"0"}}, want: &nodeLogQuery{options: options{Boot: ptr.To(0)}}}, + {query: url.Values{"boot": []string{"-23"}}, want: &nodeLogQuery{options: options{Boot: ptr.To(-23)}}}, {query: url.Values{"boot": []string{"foo"}}, wantErr: true}, {query: url.Values{"boot": []string{" "}}, wantErr: true}, @@ -128,14 +133,15 @@ func Test_newNodeLogQuery(t *testing.T) { func Test_validateServices(t *testing.T) { var ( - service1 = "svc1" - service2 = "svc2" - service3 = "svc.foo" - service4 = "svc_foo" - service5 = "svc@foo" - service6 = "svc:foo" - invalid1 = "svc\n" - invalid2 = "svc.foo\n" + service1 = "svc1" + service2 = "svc2" + serviceDot = "svc.foo" + serviceUnderscore = "svc_foo" + serviceAt = "svc@foo" + serviceColon = "svc:foo" + invalidServiceNewline = "svc\n" + invalidServiceNewlineDot = "svc.foo\n" + invalidServiceSlash = "svc/foo" ) tests := []struct { name string @@ -144,14 +150,15 @@ func Test_validateServices(t *testing.T) { }{ {name: "one service", services: []string{service1}}, {name: "two services", services: []string{service1, service2}}, - {name: "dot service", services: []string{service3}}, - {name: "underscore service", services: []string{service4}}, - {name: "at service", services: []string{service5}}, - {name: "colon service", services: []string{service6}}, - {name: "invalid service new line", services: []string{invalid1}, wantErr: true}, - {name: "invalid service with dot", services: []string{invalid2}, wantErr: true}, + {name: "dot service", services: []string{serviceDot}}, + {name: "underscore service", services: []string{serviceUnderscore}}, + {name: "at service", services: []string{serviceAt}}, + {name: "colon service", services: []string{serviceColon}}, + {name: "invalid service new line", services: []string{invalidServiceNewline}, wantErr: true}, + {name: "invalid service with dot", services: []string{invalidServiceNewlineDot}, wantErr: true}, + {name: "invalid service with slash", services: []string{invalidServiceSlash}, wantErr: true}, {name: "long service", services: []string{strings.Repeat(service1, 100)}, wantErr: true}, - {name: "max number of services", services: []string{service1, service2, service3, service4, service5}, wantErr: true}, + {name: "max number of services", services: []string{service1, service2, serviceDot, serviceUnderscore, serviceAt}, wantErr: true}, } for _, tt := range tests { errs := validateServices(tt.services) @@ -198,10 +205,10 @@ func Test_nodeLogQuery_validate(t *testing.T) { {name: "since until", Services: []string{service1}, options: options{SinceTime: &until, UntilTime: &since}, wantErr: true}, // boot is not supported on Windows. - {name: "boot", Services: []string{service1}, options: options{Boot: intPtr(-1)}, wantErr: runtime.GOOS == "windows"}, - {name: "boot out of range", Services: []string{service1}, options: options{Boot: intPtr(1)}, wantErr: true}, - {name: "tailLines", Services: []string{service1}, options: options{TailLines: intPtr(100)}}, - {name: "tailLines out of range", Services: []string{service1}, options: options{TailLines: intPtr(100000)}}, + {name: "boot", Services: []string{service1}, options: options{Boot: ptr.To(-1)}, wantErr: runtime.GOOS == "windows"}, + {name: "boot out of range", Services: []string{service1}, options: options{Boot: ptr.To(1)}, wantErr: true}, + {name: "tailLines", Services: []string{service1}, options: options{TailLines: ptr.To(100)}}, + {name: "tailLines out of range", Services: []string{service1}, options: options{TailLines: ptr.To(100000)}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -219,6 +226,63 @@ func Test_nodeLogQuery_validate(t *testing.T) { } } -func intPtr(i int) *int { - return &i +func Test_heuristicsCopyFileLogs(t *testing.T) { + ctx := context.TODO() + buf := &bytes.Buffer{} + + dir, err := os.MkdirTemp("", "logs") + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.RemoveAll(dir) }() + + // Check missing logs + heuristicsCopyFileLogs(ctx, buf, dir, "service.log") + if !strings.Contains(buf.String(), "log not found for service.log") { + t.Fail() + } + buf.Reset() + + // Check missing service logs + heuristicsCopyFileLogs(ctx, buf, dir, "service") + if !strings.Contains(buf.String(), "log not found for service") { + t.Fail() + } + buf.Reset() + + // Check explicitly-named files + if err := os.WriteFile(filepath.Join(dir, "service.log"), []byte("valid logs"), 0o444); err != nil { + t.Fatal(err) + } + heuristicsCopyFileLogs(ctx, buf, dir, "service.log") + if buf.String() != "valid logs" { + t.Fail() + } + buf.Reset() + + // Check service logs + heuristicsCopyFileLogs(ctx, buf, dir, "service") + if buf.String() != "valid logs" { + t.Fail() + } + buf.Reset() + + // Check that a directory doesn't cause errors + if err := os.Mkdir(filepath.Join(dir, "service"), 0o755); err != nil { + t.Fatal(err) + } + heuristicsCopyFileLogs(ctx, buf, dir, "service") + if buf.String() != "valid logs" { + t.Fail() + } + buf.Reset() + + // Check that service logs return the first matching file + if err := os.WriteFile(filepath.Join(dir, "service", "service.log"), []byte("error"), 0o444); err != nil { + t.Fatal(err) + } + heuristicsCopyFileLogs(ctx, buf, dir, "service") + if buf.String() != "valid logs" { + t.Fail() + } }