Skip to content

feat(recipe): extract Validation as standalone type with hybrid resource pattern#830

Merged
mchmarny merged 10 commits into
NVIDIA:mainfrom
xdu31:feature/issue-732-extract-validation-type
May 12, 2026
Merged

feat(recipe): extract Validation as standalone type with hybrid resource pattern#830
mchmarny merged 10 commits into
NVIDIA:mainfrom
xdu31:feature/issue-732-extract-validation-type

Conversation

@xdu31

@xdu31 xdu31 commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Extracts Validation as a standalone type from RecipeResult and implements a hybrid resource pattern that supports both standalone file usage (with full Kubernetes resource metadata) and clean embedding in Custom Resources (with metadata auto-omitted via omitempty tags).

Motivation/Context

Fixes #732

The Validation configuration was previously embedded within RecipeResult, making it difficult to:

  • Use validation specs independently of recipe generation
  • Embed validation configs cleanly in Kubernetes CRs without redundant metadata
  • Share validation types across CLI and API boundaries

This change enables the same Validation type to work seamlessly in both contexts:

  1. Standalone files (validation.yaml) with full K8s resource metadata
  2. Embedded in CRs with clean spec-only embedding (no duplicate metadata)

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Refactoring (no functional changes, code improvement)

Components Affected

  • pkg/recipe - New validation.go and validation_test.go
  • pkg/validator - Updated to use standalone Validation type
  • pkg/cli - Updated validate command
  • validators/* - 16 validator implementations updated

Implementation Notes

New Files

  • pkg/recipe/validation.go (161 lines) - Standalone Validation type with:

    • Optional resource metadata fields (APIVersion, Kind, Metadata) with omitempty
    • Custom ValidationMetadata type (lighter than metav1.ObjectMeta)
    • KindValidation constant for type safety
    • ToValidation() converter from RecipeResult
    • NewValidation() constructor

  • pkg/recipe/validation_test.go (310 lines) - Comprehensive test coverage:

    • Conversion tests (ToValidation)
    • JSON/YAML marshaling tests
    • omitempty behavior verification
    • CR embedding simulation tests

Key Design Decisions

  1. No nested Spec wrapper - Avoids K8s anti-pattern of spec.validation.spec
  2. Custom metadata type - ValidationMetadata instead of heavy metav1.ObjectMeta
  3. Pointer for Metadata - Enables clean nil check and omitempty behavior
  4. All fields optional except ValidationConfig - Maximum flexibility for different contexts
  5. KindValidation constant - Type-safe kind string (addresses goconst linter)

Removed from RecipeResult

  • Validation *ValidationConfig field moved to standalone type
  • Reduced coupling between recipe generation and validation

Testing

  • ✅ All unit tests passing (75.3% coverage)
  • ✅ pkg/recipe coverage: 90.6%
  • ✅ New validation_test.go: 7 comprehensive tests
  • ✅ All 18 E2E tests passing
  • ✅ golangci-lint clean (goconst, gofmt addressed)
  • ✅ make qualify: PASS

Test Coverage Details

New tests verify:

  • RecipeResult → Validation conversion with metadata population
  • Nil safety
  • JSON serialization/deserialization
  • YAML serialization
  • omitempty behavior (clean when fields empty)
  • CR embedding without redundant metadata

Risk Assessment

Low Risk

  • Purely additive change - new standalone type
  • All existing code updated and tested
  • No breaking API changes
  • Backward compatible with existing RecipeResult usage

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Code commented where needed (type/function docs)
  • Documentation updated (godoc comments)
  • No new warnings generated
  • Tests added for new functionality
  • All tests passing locally
  • make qualify passed
  • Dependent changes merged

@xdu31 xdu31 requested a review from a team as a code owner May 9, 2026 16:49
@coderabbitai

This comment was marked as resolved.

Sign in to view
coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

@xdu31 xdu31 requested a review from a team as a code owner May 10, 2026 00:30
xdu31 added a commit to xdu31/aicr that referenced this pull request May 10, 2026
@xdu31
fix(ci): set AICR_VALIDATOR_IMAGE_TAG=latest in E2E workflow
df4cae7 
The E2E action builds validator images with :latest tag but the validator
catalog resolution defaults to :sha-<commit> when AICR_VALIDATOR_IMAGE_TAG
is not set. This causes ImagePullBackOff errors and validator jobs to report
'pod not found' with status 'other'.

Setting AICR_VALIDATOR_IMAGE_TAG=latest ensures the validator jobs use the
locally-built images that are available in the Kind registry.

Fixes the E2E test failures in NVIDIA#830 where all validators were reporting
status 'other' instead of pass/fail.
@xdu31 xdu31 force-pushed the feature/issue-732-extract-validation-type branch 2 times, most recently from df4cae7 to 17778ce Compare May 10, 2026 01:43
@github-actions github-actions Bot removed the area/ci label May 10, 2026
coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

@xdu31 xdu31 force-pushed the feature/issue-732-extract-validation-type branch from 17778ce to 6440b40 Compare May 10, 2026 01:56
coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

mchmarny

This comment was marked as resolved.

Sign in to view

@mchmarny mchmarny mentioned this pull request May 11, 2026
Merged
10 tasks
@xdu31 xdu31 force-pushed the feature/issue-732-extract-validation-type branch from 00751ff to a336a2d Compare May 11, 2026 23:25
@github-actions

github-actions Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

@xdu31 this PR now has merge conflicts with main. Please rebase to resolve them.

xdu31 added 6 commits May 11, 2026 16:47
@xdu31
refactor(recipe): extract Validation as standalone type for issue NVI…
e08efa9 
…DIA#732
@xdu31
feat(recipe): add optional APIVersion/Kind/Metadata to Validation (hy…
1848c79 
…brid approach)
@xdu31
feat(recipe): add optional resource metadata to Validation type
175c1ea 
Make Validation type support both standalone file usage and embedding in
Kubernetes CRs by adding optional APIVersion/Kind/Metadata fields with
omitempty tags.

Changes:
- Add APIVersion, Kind, and Metadata fields to Validation struct
- Use custom ValidationMetadata type instead of metav1.ObjectMeta
- Add omitempty tags to all optional fields (APIVersion, Kind, Metadata,
  ComponentRefs, Criteria, Constraints)
- Update ToValidation() to populate resource metadata from RecipeResult
- Update NewValidation() to initialize with zero values
- Update tests to verify resource metadata handling

This enables clean embedding without redundant metadata when used in CRs
while supporting standalone validation.yaml files with full resource
metadata.
@xdu31
fix(recipe): address golangci-lint issues in Validation
8f6a7e3 
- Add KindValidation constant to fix goconst warning
- Format files with gofmt to fix formatting issues
- Update tests to use constant instead of string literal
@xdu31
fix(recipe): format validation_test.go with gofmt
77fea80
@xdu31
fix(validator): update ConfigMap references from recipe to validation
3163c93 
The PR changed validator.go to store Validation in ConfigMap with name
aicr-validation-<runID> and key validation.yaml, but deployer.go and
validators/context.go were not updated to match.

This caused 'pod not found for Job' error because:
- validator.go creates ConfigMap: aicr-validation-<runID>
- deployer.go tries to mount: aicr-recipe-<runID> (doesn't exist)
- Kubernetes cannot create Pod without the required ConfigMap

Changes:
- deployer.go: Mount aicr-validation-* ConfigMap at /data/validation/
- deployer.go: Update env var AICR_RECIPE_PATH → AICR_VALIDATION_PATH
- validators/context.go: Load Validation directly instead of RecipeResult

This completes the Validation extraction by updating all references to
use the new ConfigMap name and structure.
@xdu31 xdu31 force-pushed the feature/issue-732-extract-validation-type branch from a336a2d to 3163c93 Compare May 11, 2026 23:48
@xdu31 xdu31 requested a review from mchmarny May 12, 2026 06:49
@mchmarny mchmarny enabled auto-merge (squash) May 12, 2026 16:24
@mchmarny mchmarny disabled auto-merge May 12, 2026 16:24
@mchmarny mchmarny enabled auto-merge (squash) May 12, 2026 16:57
@mchmarny mchmarny merged commit ee2e7ae into NVIDIA:main May 12, 2026
30 checks passed
@njhensley njhensley mentioned this pull request May 13, 2026
Closed
@mchmarny mchmarny mentioned this pull request May 23, 2026
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@mchmarny mchmarny mchmarny approved these changes

Assignees

@xdu31 xdu31

Labels

area/api area/cli area/validator size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extract Validation as standalone portable type from pkg/recipe

2 participants

@xdu31 @mchmarny