Skip to content

Enhancement: Add strict format validation for yurthub storage keys - #2776

Open
mohammd-khalil wants to merge 1 commit into
openyurtio:masterfrom
mohammd-khalil:fix-validate-key-format
Open

Enhancement: Add strict format validation for yurthub storage keys#2776
mohammd-khalil wants to merge 1 commit into
openyurtio:masterfrom
mohammd-khalil:fix-validate-key-format

Conversation

@mohammd-khalil

Copy link
Copy Markdown

What type of PR is this?

/kind enhancement
/sig storage

What this PR does / why we need it:

ValidateKey in pkg/yurthub/storage/utils/validate.go currently only checks
that a storage.Key is non-nil/non-empty and has the expected concrete Go
type (via reflect.TypeOf). It does not validate the internal format of the
key's path, as noted by an existing TODO comment:

// TODO: should also valid the key format

This means a key with the correct type but a malformed internal path (e.g.
missing segments, invalid resource/version/group format) passes validation
silently, and only fails later when it is actually parsed/used (e.g. by
disk.ExtractKeyBuildInfo).

This PR completes the TODO by:

  • Adding an optional KeyFormatValidator interface (Validate() error) in
    pkg/yurthub/storage, kept separate from the existing minimal Key
    interface so it stays backward compatible for key types that don't need
    format validation (e.g. ClusterInfoKey).
  • Implementing Validate() on disk.storageKey, reusing the same parsing
    rules already used by ExtractKeyBuildInfo.
  • Updating ValidateKey in utils/validate.go to call Validate() via a
    type assertion, only when the key implements it.
  • Adding unit tests covering both disk.storageKey.Validate() and the
    updated utils.ValidateKey.

Which issue(s) this PR fixes:

Fixes #2775

Special notes for your reviewer:

  • Key interface itself (Key() string) was left unchanged; KeyFormatValidator
    is an additive, opt-in interface, so no existing implementation is broken.
  • etcd storage key was not modified in this PR since its key format/needs
    weren't covered — happy to follow up separately if a similar check is
    wanted there.

Does this PR introduce a user-facing change?

NONE

other Note

@mohammd-khalil
mohammd-khalil requested a review from a team as a code owner August 27, 2026 23:12
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ValidateKey does not validate the internal format of storage keys

1 participant