Skip to content

fix: support PVC overlays for DM built-in volumes - #7024

Open
fgksgf wants to merge 2 commits into
mainfrom
agent/fix-dm-builtin-pvc-overlays
Open

fix: support PVC overlays for DM built-in volumes#7024
fgksgf wants to merge 2 commits into
mainfrom
agent/fix-dm-builtin-pvc-overlays

Conversation

@fgksgf

@fgksgf fgksgf commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • Allow DM PVC overlays to target the built-in dataVolume.
  • Allow DMWorker PVC overlays to target the built-in relayVolume.
  • Apply the existing PVC overlay logic to those built-in PVCs without changing PVC specs, ownership, naming, or additional-volume behavior.

Why

ResourceTagger derives EBS cloud tags from tags.tidbcloud.com/* labels on PVCs. O11Y-2632914 identified three DM PVCs without those source labels because Cluster Service could not express PVC overlays for the built-in DM data and relay volumes.

The first E2E runs for this PR failed before executing any specs because Kubernetes rejected the generated DMGroup CRD: the new CEL validation rule exceeded the static cost budget. This update keeps the same validation semantics with a lower-cost expression and adds regression coverage for CRD CEL costs.

How

  • Extend DM and DMWorker CEL validation so overlay names may reference the built-in volume or an additional volume, while unknown names remain invalid.
  • Use singleton-list membership for the built-in volume comparison to stay within the Kubernetes static CEL cost budget.
  • Apply OverlayPersistentVolumeClaim to the built-in master data PVC and worker relay PVC.
  • Remove the consumed built-in overlay from a deep copy before invoking the existing additional-volume PVC builder.
  • Regenerate the four affected DM and DMWorker CRDs.
  • Add validation tests that compile the CEL rules for all four CRDs and enforce the Kubernetes runtime cost budget.

Testing

  • go test ./pkg/controllers/dm/tasks ./pkg/controllers/dmworker/tasks
  • cd tests/validation && go test ./...
  • make lint
  • make crd
  • Regenerated CRDs in a clean detached worktree and confirmed no diff.
  • Applied all generated CRDs to a local Kubernetes API server.
  • Server-side dry-run accepted built-in DM data and DMWorker relay overlays and rejected an unknown overlay name.

make verify/crd was attempted, but its temporary worktree failed before generation because Go resolved the macOS /var path as /private/var and treated the build path as outside the module. The equivalent clean-worktree regeneration check passed.

Risks and Reviewer Focus

  • This extends the legal meaning of an existing CRD field; it does not add a field or modify Pod labels.
  • A compatible Operator version and CRD must be released and deployed before Cluster Service emits built-in DM PVC overlays.
  • Existing PVCs are not backfilled by this change.

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes

    • PVC overlays now correctly apply to built-in data and relay volumes.
    • Prevented built-in volume overlays from being applied twice when creating additional PVCs.
    • Volume claim validation now accepts overlays targeting built-in or additional volumes.
    • Invalid references to unknown volume claims continue to be rejected.
  • Tests

    • Added coverage for overlay application, validation rules, and invalid volume claim references.

@ti-chi-bot

ti-chi-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fgksgf for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions github-actions Bot added the v2 for operator v2 label Aug 7, 2026
@ti-chi-bot ti-chi-bot Bot added the size/L label Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change permits named overlays for built-in data and relay volume claims. DM and DMWorker PVC reconciliation applies these overlays to primary PVCs and excludes them from additional PVC generation. Validation tests cover valid built-in claims, invalid unknown claims, and CEL static costs.

Changes

Volume claim overlays

Layer / File(s) Summary
Built-in volume overlay validation
api/core/v1alpha1/*_types.go, manifests/crd/*dm*.yaml, tests/validation/*
Validation accepts dataVolume or relayVolume claim names in addition to entries in volumes. Unknown claim names remain invalid. Tests also compile CEL rules and check their static cost.
DM PVC overlay application
pkg/controllers/dm/tasks/pvc.go, pkg/controllers/dm/tasks/pvc_test.go
DM PVC generation applies the data volume overlay and removes it from the copied resource before generating additional PVCs. Tests verify labels and annotations.
DMWorker PVC overlay application
pkg/controllers/dmworker/tasks/pvc.go, pkg/controllers/dmworker/tasks/pvc_test.go
DMWorker PVC generation applies the relay volume overlay and removes it from the copied resource before generating additional PVCs. Tests verify both generated PVCs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DMResource
  participant DMPVCNewer
  participant PVCOverlay
  participant GeneratedPVCs
  DMResource->>DMPVCNewer: Reconcile PVCs
  DMPVCNewer->>PVCOverlay: Find data volume overlay
  PVCOverlay-->>DMPVCNewer: Return matching overlay
  DMPVCNewer->>GeneratedPVCs: Apply overlay to data PVC
  DMPVCNewer->>GeneratedPVCs: Generate additional PVCs without data overlay
Loading
sequenceDiagram
  participant DMWorkerResource
  participant DMWorkerPVCNewer
  participant PVCOverlay
  participant GeneratedPVCs
  DMWorkerResource->>DMWorkerPVCNewer: Reconcile PVCs
  DMWorkerPVCNewer->>PVCOverlay: Find relay volume overlay
  PVCOverlay-->>DMWorkerPVCNewer: Return matching overlay
  DMWorkerPVCNewer->>GeneratedPVCs: Apply overlay to relay PVC
  DMWorkerPVCNewer->>GeneratedPVCs: Generate additional PVCs without relay overlay
Loading

Suggested reviewers: tennix

Poem

I’m a rabbit with overlays bright,
Data and relay now dress just right.
Claims are checked by name and place,
Extra PVCs keep their space.
Hop, hop—no overlay twice!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: support for PVC overlays on built-in DM volumes.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-dm-builtin-pvc-overlays

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.00%. Comparing base (92aa7f6) to head (ebfa959).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7024      +/-   ##
==========================================
+ Coverage   40.58%   41.00%   +0.42%     
==========================================
  Files         436      439       +3     
  Lines       25274    25541     +267     
==========================================
+ Hits        10258    10474     +216     
- Misses      15016    15067      +51     
Flag Coverage Δ
unittest 41.00% <100.00%> (+0.42%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fgksgf
fgksgf requested review from tennix and a balanced review from Copilot August 7, 2026 11:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the existing PVC overlay mechanism so that DM's built-in dataVolume and DMWorker's built-in relayVolume can be targeted by overlay.volumeClaims, in addition to the previously-supported additional volumes. Previously the CEL validation rejected any overlay whose name did not match an additional volume, which excluded these built-in volumes. The change loosens validation to also accept the built-in volume name and applies the overlay when constructing the built-in PVCs, while removing the consumed overlay entry before delegating to the shared additional-volume builder (which panics on unknown overlay names).

Changes:

  • Broaden the CEL XValidation rule on DMTemplateSpec/DMWorkerTemplateSpec to accept the built-in volume name (dataVolume/relayVolume) or an additional volume name, and regenerate the four affected CRDs.
  • Apply OverlayPersistentVolumeClaim to the built-in data/relay PVCs, and strip the consumed built-in overlay from a deep copy before calling coreutil.PVCs.
  • Add unit tests for overlay application and validation tests for the new/rejected cases.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
api/core/v1alpha1/dm_types.go Updates CEL rule to allow overlays targeting the built-in dataVolume.
api/core/v1alpha1/dmworker_types.go Updates CEL rule to allow overlays targeting the optional built-in relayVolume.
manifests/crd/core.pingcap.com_dms.yaml Regenerated CRD reflecting the DM CEL rule change.
manifests/crd/core.pingcap.com_dmgroups.yaml Regenerated CRD reflecting the DM CEL rule change.
manifests/crd/core.pingcap.com_dmworkers.yaml Regenerated CRD reflecting the DMWorker CEL rule change.
manifests/crd/core.pingcap.com_dmworkergroups.yaml Regenerated CRD reflecting the DMWorker CEL rule change.
pkg/controllers/dm/tasks/pvc.go Applies overlay to the data PVC and removes it from the additional-volume overlay copy.
pkg/controllers/dmworker/tasks/pvc.go Applies overlay to the relay PVC and removes it from the additional-volume overlay copy.
pkg/controllers/dm/tasks/pvc_test.go Adds test verifying data + additional volume overlays are applied.
pkg/controllers/dmworker/tasks/pvc_test.go Adds test verifying relay + additional volume overlays are applied.
tests/validation/dm_test.go Adds validation cases for built-in data overlay acceptance/rejection.
tests/validation/dmworker_test.go Adds validation cases for built-in relay overlay acceptance/rejection.
tests/validation/common_test.go Adds BuiltInVolumeClaimsOverlay helper generating the shared validation cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fgksgf

fgksgf commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

/test pull-e2e

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/validation/validation.go`:
- Around line 161-162: In the validation test before the loop over results, add
a non-empty assertion for results with the message “no CEL validation rules
found”. Keep the existing per-result require.Nil check unchanged.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4acab17f-24f8-4341-af1e-82430b830733

📥 Commits

Reviewing files that changed from the base of the PR and between efa42a2 and ebfa959.

📒 Files selected for processing (9)
  • api/core/v1alpha1/dm_types.go
  • api/core/v1alpha1/dmworker_types.go
  • manifests/crd/core.pingcap.com_dmgroups.yaml
  • manifests/crd/core.pingcap.com_dms.yaml
  • manifests/crd/core.pingcap.com_dmworkergroups.yaml
  • manifests/crd/core.pingcap.com_dmworkers.yaml
  • tests/validation/dm_test.go
  • tests/validation/dmworker_test.go
  • tests/validation/validation.go
🚧 Files skipped from review as they are similar to previous changes (6)
  • api/core/v1alpha1/dm_types.go
  • api/core/v1alpha1/dmworker_types.go
  • manifests/crd/core.pingcap.com_dmgroups.yaml
  • manifests/crd/core.pingcap.com_dms.yaml
  • manifests/crd/core.pingcap.com_dmworkergroups.yaml
  • manifests/crd/core.pingcap.com_dmworkers.yaml

Comment on lines +161 to +162
for i, result := range results {
require.Nil(t, result.Error, "%s validation rule %d", crdPath, i)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
fd -a 'validation.go|dm_test.go|dmworker_test.go|go.mod' . | sed 's#^\./##' | sort

echo
echo "== go.mod relevant deps =="
if [ -f go.mod ]; then
  rg 'k8s\.io/(api|apiextensions-apiserver|apimachinery)' go.mod
fi

echo
echo "== tests/validation/validation.go relevant section =="
if [ -f tests/validation/validation.go ]; then
  nl -ba tests/validation/validation.go | sed -n '1,220p'
fi

echo
echo "== tests/validation dm tests references =="
for f in tests/validation/dm_test.go tests/validation/dmworker_test.go; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    nl -ba "$f" | sed -n '1,220p'
  fi
done

Repository: pingcap/tidb-operator

Length of output: 1101


🌐 Web query:

Kubernetes apiextensions CelValidation Compile returns one validation result per CEL validation rule v0.31.14

💡 Result:

In Kubernetes, the CEL validation compilation process is designed to return one validation result for each defined Common Expression Language (CEL) validation rule [1]. This behavior is implemented in the Kubernetes API server's schema validation logic, specifically within the apiextensions-apiserver package [2][3]. When Compile is called on a set of x-kubernetes-validations rules, it iterates through the provided validation rules and performs the following for each: 1. It compiles the CEL expression string [1]. 2. It generates a CompilationResult that encapsulates the output of this process [1]. 3. Even if a rule is empty or fails compilation, a result is typically returned to account for that specific rule definition [1]. The CompilationResult structure ensures that compilation errors, successful ASTs, and other metadata (such as whether the rule is a transition rule or its estimated cost) are tracked on a per-rule basis [1]. While compilation occurs once during policy or CRD loading [4], it is distinct from runtime evaluation behavior. At runtime, Kubernetes may stop evaluation after the first validation failure depending on the specific enforcement context (such as in ValidatingAdmissionPolicy), though this is a separate concern from the compilation phase's output structure [5][6]. The v0.31.14 release update referenced in your query primarily involved dependency updates [7][8][9] rather than a fundamental change to this established compilation logic.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== validation.go relevant sections =="
python3 - <<'PY'
from pathlib import Path
p = Path('tests/validation/validation.go')
lines = p.read_text().splitlines()
for start,end in [(1,220), (120,190)]:
    print(f"--- lines {start}-{end} ---")
    for i in range(start, min(end, len(lines)) + 1):
        print(f"{i:4d}\t{lines[i-1]}")
PY

echo
echo "== dm/dmworker tests relevant sections =="
for f in tests/validation/dm_test.go tests/validation/dmworker_test.go; do
  echo "--- $f ---"
  python3 - <<PY
from pathlib import Path
p = Path('$f')
lines = p.read_text().splitlines()
for start,end in [(1,260)]:
    for i in range(start, min(end, len(lines)) + 1):
        print(f"{i:4d}\t{lines[i-1]}")
PY
done

echo
echo "== Kubernetes source: Compile result behavior =="
python3 - <<'PY'
try:
    import urllib.request as ureq
    url = 'https://raw.githubusercontent.com/kubernetes/kubernetes/v0.32.13/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go'
    with ureq.urlopen(url, timeout=20) as r:
        txt = r.read().decode('utf-8', 'replace').splitlines()
    for i, line in enumerate(txt, 1):
        if line.strip().startswith('func Compile(') or 'Append' in line and ('validation' in line.lower() or 'ValidationError' in line):
            a=max(1,i-8); b=min(len(txt),i+22); print(f"--- {a}-{b} ---")
            for j in range(a,b+1):
                print(f"{j:4d}\t{txt[j-1]}")
except Exception as e:
    print(f'ERROR: {e}')
PY

Repository: pingcap/tidb-operator

Length of output: 16812


🌐 Web query:

Kubernetes 0.32.13 pkg/apiserver/schema/cel validation.go Compile append ValidationResults CELValidation

💡 Result:

In Kubernetes, the CEL (Common Expression Language) validation logic is primarily located within the k8s.io/apiextensions-apiserver module, specifically in files such as pkg/apiserver/schema/cel/validation.go [1][2]. This file manages the validation of custom resources against CEL expressions defined in CRDs [2][3]. The internal implementation involves compiling these expressions and checking them against structural schemas [2][4]. Regarding the specific pattern mentioned: - ValidationResults and CELValidation are core components used to track and report errors during the validation process [4][5]. - The use of append in this context typically refers to the collection of ValidationResult objects or individual field errors as the validator traverses the object schema and evaluates rules [4][5][6]. - Recent updates to pkg/apiserver/schema/cel/validation.go have included refinements to how validation errors are reported and how the validator interacts with schema types (e.g., ensuring "object" or "array" types are handled correctly to prevent issues like crashloops or improper error messaging) [2][4]. For example, a recent change involved adjusting the validation logic to better handle schema types, where the code was updated to avoid improperly reporting the type string in error messages when a CEL rule fails, improving both clarity and stability [4]. This type of change ensures that when an error is generated and appended to the results list, it correctly reflects the validation failure without making incorrect assumptions about the underlying structural schema [4]. For authoritative technical details on current implementation, refer to the k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel package in the official Kubernetes repository [1].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== find CEL validation source in module cache =="
go env GOPATH 2>/dev/null || true
find "$(go env GOPATH 2>/dev/null || true)/pkg/mod" -path '*/k8s.io/apiextensions-apiserver@v0.32.13/pkg/apiserver/schema/cel/*.go' 2>/dev/null | sort | head -20

echo
echo "== inspect k8s.io/apiextensions-apiserver Compile implementation =="python3 - <<'PY'
from pathlib import Path
import subprocess, sys, re, textwrap, shlex

cmd = shlex.split('go env GOPATH MODCACHE GOMODCACHE 2>/dev/null')
envs = subprocess.run(cmd, text=True, capture_output=True)
for line in envs.stdout.splitlines():
    print(line)

paths=[]
for root in [Path.home()/'.cache/go-build','']:
    pass
mod="$(go env GOMODCACHE 2>/dev/null)/pkg/mod/k8s.io/apiextensions-apiserver@v0.32.13/pkg/apiserver/schema/cel"
files=[]
try:
    import subprocess
    files=subprocess.check_output(shlex.split(f'find "{mod}" -name "*.go"'), text=True).splitlines()
except Exception as e:
    print(f'go cache read failed: {e}')
    sys.exit(0)

def show(path, needles):
    lines = Path(path).read_text(errors='replace').splitlines()
    print(f'--- {path} ---')
    for idx, line in enumerate(lines, 1):
        if any(n in line for n in needles):
            a=max(1, idx-12); b=max(0, idx+25)
            for j in range(a,b):
                print(f'{j:4d}\t{lines[j-1]}')
            print()

for path in files:
    if str(path) == str(mod):
        continue
    txt = Path(path).read_text(errors='replace')
    if 'func Compile(' in txt or 'ValidationResult struct' in txt or 'compileC' in txt or 'compileValidator' in txt:
        show(path, ['func Compile(', 'ValidationResult', 'compileCustomResourceValidator', 'compileArrayValidator', 'compileObjectValidator'])

# narrow source scan for validation rule handling
for path in files:
    if Path(path).stem in ('validation', 'compile'):
        txt=Path(path).read_text(errors='replace')
        if 'Structural' in txt and ('ValidationError' in txt or 'ValidationResult' in txt or 'cel' in path.lower()):
            show(path, ['ValidationError', 'ValidationResult', 'Compile'])
PY

echo
echo "== direct module fetch source =="
mkdir -p /tmp/kcel-src
if command -v curl >/dev/null 2>&1; then
  curl -sS --connect-timeout 20 --max-time 60 \
    https://raw.githubusercontent.com/kubernetes/apiextensions-apiserver/v0.32.13/pkg/apiserver/schema/cel/validation.go \
    -o /tmp/kcel-src/validation.go || true
  if [ -s /tmp/kcel-src/validation.go ]; then
    python3 - <<'PY'
from pathlib import Path
lines = Path('/tmp/kcel-src/validation.go').read_text(errors='replace').splitlines()
needles = ['func Compile(', 'Compile(', 'CompileValidationResults', 'ValidationResult', 'ValidationResults']
print('--- validation.go snippets ---')
for i,l in enumerate(lines,1):
    if any(n in l for n in needles):
        a=max(1,i-12); b=min(len(lines),i+35)
        for j in range(a,b+1):
            print(f'{j:4d}\t{lines[j-1]}')
        print()
PY
  fi
fi

Repository: pingcap/tidb-operator

Length of output: 226


🌐 Web query:

pkg.go.dev k8s.io/apiextensions-apiserver v0.32.13 pkg/apiserver/schema/cel Compile CompilationResult

💡 Result:

The CompilationResult type and the Compile function are components of the CEL (Common Expression Language) integration within the k8s.io/apiextensions-apiserver package, specifically located in the pkg/apiserver/schema/cel path [1][2]. These are used to pre-compile and validate x-kubernetes-validations rules defined in Custom Resource Definitions (CRDs) when they are created or updated [3][4][5]. CompilationResult is a struct used to hold the output of the compilation process for a single validation rule [3][6]. Key fields typically include [3]: Program: The compiled CEL program that can be executed during resource validation [3]. Error: Any error encountered during compilation, such as syntax errors or type mismatches [3]. MaxCost: The estimated maximum cost of the compiled expression, used for runtime budget tracking [3]. MaxCardinality: The estimated maximum cardinality, if applicable [3]. TransitionRule: A boolean flag indicating whether the rule uses the oldSelf variable (often used in transition-related validations) [3]. The Compile function is responsible for taking an OpenAPIv3 structural schema and its associated CEL expressions and producing a slice of CompilationResult objects [3][7][6]. It performs several steps [3]: 1. Compiles the CEL expression string into an AST using the provided environment [3]. 2. Validates that the expression evaluates to a boolean type [3]. 3. Pre-checks for specific variables like oldSelf to flag transition rules [3]. 4. Instantiates the CEL program with optimization and cost-tracking options enabled [3]. 5. Estimates the expression's cost to ensure it adheres to configured per-call limits [3]. If compilation fails, the resulting errors are reported during the CRD's creation or update validation, effectively preventing invalid rules from being persisted [3][4][5]. These components are internal to the Kubernetes API extensions server and are essential for safely evaluating CEL expressions against custom resources [8][6].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== check module cache file =="
ls -l "$(go env GOMODCACHE 2>/dev/null)/pkg/mod/k8s.io/apiextensions-apiserver@v0.32.13/pkg/apiserver/schema/cel/validation.go" 2>/dev/null || true

echo
echo "== inspect validation.go source if present =="
python3 - <<'PY'
from pathlib import Path
p = Path('/tmp/kcel-src/validation.go')
if not p.exists():
    import urllib.request
    urllib.request.urlretrieve('https://raw.githubusercontent.com/kubernetes/apiextensions-apiserver/v0.32.13/pkg/apiserver/schema/cel/validation.go', str(p))
lines = p.read_text(errors='replace').splitlines()
# Print around Compile and ValidationResult append statements without assuming line numbers.
for idx, line in enumerate(lines, 1):
    if 'func Compile(' in line or 'CompileValidationResults' in line or 'validationResults' in line or 'append(validationResults' in line or 'ValidationError' in line or 'ValidationResult' in line:
        start=max(1, idx-15); end=min(len(lines), idx+50)
        print(f'--- validation.go {start}-{end} ---')
        for j in range(start, end+1):
            print(f'{j:4d}\t{lines[j-1]}')
        print()
        if idx > 300:
            break

# Also inspect source around all functions named compile* andCELValidations usage.
print('--- source scan ---')
for idx, line in enumerate(lines, 1):
    if 'CELValidations' in line or 'append(validationResults' in line or 'validationResults' in line:
        print(f'{idx:4d}\t{line.strip()}')
PY

echo
echo "== repository CEL rule files around dm/volume claims =="
for f in $(git ls-files |rg 'crd/(core\.pingcap\.com_dm)?(groups|workers|dms|dmworkers)\.yaml|crd/.*dm.*\.yaml'); do
  echo "--- $f ---"
  python3 - <<PY
import re
from pathlib import Path
p = Path('$f')
text = p.read_text()
for m in re.finditer(r'(?P<start>\d+):\s*.*CEL|(?P<start>\d+):\s*x-kubernetes-validations|(?P<start>\d+):\s*.*rule\s*:', text.splitlines(False), re.I):
    pass
lines = text.splitlines()
for i,l in enumerate(lines,1):
    if 'x-kubernetes-validations' in l or 'CEL' in l or 'Validations' in l:
        a=max(1,i-3); b=min(len(lines),i+40)
        for j in range(a,b+1):
            print(f'{j:4d}\t{lines[j-1]}')
        print()
PY
done

Repository: pingcap/tidb-operator

Length of output: 2651


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== CEL validation rule counts =="
python3 - <<'PY'
from pathlib import Path
import re, sys

crds = [
    'crd/core.pingcap.com_dmgroups.yaml',
    'crd/core.pingcap.com_dms.yaml',
    'crd/core.pingcap.com_dmworkergroups.yaml',
    'crd/core.pingcap.com_dmworkers.yaml',
]
for p in crds:
    text = Path(p).read_text()
    fields = [(m.group(1).strip('#').strip('"'), m.start()) for m in re.finditer(r'(^|[\n\r])(\s*(?:^[ \t]*[A-Za-z0-9_\-\[\]]+)?|)\s*#.*', text.splitlines(), re.S)]
    print(f'--- {p} ---')
    count = text.count('x-kubernetes-validations')
    print(f'x-kubernetes-validations occurrences: {count}')
    for m in re.finditer(r'(?:^|\s)message=(.*?)(?:\n|$|\})', text):
        print(f'message: {m.group(1)}')
PY

echo
echo "== source evidence from repository vendored docs if any =="
git ls-files | rg 'vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel|apiextensions-apiserver|cel' || true

Repository: pingcap/tidb-operator

Length of output: 738


🌐 Web query:

Kubernetes apiextensions-apiserver pkg/apiserver/schema/cel validation.go Compile returns CompilationResult slice per CELValidation validation rule

💡 Result:

In the Kubernetes codebase, the Compile function located in k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go is responsible for compiling CEL validation rules defined within Custom Resource Definition (CRD) schemas [1]. When called, the Compile function iterates through the validation rules (specifically those defined in x-kubernetes-validations) and returns a slice of CompilationResult objects [1]. Each CompilationResult corresponds to one of the provided validation rules [1]. A CompilationResult typically contains: - The compiled cel.Program, which is used for execution during object validation [2]. - Any compilation errors (if the expression is syntactically invalid or fails type checking) [1][2]. - Metadata or accessors necessary to evaluate the expression against the object, such as the ExpressionAccessor [2]. This mechanism allows the API server to perform pre-computation of CEL rules at the time of CRD creation or update [1]. By returning a slice of results, the system can independently track the success or failure of each specific validation rule [1]. It is important to distinguish this from other CEL compilation utilities in Kubernetes, such as those used for Validating Admission Policies (k8s.io/apiserver/pkg/admission/plugin/policy/...) or authorization, which may define their own, similar CompilationResult structures tailored to their specific admission or authorization contexts [2][3][4].

Citations:


Prevent an empty CEL result list from passing silently.

cel.Compile can return a valid empty slice when the selected CRD schema has no x-kubernetes-validations rules. In that case, the loop runs zero times and the test passes without checking the unknown volume claim name CEL cost. Add require.NotEmpty(t, results, "no CEL validation rules found") before the loop.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/validation/validation.go` around lines 161 - 162, In the validation
test before the loop over results, add a non-empty assertion for results with
the message “no CEL validation rules found”. Keep the existing per-result
require.Nil check unchanged.

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

Labels

size/L v2 for operator v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants