Repository: linkerd/linkerd2 (go)

Issue:
`duplicate metrics` warnings in service-mirror
### What is the issue?

I'm seeing `failed to register Prometheus gauge ... duplicate metrics collector registration attempted` warnings for `service_cache_size` and `endpoints_cache_size` metrics in the service-mirror log.

### How can it be reproduced?

Follow the Multicluster install instructions: https://linkerd.io/2.14/tasks/multicluster/, then:
```bash
kubectl -n linkerd-multicluster logs -f deploy/linkerd-service-mirror-east -c service-mirror
```

### Logs, error output, etc

```bash
$ kubectl -n linkerd-multicluster logs -f deploy/linkerd-service-mirror-east -c service-mirror
...
time="2023-12-26T22:40:54Z" level=warning msg="failed to register Prometheus gauge Desc{fqName: \"service_cache_size\", help: \"Number of items in the client-go service cache\", constLabels: {cluster=\"remote\"}, variableLabels: []}: duplicate metrics collector registration attempted"
time="2023-12-26T22:40:54Z" level=warning msg="failed to register Prometheus gauge Desc{fqName: \"endpoints_cache_size\", help: \"Number of items in the client-go endpoints cache\", constLabels: {cluster=\"remote\"}, variableLabels: []}: duplicate metrics collector registration attempted"
```

### output of `linkerd check -o short`

```bash
$ linkerd check -o short
linkerd-buoyant
---------------
‼ Linkerd health ok
    Linkerd not connected to Buoyant Cloud
    see https://buoyant.io/linkerd/debugging#linkerd-health for hints
‼ Linkerd vulnerability report ok
    Linkerd not connected to Buoyant Cloud
    see https://buoyant.io/linkerd/debugging#linkerd-vulnerability-report for hints
‼ Linkerd data plane upgrade assistance ok
    Linkerd not connected to Buoyant Cloud
    see https://buoyant.io/linkerd/debugging#linkerd-data-plane-upgrade for hints
‼ Linkerd trust anchor rotation assistance ok
    Linkerd not connected to Buoyant Cloud
    see https://buoyant.io/linkerd/debugging#linkerd-trust-anchor-rotation for hints

Status check results are √
```

### Environment

- Kubernetes Version: `v1.27.3`
- Cluster Environment: kind
- Host OS: `Ubuntu 22.04.3 LTS`
- Linkerd version: `stable-2.14.7`

### Possible solution

Ensure Prometheus metrics are registered only once, preferably at startup, and/or scope each registration with subsystem names.

### Additional context

_No response_

### Would you like to work on fixing this bug?

maybe

Pull request description:
Unregister prom gauges when recycling cluster watcher
Fixes #11839

When in `restartClusterWatcher` we fail to connect to the target cluster for whatever reason, the function gets called again 10s later, and tries to register the same prometheus metrics without unregistering them first, which generates warnings.

The problem lies in `NewRemoteClusterServiceWatcher`, which instantiates the remote kube-api client and registers the metrics, returning a nil object if the client can't connect. `cleanupWorkers` at the beginning of `restartClusterWatcher` won't unregister those metrics because of that nil object.

This fix reorders `NewRemoteClusterServiceWatcher` so that an object is returned even when there's an error, so cleanup on that object can be performed.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 91080e27c..1f47adcfc 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -25,9 +25,9 @@ checksum = "aae1277d39aeec15cb388266ecc24b11c80469deae6067e17a1a7aa9e5c1f234"
 
 [[package]]
 name = "ahash"
