Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential KDS syncing problem by the resource name length #13015

Open
Icarus9913 opened this issue Mar 5, 2025 · 2 comments
Open

Potential KDS syncing problem by the resource name length #13015

Icarus9913 opened this issue Mar 5, 2025 · 2 comments
Labels
kind/bug A bug triage/accepted The issue was reviewed and is complete enough to start working on it

Comments

@Icarus9913
Copy link
Contributor

Icarus9913 commented Mar 5, 2025

Kuma Version

master

Describe the bug

We decide to use RFC 1035 Label Names for the name validation of our resources like Mesh, MeshService, MeshExternalService, MeshMultizoneService, Zone. Then we'll have the length restriction for them at most 63 characters.

In our KDS system, like you create a MeshService resource with length 63, but the name of new syncing MS to zones could be 80 (63 + 17) which breaks the validation.

To Reproduce

  1. Build a multi-zone environment (1 global, 3 zones)
  2. Create a MeshService resource with 63-length name on zone1(k8s).
apiVersion: kuma.io/v1alpha1
kind: MeshService
metadata:
  name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  namespace: kuma-system
  labels:
    kuma.io/mesh: default
    kuma.io/origin: zone
spec:
  selector:
    dataplaneTags:
      app: redis
  ports:
  - port: 6739
    targetPort: 6739
    appProtocol: tcp
  1. Then you'll notice the MeshService resource is synced to Global(k8s), zone2(k8s) and zone3(universal) with name aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dww29994z4bd2292

Global:

apiVersion: kuma.io/v1alpha1
kind: MeshService
metadata:
  annotations:
    kuma.io/display-name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  creationTimestamp: "2025-03-11T06:46:35Z"
  generation: 3
  labels:
    k8s.kuma.io/namespace: kuma-system
    kuma.io/env: kubernetes
    kuma.io/mesh: default
    kuma.io/origin: zone
    kuma.io/zone: kuma-2
  name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dww29994z4bd2292
  namespace: kuma-system
  ownerReferences:
  - apiVersion: kuma.io/v1alpha1
    kind: Zone
    name: kuma-2
    uid: 5d5476af-a033-4c8d-9990-d98f81fc7a63
  resourceVersion: "8920"
  uid: 06318032-bd97-4ed3-88cd-43d31b9ba331
spec:
  ports:
  - appProtocol: tcp
    port: 6739
    targetPort: 6739
  selector:
    dataplaneTags:
      app: redis
  state: Unavailable
status:
  dataplaneProxies: {}
  tls:
    status: Ready
  vips:
  - ip: 241.0.0.0

Zone2

apiVersion: kuma.io/v1alpha1
kind: MeshService
metadata:
  annotations:
    kuma.io/display-name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  creationTimestamp: "2025-03-11T06:46:37Z"
  generation: 2
  labels:
    k8s.kuma.io/namespace: kuma-system
    kuma.io/env: kubernetes
    kuma.io/mesh: default
    kuma.io/origin: global
    kuma.io/zone: kuma-2
  name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dww29994z4bd2292
  namespace: kuma-system
  resourceVersion: "3894"
  uid: 4674bd4a-edfd-49ad-b0db-5f4a364be8f1
spec:
  ports:
  - appProtocol: tcp
    port: 6739
    targetPort: 6739
  selector:
    dataplaneTags:
      app: redis
  state: Unavailable
status:
  dataplaneProxies: {}
  tls: {}
  vips:
  - ip: 241.0.0.0

Zone3

root@4dbed73f5cd8:~/test# ./kumactl get meshservice aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dww29994z4bd2292 -o yaml
creationTime: "2025-03-11T15:02:45.112197004+08:00"
labels:
  k8s.kuma.io/namespace: kuma-system
  kuma.io/display-name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  kuma.io/env: kubernetes
  kuma.io/mesh: default
  kuma.io/origin: global
  kuma.io/zone: kuma-2
mesh: default
modificationTime: "2025-03-11T15:02:52.455199587+08:00"
name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dww29994z4bd2292
spec:
  ports:
  - appProtocol: tcp
    port: 6739
    targetPort: 6739
  selector:
    dataplaneTags:
      app: redis
  state: Unavailable
status:
  dataplaneProxies: {}
  tls: {}
  vips:
  - ip: 241.0.0.0
type: MeshService

Expected behavior

No response

Additional context (optional)

Comes from #13003 (comment)

@Icarus9913 Icarus9913 added kind/bug A bug triage/pending This issue will be looked at on the next triage meeting labels Mar 5, 2025
@lahabana
Copy link
Contributor

Let's move the hashing for KDS to be a function in the ResourceTypeDescriptor.
Obviously the function should default to the current implementation(code below) but we can now specify a few resources (MeshService...) to be restricted to 64 chars in length

func HashedName(mesh, name string, additionalValuesToHash ...string) string {
return addSuffix(name, hash(append([]string{mesh, name}, additionalValuesToHash...)))
}
func addSuffix(name, hash string) string {
const hashLength = 1 + 16 // 1 dash plus 8-byte hash that is encoded with hex
const k8sNameLengthLimit = 253
shortenName := k8s_strings.ShortenString(name, k8sNameLengthLimit-hashLength)
return fmt.Sprintf("%s-%s", shortenName, hash)
}
func hash(ss []string) string {
hasher := fnv.New64a()
for _, s := range ss {
_, _ = hasher.Write([]byte(s))
}
b := []byte{}
b = hasher.Sum(b)
return rand.SafeEncodeString(hex.EncodeToString(b))
}

Then we could remove all this with:

func HashSuffixMapper(checkKDSFeature bool, labelsToUse ...string) reconcile_v2.ResourceMapper {

to something like: r.CloneHash(labelsToUse)

CloneHash could then be using the defaulting interface feature:

func (t *DoNothingPolicyResource) Validate() error {
if v, ok := interface{}(t).(interface{ validate() error }); !ok {
return nil
} else {
return v.validate()
}
}

@lahabana
Copy link
Contributor

On the compatibility front it "should" be alright because on upgrade KDS would remove the old one and add the new one. But this has to be checked.

@lobkovilya lobkovilya added triage/accepted The issue was reviewed and is complete enough to start working on it and removed triage/pending This issue will be looked at on the next triage meeting labels Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug triage/accepted The issue was reviewed and is complete enough to start working on it
Projects
None yet
Development

No branches or pull requests

3 participants