Skip to content

Commit

Permalink
[SPARK-39701][CORE][K8S][TESTS] Move withSecretFile to `SparkFunSui…
Browse files Browse the repository at this point in the history
…te` to reuse

### What changes were proposed in this pull request?

This PR aims to move `withSecretFile` to `SparkFunSuite` and reuse it in Kubernetes tests.

### Why are the changes needed?

Currently, K8s unit tests generate a leftover because it doesn't clean up the temporary secret files. By reusing the existing method, we can avoid this
```
$ build/sbt -Pkubernetes "kubernetes/test"
$ git status
On branch master
Your branch is up to date with 'apache/master'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	resource-managers/kubernetes/core/temp-secret/
```

### Does this PR introduce _any_ user-facing change?

No. This is a test-only change.

### How was this patch tested?

Pass the CIs.

Closes apache#37106 from dongjoon-hyun/SPARK-39701.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
  • Loading branch information
dongjoon-hyun committed Jul 6, 2022
1 parent 9983bdb commit 1cf4fe5
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 29 deletions.
11 changes: 1 addition & 10 deletions core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.apache.spark.internal.config._
import org.apache.spark.internal.config.UI._
import org.apache.spark.launcher.SparkLauncher
import org.apache.spark.security.GroupMappingServiceProvider
import org.apache.spark.util.{ResetSystemProperties, SparkConfWithEnv, Utils}
import org.apache.spark.util.{ResetSystemProperties, SparkConfWithEnv}

class DummyGroupMappingServiceProvider extends GroupMappingServiceProvider {

Expand Down Expand Up @@ -513,14 +513,5 @@ class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties {
private def encodeFileAsBase64(secretFile: File) = {
Base64.getEncoder.encodeToString(Files.readAllBytes(secretFile.toPath))
}

private def withSecretFile(contents: String = "test-secret")(f: File => Unit): Unit = {
val secretDir = Utils.createTempDir("temp-secrets")
val secretFile = new File(secretDir, "temp-secret.txt")
Files.write(secretFile.toPath, contents.getBytes(UTF_8))
try f(secretFile) finally {
Utils.deleteRecursively(secretDir)
}
}
}

16 changes: 15 additions & 1 deletion core/src/test/scala/org/apache/spark/SparkFunSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
package org.apache.spark

import java.io.File
import java.nio.file.Path
import java.nio.charset.StandardCharsets.UTF_8
import java.nio.file.{Files, Path}
import java.util.{Locale, TimeZone}

import scala.annotation.tailrec
Expand Down Expand Up @@ -223,6 +224,19 @@ abstract class SparkFunSuite
}
}

/**
* Creates a temporary directory containing a secret file, which is then passed to `f` and
* will be deleted after `f` returns.
*/
protected def withSecretFile(contents: String = "test-secret")(f: File => Unit): Unit = {
val secretDir = Utils.createTempDir("temp-secrets")
val secretFile = new File(secretDir, "temp-secret.txt")
Files.write(secretFile.toPath, contents.getBytes(UTF_8))
try f(secretFile) finally {
Utils.deleteRecursively(secretDir)
}
}

/**
* Adds a log appender and optionally sets a log level to the root logger or the logger with
* the specified name, then executes the specified function, and in the end removes the log
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@
*/
package org.apache.spark.deploy.k8s.features

import java.io.File
import java.nio.charset.StandardCharsets
import java.nio.file.Files

import scala.collection.JavaConverters._

import com.google.common.net.InternetDomainName
Expand Down Expand Up @@ -283,21 +279,20 @@ class BasicExecutorFeatureStepSuite extends SparkFunSuite with BeforeAndAfter {
}

test("Auth secret shouldn't propagate if files are loaded.") {
val secretDir = Utils.createTempDir("temp-secret")
val secretFile = new File(secretDir, "secret-file.txt")
Files.write(secretFile.toPath, "some-secret".getBytes(StandardCharsets.UTF_8))
val conf = baseConf.clone()
.set(config.NETWORK_AUTH_ENABLED, true)
.set(config.AUTH_SECRET_FILE, secretFile.getAbsolutePath)
.set("spark.master", "k8s://127.0.0.1")
val secMgr = new SecurityManager(conf)
secMgr.initializeAuth()
val step = new BasicExecutorFeatureStep(KubernetesTestConf.createExecutorConf(sparkConf = conf),
secMgr, defaultProfile)
withSecretFile("some-secret") { secretFile =>
val conf = baseConf.clone()
.set(config.NETWORK_AUTH_ENABLED, true)
.set(config.AUTH_SECRET_FILE, secretFile.getAbsolutePath)
.set("spark.master", "k8s://127.0.0.1")
val secMgr = new SecurityManager(conf)
secMgr.initializeAuth()
val step = new BasicExecutorFeatureStep(
KubernetesTestConf.createExecutorConf(sparkConf = conf), secMgr, defaultProfile)

val executor = step.configurePod(SparkPod.initialPod())
assert(!KubernetesFeaturesTestUtils.containerHasEnvVar(
executor.container, SecurityManager.ENV_AUTH_SECRET))
val executor = step.configurePod(SparkPod.initialPod())
assert(!KubernetesFeaturesTestUtils.containerHasEnvVar(
executor.container, SecurityManager.ENV_AUTH_SECRET))
}
}

test("SPARK-32661 test executor offheap memory") {
Expand Down

0 comments on commit 1cf4fe5

Please sign in to comment.