-version = "0.8.6"
+version = "0.8.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "91429305e9f0a25f6205c5b8e0d2db09e0708a7a6df0f42212bb56c32c8ac97a"
+checksum = "77c3a9648d43b9cd48db467b3f87fdd6e146bcc88ab0180006cef2179fe11d01"
 dependencies = [
  "cfg-if",
  "getrandom",
@@ -826,9 +826,9 @@ dependencies = [
 
 [[package]]
 name = "indexmap"
-version = "1.9.2"
+version = "1.9.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1885e79c1fc4b10f0e172c475f458b7f7b93061064d98c3293e98c5ba0c8b399"
+checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99"
 dependencies = [
  "autocfg",
  "hashbrown 0.12.3",
@@ -1181,6 +1181,7 @@ dependencies = [
  "linkerd-policy-controller-k8s-status",
  "openssl",
  "parking_lot",
+ "regex",
  "serde",
  "serde_json",
  "thiserror",
@@ -1364,9 +1365,9 @@ dependencies = [
 
 [[package]]
 name = "matchit"
-version = "0.7.0"
+version = "0.7.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b87248edafb776e59e6ee64a79086f65890d3510f2c656c000bf2a7e8a0aea40"
+checksum = "0e7465ac9959cc2b1404e8e2367b43684a6d13790fe23056cc8c6c5a6b7bcb94"
 
 [[package]]
 name = "memchr"
@@ -1564,9 +1565,9 @@ checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e"
 
 [[package]]
 name = "pest"
-version = "2.7.5"
+version = "2.7.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ae9cee2a55a544be8b89dc6848072af97a20f2422603c10865be2a42b580fff5"
+checksum = "1f200d8d83c44a45b21764d1916299752ca035d15ecd46faca3e9a2a2bf6ad06"
 dependencies = [
  "memchr",
  "thiserror",
@@ -2391,7 +2392,7 @@ checksum = "b8fa9be0de6cf49e536ce1851f987bd21a43b771b09473c3549a6c853db37c1c"
 dependencies = [
  "futures-core",
  "futures-util",
- "indexmap 1.9.2",
+ "indexmap 1.9.3",
  "pin-project",
  "pin-project-lite",
  "rand",
diff --git a/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml b/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml
index 6840d5ee6..58ee815f5 100644
--- a/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml
+++ b/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml
@@ -49,7 +49,6 @@ spec:
                   minItems: 1
                   items:
                     type: string
-                    pattern: '^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'
                 identityRefs:
                   type: array
                   minItems: 1
diff --git a/charts/linkerd2-cni/README.md b/charts/linkerd2-cni/README.md
index 4724ca9ac..e24dc6e31 100644
--- a/charts/linkerd2-cni/README.md
+++ b/charts/linkerd2-cni/README.md
@@ -31,7 +31,7 @@ Kubernetes: `>=1.22.0-0`
 | ignoreOutboundPorts | string | `""` | Default set of outbound ports to skip via iptables |
 | image.name | string | `"cr.l5d.io/linkerd/cni-plugin"` | Docker image for the CNI plugin |
 | image.pullPolicy | string | `"IfNotPresent"` | Pull policy for the linkerd-cni container |
-| image.version | string | `"v1.2.2"` | Tag for the CNI container Docker image |
+| image.version | string | `"v1.3.0"` | Tag for the CNI container Docker image |
 | imagePullSecrets | list | `[]` |  |
 | inboundProxyPort | int | `4143` | Inbound port for the proxy container |
 | logLevel | string | `"info"` | Log level for the CNI plugin |
@@ -43,7 +43,17 @@ Kubernetes: `>=1.22.0-0`
 | proxyAdminPort | int | `4191` | Admin port for the proxy container |
 | proxyControlPort | int | `4190` | Control port for the proxy container |
 | proxyUID | int | `2102` | User id under which the proxy shall be ran |
-| resources | object | `{"cpu":{"limit":"","request":""},"ephemeral-storage":{"limit":"","request":""},"memory":{"limit":"","request":""}}` | Resource requests and limits for linkerd-cni daemonset containers |
+| repairController.enableSecurityContext | bool | `true` | Include a securityContext in the repair-controller container |
+| repairController.enabled | bool | `false` | Enables the repair-controller container |
+| repairController.logFormat | string | plain | Log format (`plain` or `json`) for the repair-controller container |
+| repairController.logLevel | string | info | Log level for the repair-controller container |
+| repairController.resources.cpu.limit | string | `""` | Maximum amount of CPU units that the repair-controller container can use |
+| repairController.resources.cpu.request | string | `""` | Amount of CPU units that the repair-controller container requests |
+| repairController.resources.ephemeral-storage.limit | string | `""` | Maximum amount of ephemeral storage that the repair-controller container can use |
+| repairController.resources.ephemeral-storage.request | string | `""` | Amount of ephemeral storage that the repair-controller container requests |
+| repairController.resources.memory.limit | string | `""` | Maximum amount of memory that the repair-controller container can use |
+| repairController.resources.memory.request | string | `""` | Amount of memory that the repair-controller container requests |
+| resources | object | `{"cpu":{"limit":"","request":""},"ephemeral-storage":{"limit":"","request":""},"memory":{"limit":"","request":""}}` | Resource requests and limits for linkerd-cni daemonset container |
 | resources.cpu.limit | string | `""` | Maximum amount of CPU units that the cni container can use |
 | resources.cpu.request | string | `""` | Amount of CPU units that the cni container requests |
 | resources.ephemeral-storage.limit | string | `""` | Maximum amount of ephemeral storage that the cni container can use |
diff --git a/charts/linkerd2-cni/templates/cni-plugin.yaml b/charts/linkerd2-cni/templates/cni-plugin.yaml
index 160449ee3..69d3e0e64 100644
--- a/charts/linkerd2-cni/templates/cni-plugin.yaml
+++ b/charts/linkerd2-cni/templates/cni-plugin.yaml
@@ -112,6 +112,14 @@ rules:
 - apiGroups: [""]
   resources: ["pods", "nodes", "namespaces", "services"]
   verbs: ["list", "get", "watch"]
+{{- if .Values.repairController.enabled }}
+- apiGroups: [""]
+  resources: ["pods"]
+  verbs: ["delete"]
+- apiGroups: ["events.k8s.io"]
+  resources: ["events"]
+  verbs: ["create"]
+{{- end }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
@@ -274,6 +282,59 @@ spec:
         {{- if .Values.resources }}
         {{- include "partials.resources" .Values.resources | nindent 8 }}
         {{- end }}
+      {{- if .Values.repairController.enabled }}
+      # This container watches over pods whose linkerd-network-validator
+      # container failed, probably because of a race condition while setting up
+      # the CNI plugin chain, and deletes those pods so they can try acquiring a
+      # proper network config again
+      - name: repair-controller
+        image: {{ .Values.image.name -}}:{{- .Values.image.version }}
+        imagePullPolicy: {{ .Values.image.pullPolicy }}
+        {{- if .Values.repairController.enableSecurityContext }}
+        env:
+        - name: LINKERD_CNI_REPAIR_CONTROLLER_NODE_NAME
+          valueFrom:
+            fieldRef:
+              fieldPath: spec.nodeName
+        - name: LINKERD_CNI_REPAIR_CONTROLLER_POD_NAME
+          valueFrom:
+            fieldRef:
+              fieldPath: metadata.name
+        command:
+          - /usr/lib/linkerd/linkerd-cni-repair-controller
+        args:
+          - --admin-addr=0.0.0.0:9990
+          - --log-format
+          - {{ .Values.repairController.logFormat }}
+          - --log-level
+          - {{ .Values.repairController.logLevel }}
+        livenessProbe:
+          httpGet:
+            path: /live
+            port: admin-http
+        readinessProbe:
+          failureThreshold: 7
+          httpGet:
+            path: /ready
+            port: admin-http
+          initialDelaySeconds: 10
+        ports:
+        - containerPort: 9990
+          name: admin-http
+        securityContext:
+          allowPrivilegeEscalation: false
+          capabilities:
+            drop:
+            - ALL
+          privileged: false
+          readOnlyRootFilesystem: true
+          seccompProfile:
+            type: RuntimeDefault
+        {{- end }}
+        {{- if .Values.resources }}
+        {{- include "partials.resources" .Values.resources | nindent 8 }}
+        {{- end }}
+      {{- end }}
       volumes:
       {{- if ne .Values.destCNIBinDir .Values.destCNINetDir }}
       - name: cni-bin-dir
diff --git a/charts/linkerd2-cni/values.yaml b/charts/linkerd2-cni/values.yaml
index 977763edb..a9f9e8fd4 100644
--- a/charts/linkerd2-cni/values.yaml
+++ b/charts/linkerd2-cni/values.yaml
@@ -53,7 +53,7 @@ image:
   # -- Docker image for the CNI plugin
   name: "cr.l5d.io/linkerd/cni-plugin"
   # -- Tag for the CNI container Docker image
-  version: "v1.2.2"
+  version: "v1.3.0"
   # -- Pull policy for the linkerd-cni container
   pullPolicy: IfNotPresent
 
@@ -71,22 +71,44 @@ imagePullSecrets: []
 
 # -- Add additional initContainers to the daemonset
 extraInitContainers: []
-# - name: wait-for-other-cni
-#   image: busybox:1.33
-#   command:
-#     - /bin/sh
-#     - -xc
-#     - |
-#       for i in $(seq 1 180); do
-#         test -f /host/etc/cni/net.d/10-aws.conflist && exit 0
-#         sleep 1
-#       done
-#       exit 1
-#   volumeMounts:
-#     - mountPath: /host/etc/cni/net.d
-#       name: cni-net-dir
 
-# -- Resource requests and limits for linkerd-cni daemonset containers
+# The cni-repair-controller scans pods in each node to find those that have
+# been injected by linkerd, and whose linkerd-network-validator container has
+# failed. This is usually caused by a race between linkerd-cni and the CNI
+# plugin used in the cluster. This controller deletes those failed pods so they
+# can restart and rety re-acquiring a proper network config.
+repairController:
+  # -- Enables the repair-controller container
+  enabled: false
+
+  # -- Log level for the repair-controller container
+  # @default -- info
+  logLevel: info
+  # -- Log format (`plain` or `json`) for the repair-controller container
+  # @default -- plain
+  logFormat: plain
+
+  # -- Include a securityContext in the repair-controller container
+  enableSecurityContext: true
+
+  resources:
+    cpu:
+      # -- Maximum amount of CPU units that the repair-controller container can use
+      limit: ""
+      # -- Amount of CPU units that the repair-controller container requests
+      request: ""
+    memory:
+      # -- Maximum amount of memory that the repair-controller container can use
+      limit: ""
+      # -- Amount of memory that the repair-controller container requests
+      request: ""
+    ephemeral-storage:
+      # -- Maximum amount of ephemeral storage that the repair-controller container can use
+      limit: ""
+      # -- Amount of ephemeral storage that the repair-controller container requests
+      request: ""
+
+# -- Resource requests and limits for linkerd-cni daemonset container
 resources:
   cpu:
     # -- Maximum amount of CPU units that the cni container can use
diff --git a/controller/k8s/api.go b/controller/k8s/api.go
index e3f800359..728c3959f 100644
--- a/controller/k8s/api.go
+++ b/controller/k8s/api.go
@@ -67,7 +67,10 @@ type API struct {
 	l5dCrdSharedInformers l5dcrdinformer.SharedInformerFactory
 }
 
-// InitializeAPI creates Kubernetes clients and returns an initialized API wrapper.
+// InitializeAPI creates Kubernetes clients and returns an initialized API
+// wrapper. This creates informers on each one of resources passed, registering
+// metrics on each one; don't forget to call UnregisterGauges() on the returned
+// API reference to clean them up!
 func InitializeAPI(ctx context.Context, kubeConfig string, ensureClusterWideAccess bool, cluster string, resources ...APIResource) (*API, error) {
 	config, err := k8s.GetConfig(kubeConfig, "")
 	if err != nil {
@@ -87,7 +90,10 @@ func InitializeAPI(ctx context.Context, kubeConfig string, ensureClusterWideAcce
 	return initAPI(ctx, k8sClient, dynamicClient, config, ensureClusterWideAccess, cluster, resources...)
 }
 
-// InitializeAPIForConfig creates Kubernetes clients and returns an initialized API wrapper.
+// InitializeAPIForConfig creates Kubernetes clients and returns an initialized
+// API wrapper. This creates informers on each one of resources passed,
+// registering metrics on each one; don't forget to call UnregisterGauges() on
+// the returned API reference to clean them up!
 func InitializeAPIForConfig(ctx context.Context, kubeConfig *rest.Config, ensureClusterWideAccess bool, cluster string, resources ...APIResource) (*API, error) {
 	k8sClient, err := k8s.NewAPIForConfig(kubeConfig, "", []string{}, 0, 0, 0)
 	if err != nil {
@@ -141,7 +147,10 @@ func initAPI(ctx context.Context, k8sClient *k8s.KubernetesAPI, dynamicClient dy
 	return api, nil
 }
 
-// NewClusterScopedAPI takes a Kubernetes client and returns an initialized cluster-wide API.
+// NewClusterScopedAPI takes a Kubernetes client and returns an initialized
+// cluster-wide API. This creates informers on each one of resources passed,
+// registering metrics on each one; don't forget to call UnregisterGauges() on
+// the returned API reference to clean them up!
 func NewClusterScopedAPI(
 	k8sClient kubernetes.Interface,
 	dynamicClient dynamic.Interface,
@@ -153,7 +162,10 @@ func NewClusterScopedAPI(
 	return newAPI(k8sClient, dynamicClient, l5dCrdClient, sharedInformers, cluster, resources...)
 }
 
-// NewNamespacedAPI takes a Kubernetes client and returns an initialized API scoped to namespace.
+// NewNamespacedAPI takes a Kubernetes client and returns an initialized API
+// scoped to namespace. This creates informers on each one of resources passed,
+// registering metrics on each one; don't forget to call UnregisterGauges() on
+// the returned API reference to clean them up!
 func NewNamespacedAPI(
 	k8sClient kubernetes.Interface,
 	dynamicClient dynamic.Interface,
diff --git a/multicluster/cmd/service-mirror/main.go b/multicluster/cmd/service-mirror/main.go
index 8e34b40d6..614df0419 100644
--- a/multicluster/cmd/service-mirror/main.go
+++ b/multicluster/cmd/service-mirror/main.go
@@ -303,7 +303,7 @@ func restartClusterWatcher(
 	if err != nil {
 		return fmt.Errorf("unable to parse kube config: %w", err)
 	}
-	clusterWatcher, err = servicemirror.NewRemoteClusterServiceWatcher(
+	cw, err := servicemirror.NewRemoteClusterServiceWatcher(
 		ctx,
 		namespace,
 		controllerK8sAPI,
@@ -317,6 +317,7 @@ func restartClusterWatcher(
 	if err != nil {
 		return fmt.Errorf("unable to create cluster watcher: %w", err)
 	}
+	clusterWatcher = cw
 	err = clusterWatcher.Start(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to start cluster watcher: %w", err)
diff --git a/multicluster/service-mirror/cluster_watcher.go b/multicluster/service-mirror/cluster_watcher.go
index 5fb0f6632..2b52aea6a 100644
--- a/multicluster/service-mirror/cluster_watcher.go
+++ b/multicluster/service-mirror/cluster_watcher.go
@@ -175,6 +175,7 @@ func NewRemoteClusterServiceWatcher(
 	}
 	_, err = remoteAPI.Client.Discovery().ServerVersion()
 	if err != nil {
+		remoteAPI.UnregisterGauges()
 		return nil, fmt.Errorf("cannot connect to api for target cluster %s: %w", clusterName, err)
 	}
 
diff --git a/pkg/charts/cni/values.go b/pkg/charts/cni/values.go
index 6c04d7764..46e29d23d 100644
--- a/pkg/charts/cni/values.go
+++ b/pkg/charts/cni/values.go
@@ -35,6 +35,15 @@ type Resources struct {
 	EphemeralStorage Constraints `json:"ephemeral-storage"`
 }
 
+// RepairController contains the config for the repair-controller container
+type RepairController struct {
+	Image                 Image     `json:"image"`
+	LogLevel              string    `json:"logLevel"`
+	LogFormat             string    `json:"logFormat"`
+	EnableSecurityContext bool      `json:"enableSecurityContext"`
+	Resources             Resources `json:"resources"`
+}
+
 // Values contains the top-level elements in the cni Helm chart
 type Values struct {
 	InboundProxyPort    uint                `json:"inboundProxyPort"`
@@ -60,6 +69,7 @@ type Values struct {
 	EnablePSP           bool                `json:"enablePSP"`
 	Privileged          bool                `json:"privileged"`
 	Resources           Resources           `json:"resources"`
+	RepairController    RepairController    `json:"repairController"`
 }
 
 // NewValues returns a new instance of the Values type.
diff --git a/policy-controller/Cargo.toml b/policy-controller/Cargo.toml
index 7579db45a..188f3a8bf 100644
--- a/policy-controller/Cargo.toml
+++ b/policy-controller/Cargo.toml
@@ -33,6 +33,7 @@ serde_json = "1"
 thiserror = "1"
 tokio-stream = { version = "0.1", features = ["sync"] }
 tracing = "0.1"
+regex = "1"
 
 [dependencies.clap]
 version = "4"
diff --git a/policy-controller/src/admission.rs b/policy-controller/src/admission.rs
index 09515126b..63eb41cf2 100644
--- a/policy-controller/src/admission.rs
+++ b/policy-controller/src/admission.rs
@@ -1,3 +1,4 @@
+use super::validation;
 use crate::k8s::{
     labels,
     policy::{
@@ -296,7 +297,12 @@ fn validate_identity_ref(id: &NamespacedTargetRef) -> Result<()> {
 #[async_trait::async_trait]
 impl Validate<MeshTLSAuthenticationSpec> for Admission {
     async fn validate(self, _ns: &str, _name: &str, spec: MeshTLSAuthenticationSpec) -> Result<()> {
-        // The CRD validates identity strings, but does not validate identity references.
+        for id in spec.identities.iter().flatten() {
+            if let Err(err) = validation::validate_identity(id) {
+                bail!("id {} is invalid: {}", id, err);
+            }
+        }
+
         for id in spec.identity_refs.iter().flatten() {
             validate_identity_ref(id)?;
         }
diff --git a/policy-controller/src/lib.rs b/policy-controller/src/lib.rs
index 4fbff7360..8efbb1cb9 100644
--- a/policy-controller/src/lib.rs
+++ b/policy-controller/src/lib.rs
@@ -2,6 +2,7 @@
 #![forbid(unsafe_code)]
 mod admission;
 pub mod index_list;
+mod validation;
 pub use self::admission::Admission;
 use anyhow::Result;
 use linkerd_policy_controller_core::inbound::{
diff --git a/policy-controller/src/validation.rs b/policy-controller/src/validation.rs
new file mode 100644
index 000000000..c84840268
--- /dev/null
+++ b/policy-controller/src/validation.rs
@@ -0,0 +1,245 @@
+use anyhow::Result;
+use thiserror::Error;
+
+use regex::Regex;
+
+const SCHEME_PREFIX: &str = "spiffe://";
+const VALID_TRUST_DOMAIN_CHARS: &str = "abcdefghijklmnopqrstuvwxyz0123456789-._";
+const VALID_PATH_SEGMENT_CHARS: &str =
+    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._";
+
+const DNS_LIKE_IDENTITY_REGEX: &str =
+    r"^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$";
+
+#[derive(Debug, Error, PartialEq, Clone)]
+pub enum IdError {
+    /// The trust domain name of SPIFFE ID cannot be empty.
+    #[error("SPIFFE trust domain is missing")]
+    MissingTrustDomain,
+
+    /// A trust domain name can only contain chars in a limited char set.
+    #[error(
+        "SPIFFE trust domain characters are limited to lowercase letters, numbers, dots, dashes, and \
+         underscores"
+    )]
+    BadTrustDomainChar,
+
+    /// A path segment can only contain chars in a limited char set.
+    #[error(
+        "SPIFFE path segment characters are limited to letters, numbers, dots, dashes, and underscores"
+    )]
+    BadPathSegmentChar,
+
+    /// Path cannot contain empty segments, e.g '//'
+    #[error("SPIFFE path cannot contain empty segments")]
+    EmptySegment,
+
+    /// Path cannot contain dot segments, e.g '/.', '/..'
+    #[error("SPIFFE path cannot contain dot segments")]
+    DotSegment,
+
+    /// Path cannot have a trailing slash.
+    #[error("SPIFFE path cannot have a trailing slash")]
+    TrailingSlash,
+
+    #[error(
+        "identity must be a valid SPIFFE id or a DNS SAN, matching the regex: {}",
+        DNS_LIKE_IDENTITY_REGEX
+    )]
+    Invalid,
+}
+
+// Validates that an ID is either in DNS or SPIFFE form. SPIFFE
+// validation is based on https://github.com/spiffe/spiffe/blob/27b59b81ba8c56885ac5d4be73b35b9b3305fd7a/standards/SPIFFE-ID.md.
+// Implementation is based on: https://github.com/maxlambrecht/rust-spiffe/blob/3d3614f70d0d7a4b9190ab9650e224f2ac362368/spiffe/src/spiffe_id/mod.rs
+pub(crate) fn validate_identity(id: &str) -> Result<(), IdError> {
+    if let Some(rest) = id.strip_prefix(SCHEME_PREFIX) {
+        let i = rest.find('/').unwrap_or(rest.len());
+
+        if i == 0 {
+            return Err(IdError::MissingTrustDomain);
+        }
+
+        let td = &rest[..i];
+        if td.chars().any(|c| !VALID_TRUST_DOMAIN_CHARS.contains(c)) {
+            return Err(IdError::BadTrustDomainChar);
+        }
+
+        let path = &rest[i..];
+
+        if !path.is_empty() {
+            validate_path(path)?;
+        }
+
+        Ok(())
+    } else {
+        let regex = Regex::new(DNS_LIKE_IDENTITY_REGEX).expect("should_compile");
+        if !regex.is_match(id) {
+            return Err(IdError::Invalid);
+        }
+        Ok(())
+    }
+}
+
+/// Validates that a path string is a conformant path for a SPIFFE ID.
+/// See https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md#22-path
+fn validate_path(path: &str) -> Result<(), IdError> {
+    let chars = path.char_indices().peekable();
+    let mut segment_start = 0;
+
+    for (idx, c) in chars {
+        if c == '/' {
+            match &path[segment_start..idx] {
+                "/" => return Err(IdError::EmptySegment),
+                "/." | "/.." => return Err(IdError::DotSegment),
+                _ => {}
+            }
+            segment_start = idx;
+            continue;
+        }
+
+        if !VALID_PATH_SEGMENT_CHARS.contains(c) {
+            return Err(IdError::BadPathSegmentChar);
+        }
+    }
+
+    match &path[segment_start..] {
+        "/" => return Err(IdError::TrailingSlash),
+        "/." | "/.." => return Err(IdError::DotSegment),
+        _ => {}
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn valid_dns() {
+        assert!(validate_identity("system.local").is_ok())
+    }
+
+    #[test]
+    fn valid_dns_all() {
+        assert!(validate_identity("*").is_ok())
+    }
+
+    #[test]
+    fn valid_dns_prefix() {
+        assert!(validate_identity("*.system.local").is_ok())
+    }
+
+    #[test]
+    fn invalid_dns_suffix() {
+        let err = validate_identity("system.local.*").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn invalid_dns_trailing_dot() {
+        let err = validate_identity("system.local.").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn invalid_dns_leading_dot() {
+        let err = validate_identity(".system.local").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn invalid_dns_double_dots() {
+        let err = validate_identity("system..local").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn valid_spiffe_no_path() {
+        assert!(validate_identity("spiffe://trustdomain").is_ok())
+    }
+
+    #[test]
+    fn valid_spiffe_with_path() {
+        assert!(validate_identity("spiffe://trustdomain/path/element").is_ok())
+    }
+
+    #[test]
+    fn invalid_spiffe_scheme() {
+        let err = validate_identity("http://domain.test/path/element").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn invalid_spiffe_wrong_scheme() {
+        let err = validate_identity("spiffe:/path/element").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn invalid_spiffe_empty_trust_domain() {
+        let err = validate_identity("spiffe:///path/element").unwrap_err();
+        assert_eq!(err, IdError::MissingTrustDomain);
+    }
+
+    #[test]
+    fn invalid_spiffe_no_slashes_in_scheme() {
+        let err = validate_identity("spiffe:path/element").unwrap_err();
+        assert_eq!(err, IdError::Invalid);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_with_query() {
+        let err = validate_identity("spiffe://domain.test/path/element?query=").unwrap_err();
+        assert_eq!(err, IdError::BadPathSegmentChar);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_with_fragment() {
+        let err = validate_identity("spiffe://domain.test/path/element#fragment-1").unwrap_err();
+        assert_eq!(err, IdError::BadPathSegmentChar);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_with_str_port() {
+        let err = validate_identity("spiffe://domain.test:8080/path/element").unwrap_err();
+        assert_eq!(err, IdError::BadTrustDomainChar);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_with_user_info() {
+        let err = validate_identity("spiffe://user:password@test.org/path/element").unwrap_err();
+        assert_eq!(err, IdError::BadTrustDomainChar);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_with_trailing_slash() {
+        let err = validate_identity("spiffe://test.org/").unwrap_err();
+        assert_eq!(err, IdError::TrailingSlash);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_with_empty_segment() {
+        let err = validate_identity("spiffe://test.org//").unwrap_err();
+        assert_eq!(err, IdError::EmptySegment);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_str_with_path_with_trailing_slash() {
+        let err = validate_identity("spiffe://test.org/path/other/").unwrap_err();
+        assert_eq!(err, IdError::TrailingSlash);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_str_with_dot_segment() {
+        let err = validate_identity("spiffe://test.org/./other").unwrap_err();
+        assert_eq!(err, IdError::DotSegment);
+    }
+
+    #[test]
+    fn invalid_spiffe_uri_str_with_double_dot_segment() {
+        let err = validate_identity("spiffe://test.org/../other").unwrap_err();
+        assert_eq!(err, IdError::DotSegment);
+    }
+}


Test patch:
diff --git a/cli/cmd/testdata/install_cni_helm_default_output.golden b/cli/cmd/testdata/install_cni_helm_default_output.golden
index 6bd305f06..566534f1e 100644
--- a/cli/cmd/testdata/install_cni_helm_default_output.golden
+++ b/cli/cmd/testdata/install_cni_helm_default_output.golden
@@ -111,7 +111,7 @@ spec:
       # script copies the files into place and then sleeps so
       # that Kubernetes doesn't keep trying to restart it.
       - name: install-cni
-        image: cr.l5d.io/linkerd/cni-plugin:v1.2.2
+        image: cr.l5d.io/linkerd/cni-plugin:v1.3.0
         imagePullPolicy: IfNotPresent
         env:
         - name: DEST_CNI_NET_DIR
diff --git a/cli/cmd/testdata/install_crds.golden b/cli/cmd/testdata/install_crds.golden
index abaf6946d..9632fe2c7 100644
--- a/cli/cmd/testdata/install_crds.golden
+++ b/cli/cmd/testdata/install_crds.golden
@@ -5364,7 +5364,6 @@ spec:
                   minItems: 1
                   items:
                     type: string
-                    pattern: '^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'
                 identityRefs:
                   type: array
                   minItems: 1
diff --git a/cli/cmd/testdata/install_helm_crds_output.golden b/cli/cmd/testdata/install_helm_crds_output.golden
index b010d4d93..ee66ee8f1 100644
--- a/cli/cmd/testdata/install_helm_crds_output.golden
+++ b/cli/cmd/testdata/install_helm_crds_output.golden
@@ -5370,7 +5370,6 @@ spec:
                   minItems: 1
                   items:
                     type: string
-                    pattern: '^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'
                 identityRefs:
                   type: array
                   minItems: 1
diff --git a/cli/cmd/testdata/install_helm_crds_output_ha.golden b/cli/cmd/testdata/install_helm_crds_output_ha.golden
index b010d4d93..ee66ee8f1 100644
--- a/cli/cmd/testdata/install_helm_crds_output_ha.golden
+++ b/cli/cmd/testdata/install_helm_crds_output_ha.golden
@@ -5370,7 +5370,6 @@ spec:
                   minItems: 1
                   items:
                     type: string
-                    pattern: '^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'
                 identityRefs:
                   type: array
                   minItems: 1