fix (kubernetes-client) : Change upload tarball location to target up…

…load directory Using `/tmp` as upload directory can be problematic for scenarios where `/tmp` is read only. We can directly upload the intermediate upload tarball to the directory where we are uploading the content. In case of uploading a file to a Pod, temporary tarball would be uploaded in the parent directory of that file. In case of uploading a directory to a Pod, temporary tarball would be uploaded in the same directory that's specified for upload path. Signed-off-by: Rohan Kumar <[email protected]>
fabric8io · Dec 27, 2023 · c6679be · c6679be
1 parent c550866
commit c6679be
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 * Fix #5580: [java-generator] Correctly handle defaults for IntOrString types
 * Fix #5584: Fix CRD generation when EnumMap is used
 * Fix #5626: Prevent memory accumulation from informer usage
+* Fix #5527: Unable to transfer file to pod if `/tmp` is read-only
 
 #### Improvements
 * Fix #5429: moved crd generator annotations to generator-annotations instead of crd-generator-api. Using generator-annotations introduces no transitive dependencies.

diff --git a/...-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/uploadable/PodUpload.java b/...-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/uploadable/PodUpload.java
@@ -52,10 +52,10 @@ public static boolean upload(PodOperationsImpl operation, Path pathToUpload)
       throws IOException {
 
     if (Utils.isNotNullOrEmpty(operation.getContext().getFile()) && pathToUpload.toFile().isFile()) {
-      return uploadTar(operation, getDirectoryFromFile(operation),
+      return uploadTar(operation, getDirectoryFromFile(operation.getContext().getFile()),
           tar -> addFileToTar(new File(operation.getContext().getFile()).getName(), pathToUpload.toFile(), tar));
     } else if (Utils.isNotNullOrEmpty(operation.getContext().getDir()) && pathToUpload.toFile().isDirectory()) {
-      return uploadTar(operation, operation.getContext().getDir(), tar -> {
+      return uploadTar(operation, ensureEndsWithSlash(operation.getContext().getDir()), tar -> {
         for (File file : pathToUpload.toFile().listFiles()) {
           addFileToTar(file.getName(), file, tar);
         }
@@ -64,10 +64,9 @@ public static boolean upload(PodOperationsImpl operation, Path pathToUpload)
     throw new IllegalArgumentException("Provided arguments are not valid (file, directory, path)");
   }
 
-  private static String getDirectoryFromFile(PodOperationsImpl operation) {
-    String file = operation.getContext().getFile();
+  private static String getDirectoryFromFile(String file) {
     String directoryTrimmedFromFilePath = file.substring(0, file.lastIndexOf('/'));
-    return directoryTrimmedFromFilePath.isEmpty() ? "/" : directoryTrimmedFromFilePath;
+    return ensureEndsWithSlash(directoryTrimmedFromFilePath.isEmpty() ? "/" : directoryTrimmedFromFilePath);
   }
 
   private interface UploadProcessor<T extends OutputStream> {
@@ -136,7 +135,7 @@ private static boolean uploadTar(PodOperationsImpl operation, String directory,
       UploadProcessor<TarArchiveOutputStream> processor)
       throws IOException {
 
-    String fileName = String.format("/tmp/fabric8-%s.tar", UUID.randomUUID());
+    String fileName = String.format("%sfabric8-%s.tar", directory, UUID.randomUUID());
 
     boolean uploaded = upload(operation, fileName, os -> {
       try (final TarArchiveOutputStream tar = new TarArchiveOutputStream(os)) {
@@ -188,10 +187,12 @@ private static void addFileToTar(String fileName, File file, TarArchiveOutputStr
   }
 
   static String createExecCommandForUpload(String file) {
-    String directoryTrimmedFromFilePath = file.substring(0, file.lastIndexOf('/'));
-    final String directory = directoryTrimmedFromFilePath.isEmpty() ? "/" : directoryTrimmedFromFilePath;
     return String.format(
-        "mkdir -p %s && cat - > %s", shellQuote(directory), shellQuote(file));
+        "mkdir -p %s && cat - > %s", shellQuote(getDirectoryFromFile(file)), shellQuote(file));
+  }
+
+  private static String ensureEndsWithSlash(String path) {
+    return path.endsWith("/") ? path : (path + "/");
   }
 
 }
diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/behavior/UploadTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/behavior/UploadTest.java
@@ -135,7 +135,7 @@ void uploadFailureDeletesTemp() {
         .extracting(StandardWebSocketBuilder::asHttpRequest)
         .extracting(StandardHttpRequest::uri)
         .extracting(URI::getQuery).asString()
-        .contains("command=rm /tmp/fabric8-");
+        .contains("command=rm /target/fabric8-");
   }
 
   @Nested
@@ -213,7 +213,7 @@ void createsTempDirectoryAndPipesFileInServer() {
             .extracting(StandardWebSocketBuilder::asHttpRequest)
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
-            .contains("command=mkdir -p '/tmp' && cat - > '/tmp/fabric8-");
+            .contains("command=mkdir -p '/target/' && cat - > '/target/fabric8-");
       }
 
       @Test
@@ -229,7 +229,7 @@ void verifiesUploadedTarSize() {
             .extracting(StandardWebSocketBuilder::asHttpRequest)
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
-            .contains("command=wc -c < '/tmp/fabric8-");
+            .contains("command=wc -c < '/target/fabric8-");
       }
 
       @Test
@@ -246,7 +246,7 @@ void extractsTar() {
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
             .matches(
-                ".+command=mkdir -p '/target-dir'; tar -C '/target-dir' -xmf /tmp/fabric8-.+\\.tar; e=\\$\\?; rm /tmp/fabric8-.+");
+                ".+command=mkdir -p '/target-dir/'; tar -C '/target-dir/' -xmf /target-dir/fabric8-.+\\.tar; e=\\$\\?; rm /target-dir/fabric8-.+");
       }
 
       @Nested
@@ -272,7 +272,7 @@ void validTarArchive() throws Exception {
           final byte[] tarBytes = new byte[sendCaptor.getValue().remaining() - 1];
           System.arraycopy(sendCaptor.getValue().array(), 1, tarBytes, 0, tarBytes.length);
           final TarArchiveInputStream tar = new TarArchiveInputStream(new ByteArrayInputStream(tarBytes));
-          assertThat(tar.getNextTarEntry())
+          assertThat(tar.getNextEntry())
               .hasFieldOrPropertyWithValue("name", "file-name.txt")
               .hasFieldOrPropertyWithValue("size", toUpload.toFile().length());
         }
@@ -292,7 +292,7 @@ void longFileNamesSupported() throws Exception {
           final byte[] tarBytes = new byte[sendCaptor.getValue().remaining() - 1];
           System.arraycopy(sendCaptor.getValue().array(), 1, tarBytes, 0, tarBytes.length);
           final TarArchiveInputStream tar = new TarArchiveInputStream(new ByteArrayInputStream(tarBytes));
-          assertThat(tar.getNextTarEntry())
+          assertThat(tar.getNextEntry())
               .hasFieldOrPropertyWithValue("name", longFileName);
         }
 
@@ -310,7 +310,7 @@ void bigNumbersSupported(@TempDir Path tempDir) throws Exception {
           final byte[] tarBytes = new byte[sendCaptor.getValue().remaining() - 1];
           System.arraycopy(sendCaptor.getValue().array(), 1, tarBytes, 0, tarBytes.length);
           final TarArchiveInputStream tar = new TarArchiveInputStream(new ByteArrayInputStream(tarBytes));
-          assertThat(tar.getNextTarEntry())
+          assertThat(tar.getNextEntry())
               .hasFieldOrPropertyWithValue("name", "file-name.txt")
               .hasFieldOrPropertyWithValue("lastModifiedTime", FileTime.fromMillis(9999999999999L));
         }
@@ -351,7 +351,7 @@ void createsTempDirectoryAndPipesDirInServer() {
             .extracting(StandardWebSocketBuilder::asHttpRequest)
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
-            .contains("command=mkdir -p '/tmp' && cat - > '/tmp/fabric8-");
+            .contains("command=mkdir -p '/target/location/' && cat - > '/target/location/fabric8-");
       }
 
       @Test
@@ -367,7 +367,7 @@ void verifiesUploadedTarSize() {
             .extracting(StandardWebSocketBuilder::asHttpRequest)
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
-            .contains("command=wc -c < '/tmp/fabric8-");
+            .contains("command=wc -c < '/target/location/fabric8-");
       }
 
       @Test
@@ -384,7 +384,7 @@ void extractsTar() {
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
             .matches(
-                ".+command=mkdir -p '/target-dir/location'; tar -C '/target-dir/location' -xmf /tmp/fabric8-.+\\.tar; e=\\$\\?; rm /tmp/fabric8-.+");
+                ".+command=mkdir -p '/target-dir/location/'; tar -C '/target-dir/location/' -xmf /target-dir/location/fabric8-.+\\.tar; e=\\$\\?; rm /target-dir/location/fabric8-.+");
       }
 
     }
@@ -423,7 +423,7 @@ void createsTempDirectoryAndPipesFileInServer() {
             .extracting(StandardWebSocketBuilder::asHttpRequest)
             .extracting(StandardHttpRequest::uri)
             .extracting(URI::getQuery).asString()
-            .contains("command=mkdir -p '/target' && cat - > '/target/location");
+            .contains("command=mkdir -p '/target/' && cat - > '/target/location");
       }
 
       @Test

diff --git a/...ent/src/test/java/io/fabric8/kubernetes/client/dsl/internal/uploadable/PodUploadTest.java b/...ent/src/test/java/io/fabric8/kubernetes/client/dsl/internal/uploadable/PodUploadTest.java
@@ -57,7 +57,7 @@ void withNormalFile_shouldCreateValidExecCommandForUpload() {
       // When
       String result = PodUpload.createExecCommandForUpload("/tmp/foo/cp.log");
       // Then
-      assertThat(result).isEqualTo("mkdir -p '/tmp/foo' && cat - > '/tmp/foo/cp.log'");
+      assertThat(result).isEqualTo("mkdir -p '/tmp/foo/' && cat - > '/tmp/foo/cp.log'");
     }
 
     //
@@ -66,15 +66,15 @@ void withSingleQuoteInPath() {
       // When
       String result = PodUpload.createExecCommandForUpload("/tmp/fo'o/cp.log");
       // Then
-      assertThat(result).isEqualTo("mkdir -p '/tmp/fo\'\\'\'o' && cat - > '/tmp/fo\'\\'\'o/cp.log'");
+      assertThat(result).isEqualTo("mkdir -p '/tmp/fo\'\\'\'o/' && cat - > '/tmp/fo\'\\'\'o/cp.log'");
     }
 
     @Test
     void withMultipleSingleQuotesInPath() {
       // When
       String result = PodUpload.createExecCommandForUpload("/tmp/f'o'o/c'p.log");
       // Then
-      assertThat(result).isEqualTo("mkdir -p '/tmp/f\'\\'\'o\'\\'\'o' && cat - > '/tmp/f\'\\'\'o\'\\'\'o/c\'\\'\'p.log'");
+      assertThat(result).isEqualTo("mkdir -p '/tmp/f\'\\'\'o\'\\'\'o/' && cat - > '/tmp/f\'\\'\'o\'\\'\'o/c\'\\'\'p.log'");
     }
   }