Repository: kubernetes/kubernetes (language: go)

Issue:
Compressed pod log files default to 644 permissions
### What happened?

When the pod logs rotate and get compressed, the gzipped file defaults to the permissions of 644. With the directories set to 755 this allows the logs to be world readable at that point.

This appears to be where this issue is happening at - https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/logs/container_log_manager.go#L422

### What did you expect to happen?

For the compressed logs to either inherit the original log file permissions (600) or for this file permission to be configurable in the kubelet configuration

### How can we reproduce it (as minimally and precisely as possible)?

Generate logs & wait until they compress - https://kubernetes.io/docs/concepts/cluster-administration/logging/#basic-logging-in-kubernetes

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

```console
$ kubectl version
Client Version: v1.30.9
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.30.2
```

</details>


### Cloud provider

<details>

</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
Rocky Linux 9.5
Version = 9.20250121.0.5 (Blue Onyx)
$ uname -a
5.14.0-503.21.1.el9_5.x86_64

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
/sig node


In environments  containers, where logs may need to be accessed by multiple users or processes (such as for debugging or operational monitoring), 0644 ensures the file is readable by any user on the same machine, while still ensuring that only the owner can modify the file. Other  users that are part of the same group or have general access to the system can read the logs without modifying them.

The above reason, i  think 0600 is not given as permission because only owner will have rw permission even read permission is not given to group.
WDYT about 0600 permission why it should be ?
sig-node CI meeting notes:

- Is this permission the same as uncompressed logs?

/assign
Sorry for the delay in response. For my system I see the uncompressed logs as 0600, but once they go through compression they get assigned with 0644 -

-rw-------. 1 root root  9446029 Jan 25 06:59 0.log
-rw-r--r--. 1 root root   842586 Jan 24 11:57 0.log.20250123-145214.gz
-rw-------. 1 root root 10488863 Jan 24 11:57 0.log.20250124-115712
-rw-------. 1 root root    24136 Jan 25 07:22 1.log

