This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-management.git
The following commit(s) were added to refs/heads/main by this push:
new 876507e8 Use precompiled regex patterns for name normalisation (#1002)
876507e8 is described below
commit 876507e8713d1c15a9d4ee196c182e9437d5fea0
Author: akashchamp <[email protected]>
AuthorDate: Fri Oct 9 00:32:55 2026 +0530
Use precompiled regex patterns for name normalisation (#1002)
Motivation:
String.replaceAll compiles its regex on every call. The Kubernetes
lease/rolling-update name normalisation and the bootstrap host matching
call it repeatedly with constant patterns.
Modification:
Hoist the remaining regexes into precompiled scala.util.matching.Regex
vals. Replace the fixed-character replaceAll calls (" ", "_", ".",
and square brackets) with plain String.replace.
Result:
Same output, without recompiling the patterns on every call.
Tests:
Not run locally (sbt is not available on the authoring machine); relying on
CI.
The new specs pin the normalised output of the lease path,
DNS-1039 and resource name helpers.
References:
Fixes #852
---
.../lease/kubernetes/AbstractKubernetesLease.scala | 9 ++--
.../internal/AbstractKubernetesApiImpl.scala | 8 ++++
.../kubernetes/internal/KubernetesApiImpl.scala | 4 +-
.../internal/NativeKubernetesApiImpl.scala | 4 +-
.../lease/kubernetes/KubernetesApiSpec.scala | 5 +++
.../lease/kubernetes/NativeKubernetesApiSpec.scala | 5 +++
.../bootstrap/ClusterBootstrapSettings.scala | 2 +-
.../cluster/bootstrap/SelfAwareJoinDecider.scala | 2 +-
.../rollingupdate/kubernetes/KubernetesApi.scala | 11 ++---
.../kubernetes/KubernetesApiImpl.scala | 4 +-
.../kubernetes/MakeDNS1039CompatibleSpec.scala | 51 ++++++++++++++++++++++
11 files changed, 84 insertions(+), 21 deletions(-)
diff --git
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/AbstractKubernetesLease.scala
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/AbstractKubernetesLease.scala
index 2486b5c1..df2bf9f8 100644
---
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/AbstractKubernetesLease.scala
+++
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/AbstractKubernetesLease.scala
@@ -38,6 +38,7 @@ import org.slf4j.LoggerFactory
object AbstractKubernetesLease {
val configPath = "pekko.coordination.lease.kubernetes"
private val leaseCounter = new AtomicInteger(1)
+ private val InvalidDNS1039Chars = "[^-a-z0-9]".r
// Base32 alphabet (RFC 4648 ยง6), **lowercased** so every character is a
valid DNS 1039 label
// character. '=' padding is intentionally omitted: we stream full 5-bit
groups and emit one
@@ -97,11 +98,9 @@ object AbstractKubernetesLease {
* If hashLength >= maxLength the result consists entirely of the first
maxLength hash characters.
*/
private[kubernetes] def makeDNS1039Compatible(name: String, maxLength: Int =
63, hashLength: Int = 0): String = {
- val normalized =
- Normalizer.normalize(name, Normalizer.Form.NFKD)
- .toLowerCase(Locale.ROOT)
- .replaceAll("[_.]", "-")
- .replaceAll("[^-a-z0-9]", "")
+ val lower = Normalizer.normalize(name,
Normalizer.Form.NFKD).toLowerCase(Locale.ROOT)
+ val withHyphens = lower.replace('_', '-').replace('.', '-')
+ val normalized = InvalidDNS1039Chars.replaceAllIn(withHyphens, "")
if (normalized.length <= maxLength || hashLength <= 0) {
trim(truncateToLength(normalized, maxLength), List('-'))
} else {
diff --git
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/AbstractKubernetesApiImpl.scala
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/AbstractKubernetesApiImpl.scala
index 37213eb2..2e778f5e 100644
---
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/AbstractKubernetesApiImpl.scala
+++
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/AbstractKubernetesApiImpl.scala
@@ -33,6 +33,14 @@ import java.nio.file.{ Files, Paths }
import javax.net.ssl.SSLContext
import scala.concurrent.{ ExecutionContext, Future, Promise }
import scala.util.control.NonFatal
+import scala.util.matching.Regex
+
+/**
+ * INTERNAL API
+ */
+@InternalApi private[kubernetes] object AbstractKubernetesApiImpl {
+ val InvalidLeaseNameChars: Regex = "[^\\d\\w\\-\\.]".r
+}
/**
* Could be shared between leases:
https://github.com/akka/akka-management/issues/680
diff --git
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/KubernetesApiImpl.scala
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/KubernetesApiImpl.scala
index 6ded7b8e..d9a127b5 100644
---
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/KubernetesApiImpl.scala
+++
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/KubernetesApiImpl.scala
@@ -134,9 +134,7 @@ PUTs must contain resourceVersions. Response:
override def pathForLease(name: String): Future[Uri.Path] = {
namespace.map { ns =>
Uri.Path.Empty / "apis" / "pekko.apache.org" / "v1" / "namespaces" / ns
/ "leases" /
- name
- .replaceAll("[^\\d\\w\\-\\.]", "")
- .toLowerCase(Locale.ROOT)
+ AbstractKubernetesApiImpl.InvalidLeaseNameChars.replaceAllIn(name,
"").toLowerCase(Locale.ROOT)
}(ExecutionContext.parasitic)
}
diff --git
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/NativeKubernetesApiImpl.scala
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/NativeKubernetesApiImpl.scala
index d6e2a2f7..20a99529 100644
---
a/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/NativeKubernetesApiImpl.scala
+++
b/lease-kubernetes/src/main/scala/org/apache/pekko/coordination/lease/kubernetes/internal/NativeKubernetesApiImpl.scala
@@ -144,9 +144,7 @@ object NativeKubernetesApiImpl {
override def pathForLease(name: String): Future[Uri.Path] = {
namespace.map { ns =>
Uri.Path.Empty / "apis" / "coordination.k8s.io" / "v1" / "namespaces" /
ns / "leases" /
- name
- .replaceAll("[^\\d\\w\\-\\.]", "")
- .toLowerCase(Locale.ROOT)
+ AbstractKubernetesApiImpl.InvalidLeaseNameChars.replaceAllIn(name,
"").toLowerCase(Locale.ROOT)
}(ExecutionContext.parasitic)
}
diff --git
a/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/KubernetesApiSpec.scala
b/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/KubernetesApiSpec.scala
index 92e85d10..c7bbe383 100644
---
a/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/KubernetesApiSpec.scala
+++
b/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/KubernetesApiSpec.scala
@@ -208,6 +208,11 @@ class KubernetesApiSpec
response shouldEqual Left(LeaseResource(Some(conflictedOwner),
updatedVersion, timestamp))
}
+ "strip characters that are not allowed in a resource name from the lease
path" in {
+ underTest.pathForLease("My_Lease.1@host:7355/x").futureValue.toString
shouldEqual
+ "/apis/pekko.apache.org/v1/namespaces/lease/leases/my_lease.1host7355x"
+ }
+
"remove lease via DELETE" in {
val lease = "lease-1"
stubFor(
diff --git
a/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/NativeKubernetesApiSpec.scala
b/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/NativeKubernetesApiSpec.scala
index 9fd58677..7b952a29 100644
---
a/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/NativeKubernetesApiSpec.scala
+++
b/lease-kubernetes/src/test/scala/org/apache/pekko/coordination/lease/kubernetes/NativeKubernetesApiSpec.scala
@@ -189,6 +189,11 @@ class NativeKubernetesApiSpec
fromRFC3339MicroString(timestamp)))
}
+ "strip characters that are not allowed in a resource name from the lease
path" in {
+ underTest.pathForLease("My_Lease.1@host:7355/x").futureValue.toString
shouldEqual
+
"/apis/coordination.k8s.io/v1/namespaces/lease/leases/my_lease.1host7355x"
+ }
+
"remove lease via DELETE" in {
val lease = "lease-1"
stubFor(
diff --git
a/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/ClusterBootstrapSettings.scala
b/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/ClusterBootstrapSettings.scala
index 1ffc49c8..9e8f2299 100644
---
a/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/ClusterBootstrapSettings.scala
+++
b/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/ClusterBootstrapSettings.scala
@@ -61,7 +61,7 @@ final class ClusterBootstrapSettings(config: Config, log:
LoggingAdapter) {
def effectiveName(system: ActorSystem): String =
discoveryConfig.optDefinedValue("effective-name").getOrElse {
val service =
-
serviceName.getOrElse(system.name.toLowerCase(Locale.ROOT).replaceAll(" ",
"-").replace("_", "-"))
+ serviceName.getOrElse(system.name.toLowerCase(Locale.ROOT).replace("
", "-").replace("_", "-"))
val namespace = serviceNamespace match {
case Some(ns) => s".$ns"
diff --git
a/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/SelfAwareJoinDecider.scala
b/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/SelfAwareJoinDecider.scala
index 4ae8d4de..7d301127 100644
---
a/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/SelfAwareJoinDecider.scala
+++
b/management-cluster-bootstrap/src/main/scala/org/apache/pekko/management/cluster/bootstrap/SelfAwareJoinDecider.scala
@@ -85,7 +85,7 @@ import scala.concurrent.duration._
* Checks for both host name and IP address for discovery mechanisms that
return both.
*/
protected def hostMatches(host: String, target: ResolvedTarget): Boolean = {
- val hostWithoutBracket = host.replaceAll("[\\[\\]]", "")
+ val hostWithoutBracket = host.replace("[", "").replace("]", "")
host == target.host || hostWithoutBracket == target.host ||
target.address
.map(_.getHostAddress)
diff --git
a/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApi.scala
b/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApi.scala
index 3a3ae97c..5eddc8d0 100644
---
a/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApi.scala
+++
b/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApi.scala
@@ -79,6 +79,9 @@ private[pekko] final case class PodCost(podName: String,
cost: Int, address: Str
*/
@InternalApi private[pekko] object KubernetesApi {
+ private val InvalidDNS1039Chars = "[^-a-z0-9]".r
+ private[kubernetes] val InvalidResourceNameChars = "[^\\d\\w\\-\\.]".r
+
/**
* Removes from the leading and trailing positions the specified characters.
*/
@@ -91,11 +94,9 @@ private[pekko] final case class PodCost(podName: String,
cost: Int, address: Str
* Validates the resulting name to be at most 63 characters, otherwise
throws `IllegalArgumentException`.
*/
def makeDNS1039Compatible(name: String): String = {
- val normalized =
- Normalizer.normalize(name, Normalizer.Form.NFKD)
- .toLowerCase(Locale.ROOT)
- .replaceAll("[_.]", "-")
- .replaceAll("[^-a-z0-9]", "")
+ val lower = Normalizer.normalize(name,
Normalizer.Form.NFKD).toLowerCase(Locale.ROOT)
+ val withHyphens = lower.replace('_', '-').replace('.', '-')
+ val normalized = InvalidDNS1039Chars.replaceAllIn(withHyphens, "")
if (normalized.length > 63)
throw new IllegalArgumentException(
s"Resource name [$normalized] is too long. At most 63 characters are
accepted. " +
diff --git
a/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApiImpl.scala
b/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApiImpl.scala
index 53d408f1..5017497a 100644
---
a/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApiImpl.scala
+++
b/rolling-update-kubernetes/src/main/scala/org/apache/pekko/rollingupdate/kubernetes/KubernetesApiImpl.scala
@@ -254,9 +254,7 @@ PUTs must contain resourceVersions. Response:
private def pathForPodCostResource(crName: String): Uri.Path =
Uri.Path.Empty / "apis" / "pekko.apache.org" / "v1" / "namespaces" /
namespace / "podcosts" /
- crName
- .replaceAll("[^\\d\\w\\-\\.]", "")
- .toLowerCase(Locale.ROOT)
+ KubernetesApi.InvalidResourceNameChars.replaceAllIn(crName,
"").toLowerCase(Locale.ROOT)
private def requestForPath(
path: Uri.Path,
diff --git
a/rolling-update-kubernetes/src/test/scala/org/apache/pekko/rollingupdate/kubernetes/MakeDNS1039CompatibleSpec.scala
b/rolling-update-kubernetes/src/test/scala/org/apache/pekko/rollingupdate/kubernetes/MakeDNS1039CompatibleSpec.scala
new file mode 100644
index 00000000..426dddc5
--- /dev/null
+++
b/rolling-update-kubernetes/src/test/scala/org/apache/pekko/rollingupdate/kubernetes/MakeDNS1039CompatibleSpec.scala
@@ -0,0 +1,51 @@
+/*
+ * 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.pekko.rollingupdate.kubernetes
+
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+class MakeDNS1039CompatibleSpec extends AnyWordSpec with Matchers {
+
+ "KubernetesApi.makeDNS1039Compatible" should {
+
+ "leave a valid pod name unchanged" in {
+ KubernetesApi.makeDNS1039Compatible("my-pod-1") shouldEqual "my-pod-1"
+ }
+
+ "convert underscores and dots to hyphens" in {
+ KubernetesApi.makeDNS1039Compatible("my.pod_name") shouldEqual
"my-pod-name"
+ }
+
+ "lowercase and drop characters that are not allowed" in {
+ KubernetesApi.makeDNS1039Compatible("My@Pod!Name") shouldEqual
"mypodname"
+ }
+
+ "strip accents after unicode normalization" in {
+ KubernetesApi.makeDNS1039Compatible("p\u00f6d-\u00e9") shouldEqual
"pod-e"
+ }
+
+ "trim leading and trailing hyphens" in {
+ KubernetesApi.makeDNS1039Compatible("_my-pod.") shouldEqual "my-pod"
+ }
+
+ "reject names longer than 63 characters" in {
+ an[IllegalArgumentException] should be thrownBy
KubernetesApi.makeDNS1039Compatible("a" * 64)
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
