Skip to content

Commit

Permalink
[KYUUBI #5004] Fix typo for kubernetes allowed context and namespace …
Browse files Browse the repository at this point in the history
…check

### _Why are the changes needed?_

I met below exception.
```
2023-06-29 00:10:43.174 ERROR org.apache.kyuubi.engine.KubernetesApplicationOperation: Failed to get application with 3c036904-ffef-480b-89e9-2bc6439a6d04, due to Kubernetes context Some(97) is not in the allowed list[ArrayBuffer(97, 116)]
```

The context and namespace are type of `Option[String]` and the allowed lists are type of `Seq[String]`.

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request

Closes #5004 from turboFei/context_check.

Closes #5004

3a3e8c8 [fwang12] ut
5576631 [fwang12] add ut
68f582b [fwang12] fix context and namespace check

Authored-by: fwang12 <[email protected]>
Signed-off-by: fwang12 <[email protected]>
  • Loading branch information
turboFei committed Jun 29, 2023
1 parent 9855802 commit f8ee58f
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,24 @@ class KubernetesApplicationOperation extends ApplicationOperation with Logging {
private var cleanupTerminatedAppInfoTrigger: Cache[String, ApplicationState] = _

private def getOrCreateKubernetesClient(kubernetesInfo: KubernetesInfo): KubernetesClient = {
checkKubernetesInfo(kubernetesInfo)
kubernetesClients.computeIfAbsent(kubernetesInfo, kInfo => buildKubernetesClient(kInfo))
}

// Visible for testing
private[engine] def checkKubernetesInfo(kubernetesInfo: KubernetesInfo): Unit = {
val context = kubernetesInfo.context
val namespace = kubernetesInfo.namespace

if (allowedContexts.nonEmpty && !allowedContexts.contains(context)) {
if (allowedContexts.nonEmpty && context.exists(!allowedContexts.contains(_))) {
throw new KyuubiException(
s"Kubernetes context $context is not in the allowed list[$allowedContexts]")
}

if (allowedNamespaces.nonEmpty && !allowedNamespaces.contains(namespace)) {
if (allowedNamespaces.nonEmpty && namespace.exists(!allowedNamespaces.contains(_))) {
throw new KyuubiException(
s"Kubernetes namespace $namespace is not in the allowed list[$allowedNamespaces]")
}

kubernetesClients.computeIfAbsent(kubernetesInfo, kInfo => buildKubernetesClient(kInfo))
}

private def buildKubernetesClient(kubernetesInfo: KubernetesInfo): KubernetesClient = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You 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 org.apache.kyuubi.engine

import org.apache.kyuubi.{KyuubiException, KyuubiFunSuite}
import org.apache.kyuubi.config.KyuubiConf

class KubernetesApplicationOperationSuite extends KyuubiFunSuite {

test("test check kubernetes info") {
val kyuubiConf = KyuubiConf()
kyuubiConf.set(KyuubiConf.KUBERNETES_CONTEXT_ALLOW_LIST.key, "1,2")
kyuubiConf.set(KyuubiConf.KUBERNETES_NAMESPACE_ALLOW_LIST.key, "ns1,ns2")

val operation = new KubernetesApplicationOperation()
operation.initialize(kyuubiConf)

operation.checkKubernetesInfo(KubernetesInfo(None, None))
operation.checkKubernetesInfo(KubernetesInfo(Some("1"), None))
operation.checkKubernetesInfo(KubernetesInfo(Some("2"), None))
operation.checkKubernetesInfo(KubernetesInfo(Some("1"), Some("ns1")))
operation.checkKubernetesInfo(KubernetesInfo(Some("1"), Some("ns2")))
operation.checkKubernetesInfo(KubernetesInfo(Some("2"), Some("ns1")))
operation.checkKubernetesInfo(KubernetesInfo(Some("2"), Some("ns2")))

intercept[KyuubiException] {
operation.checkKubernetesInfo(KubernetesInfo(Some("3"), Some("ns1")))
}
intercept[KyuubiException] {
operation.checkKubernetesInfo(KubernetesInfo(Some("1"), Some("ns3")))
}
intercept[KyuubiException] {
operation.checkKubernetesInfo(KubernetesInfo(Some("3"), None))
}
intercept[KyuubiException] {
operation.checkKubernetesInfo(KubernetesInfo(None, Some("ns3")))
}

kyuubiConf.unset(KyuubiConf.KUBERNETES_CONTEXT_ALLOW_LIST.key)
operation.checkKubernetesInfo(KubernetesInfo(Some("3"), None))
kyuubiConf.unset(KyuubiConf.KUBERNETES_NAMESPACE_ALLOW_LIST.key)
operation.checkKubernetesInfo(KubernetesInfo(None, Some("ns3")))
}
}

0 comments on commit f8ee58f

Please sign in to comment.