Fix patch:
diff --git a/pkg/kubelet/logs/container_log_manager.go b/pkg/kubelet/logs/container_log_manager.go
index fdb6fe0abd3d3..29dffdf3c547a 100644
--- a/pkg/kubelet/logs/container_log_manager.go
+++ b/pkg/kubelet/logs/container_log_manager.go
@@ -79,7 +79,7 @@ func GetAllLogs(log string) ([]string, error) {
 	pattern := fmt.Sprintf("%s.*", log)
 	logs, err := filepath.Glob(pattern)
 	if err != nil {
-		return nil, fmt.Errorf("failed to list all log files with pattern %q: %v", pattern, err)
+		return nil, fmt.Errorf("failed to list all log files with pattern %q: %w", pattern, err)
 	}
 	inuse, _ := filterUnusedLogs(logs)
 	sort.Strings(inuse)
@@ -113,7 +113,7 @@ func UncompressLog(log string) (_ io.ReadCloser, retErr error) {
 	}
 	f, err := os.Open(log)
 	if err != nil {
-		return nil, fmt.Errorf("failed to open log: %v", err)
+		return nil, fmt.Errorf("failed to open log: %w", err)
 	}
 	defer func() {
 		if retErr != nil {
@@ -122,7 +122,7 @@ func UncompressLog(log string) (_ io.ReadCloser, retErr error) {
 	}()
 	r, err := gzip.NewReader(f)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create gzip reader: %v", err)
+		return nil, fmt.Errorf("failed to create gzip reader: %w", err)
 	}
 	return &compressReadCloser{f: f, Reader: r}, nil
 }
@@ -158,7 +158,7 @@ func NewContainerLogManager(runtimeService internalapi.RuntimeService, osInterfa
 	}
 	parsedMaxSize, err := parseMaxSize(maxSize)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse container log max size %q: %v", maxSize, err)
+		return nil, fmt.Errorf("failed to parse container log max size %q: %w", maxSize, err)
 	}
 	// Negative number means to disable container log rotation
 	if parsedMaxSize < 0 {
@@ -205,7 +205,7 @@ func (c *containerLogManager) Clean(ctx context.Context, containerID string) err
 	defer c.mutex.Unlock()
 	resp, err := c.runtimeService.ContainerStatus(ctx, containerID, false)
 	if err != nil {
-		return fmt.Errorf("failed to get container status %q: %v", containerID, err)
+		return fmt.Errorf("failed to get container status %q: %w", containerID, err)
 	}
 	if resp.GetStatus() == nil {
 		return fmt.Errorf("container status is nil for %q", containerID)
@@ -213,12 +213,12 @@ func (c *containerLogManager) Clean(ctx context.Context, containerID string) err
 	pattern := fmt.Sprintf("%s*", resp.GetStatus().GetLogPath())
 	logs, err := c.osInterface.Glob(pattern)
 	if err != nil {
-		return fmt.Errorf("failed to list all log files with pattern %q: %v", pattern, err)
+		return fmt.Errorf("failed to list all log files with pattern %q: %w", pattern, err)
 	}
 
 	for _, l := range logs {
 		if err := c.osInterface.Remove(l); err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf("failed to remove container %q log %q: %v", containerID, l, err)
+			return fmt.Errorf("failed to remove container %q log %q: %w", containerID, l, err)
 		}
 	}
 
@@ -239,7 +239,7 @@ func (c *containerLogManager) rotateLogs(ctx context.Context) error {
 	// TODO(#59998): Use kubelet pod cache.
 	containers, err := c.runtimeService.ListContainers(ctx, &runtimeapi.ContainerFilter{})
 	if err != nil {
-		return fmt.Errorf("failed to list containers: %v", err)
+		return fmt.Errorf("failed to list containers: %w", err)
 	}
 	for _, container := range containers {
 		// Only rotate logs for running containers. Non-running containers won't
@@ -315,17 +315,17 @@ func (c *containerLogManager) rotateLog(ctx context.Context, id, log string) err
 	pattern := fmt.Sprintf("%s.*", log)
 	logs, err := filepath.Glob(pattern)
 	if err != nil {
-		return fmt.Errorf("failed to list all log files with pattern %q: %v", pattern, err)
+		return fmt.Errorf("failed to list all log files with pattern %q: %w", pattern, err)
 	}
 
 	logs, err = c.cleanupUnusedLogs(logs)
 	if err != nil {
-		return fmt.Errorf("failed to cleanup logs: %v", err)
+		return fmt.Errorf("failed to cleanup logs: %w", err)
 	}
 
 	logs, err = c.removeExcessLogs(logs)
 	if err != nil {
-		return fmt.Errorf("failed to remove excess logs: %v", err)
+		return fmt.Errorf("failed to remove excess logs: %w", err)
 	}
 
 	// Compress uncompressed log files.
@@ -334,12 +334,12 @@ func (c *containerLogManager) rotateLog(ctx context.Context, id, log string) err
 			continue
 		}
 		if err := c.compressLog(l); err != nil {
-			return fmt.Errorf("failed to compress log %q: %v", l, err)
+			return fmt.Errorf("failed to compress log %q: %w", l, err)
 		}
 	}
 
 	if err := c.rotateLatestLog(ctx, id, log); err != nil {
-		return fmt.Errorf("failed to rotate log %q: %v", log, err)
+		return fmt.Errorf("failed to rotate log %q: %w", log, err)
 	}
 
 	return nil
@@ -351,7 +351,7 @@ func (c *containerLogManager) cleanupUnusedLogs(logs []string) ([]string, error)
 	inuse, unused := filterUnusedLogs(logs)
 	for _, l := range unused {
 		if err := c.osInterface.Remove(l); err != nil {
-			return nil, fmt.Errorf("failed to remove unused log %q: %v", l, err)
+			return nil, fmt.Errorf("failed to remove unused log %q: %w", l, err)
 		}
 	}
 	return inuse, nil
@@ -404,7 +404,7 @@ func (c *containerLogManager) removeExcessLogs(logs []string) ([]string, error)
 	i := 0
 	for ; i < len(logs)-maxRotatedFiles; i++ {
 		if err := c.osInterface.Remove(logs[i]); err != nil {
-			return nil, fmt.Errorf("failed to remove old log %q: %v", logs[i], err)
+			return nil, fmt.Errorf("failed to remove old log %q: %w", logs[i], err)
 		}
 	}
 	logs = logs[i:]
@@ -413,15 +413,19 @@ func (c *containerLogManager) removeExcessLogs(logs []string) ([]string, error)
 
 // compressLog compresses a log to log.gz with gzip.
 func (c *containerLogManager) compressLog(log string) error {
+	logInfo, err := os.Stat(log)
+	if err != nil {
+		return fmt.Errorf("failed to stat log file: %w", err)
+	}
 	r, err := c.osInterface.Open(log)
 	if err != nil {
-		return fmt.Errorf("failed to open log %q: %v", log, err)
+		return fmt.Errorf("failed to open log %q: %w", log, err)
 	}
 	defer r.Close()
 	tmpLog := log + tmpSuffix
-	f, err := c.osInterface.OpenFile(tmpLog, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+	f, err := c.osInterface.OpenFile(tmpLog, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, logInfo.Mode())
 	if err != nil {
-		return fmt.Errorf("failed to create temporary log %q: %v", tmpLog, err)
+		return fmt.Errorf("failed to create temporary log %q: %w", tmpLog, err)
 	}
 	defer func() {
 		// Best effort cleanup of tmpLog.
@@ -431,19 +435,19 @@ func (c *containerLogManager) compressLog(log string) error {
 	w := gzip.NewWriter(f)
 	defer w.Close()
 	if _, err := io.Copy(w, r); err != nil {
-		return fmt.Errorf("failed to compress %q to %q: %v", log, tmpLog, err)
+		return fmt.Errorf("failed to compress %q to %q: %w", log, tmpLog, err)
 	}
 	// The archive needs to be closed before renaming, otherwise an error will occur on Windows.
 	w.Close()
 	f.Close()
 	compressedLog := log + compressSuffix
 	if err := c.osInterface.Rename(tmpLog, compressedLog); err != nil {
-		return fmt.Errorf("failed to rename %q to %q: %v", tmpLog, compressedLog, err)
+		return fmt.Errorf("failed to rename %q to %q: %w", tmpLog, compressedLog, err)
 	}
 	// Remove old log file.
 	r.Close()
 	if err := c.osInterface.Remove(log); err != nil {
-		return fmt.Errorf("failed to remove log %q after compress: %v", log, err)
+		return fmt.Errorf("failed to remove log %q after compress: %w", log, err)
 	}
 	return nil
 }
@@ -454,7 +458,7 @@ func (c *containerLogManager) rotateLatestLog(ctx context.Context, id, log strin
 	timestamp := c.clock.Now().Format(timestampFormat)
 	rotated := fmt.Sprintf("%s.%s", log, timestamp)
 	if err := c.osInterface.Rename(log, rotated); err != nil {
-		return fmt.Errorf("failed to rotate log %q to %q: %v", log, rotated, err)
+		return fmt.Errorf("failed to rotate log %q to %q: %w", log, rotated, err)
 	}
 	if err := c.runtimeService.ReopenContainerLog(ctx, id); err != nil {
 		// Rename the rotated log back, so that we can try rotating it again
@@ -466,7 +470,7 @@ func (c *containerLogManager) rotateLatestLog(ctx context.Context, id, log strin
 			// log.
 			klog.ErrorS(renameErr, "Failed to rename rotated log", "rotatedLog", rotated, "newLog", log, "containerID", id)
 		}
-		return fmt.Errorf("failed to reopen container log %q: %v", id, err)
+		return fmt.Errorf("failed to reopen container log %q: %w", id, err)
 	}
 	return nil
 }


Test patch:
diff --git a/pkg/kubelet/logs/container_log_manager_test.go b/pkg/kubelet/logs/container_log_manager_test.go
index 561efeeecca33..2bff0656c3c64 100644
--- a/pkg/kubelet/logs/container_log_manager_test.go
+++ b/pkg/kubelet/logs/container_log_manager_test.go
@@ -368,10 +368,18 @@ func TestCompressLog(t *testing.T) {
 	testFile.Close()
 
 	testLog := testFile.Name()
+	testLogInfo, err := os.Stat(testLog)
+	assert.NoError(t, err)
 	c := &containerLogManager{osInterface: container.RealOS{}}
 	require.NoError(t, c.compressLog(testLog))
-	_, err = os.Stat(testLog + compressSuffix)
+	testLogCompressInfo, err := os.Stat(testLog + compressSuffix)
 	assert.NoError(t, err, "log should be compressed")
+	if testLogInfo.Mode() != testLogCompressInfo.Mode() {
+		t.Errorf("compressed and uncompressed test log file modes do not match")
+	}
+	if err := c.compressLog("test-unknown-log"); err == nil {
+		t.Errorf("compressing unknown log should return error")
+	}
 	_, err = os.Stat(testLog + tmpSuffix)
 	assert.Error(t, err, "temporary log should be renamed")
 	_, err = os.Stat(testLog)