Skip to content

Conversation

@psav
Copy link
Collaborator

@psav psav commented Jul 7, 2025

Add ClowdApp Validating Webhook for Duplicate Name Prevention

Summary

Implements a validating webhook for ClowdApp resources to prevent duplicate names within the same ClowdEnvironment. This ensures logical uniqueness of ClowdApp names while allowing the same name to be used across different ClowdEnvironments.

What Changed

🔧 Webhook Implementation

  • Added clowdAppValidator struct with comprehensive validation logic
  • Implemented duplicate name validation that checks for existing ClowdApps with the same name in the same ClowdEnvironment
  • Added field indexing for spec.envName to enable efficient webhook queries
  • Homogenized validation function signatures to use consistent func(context.Context, client.Client, *ClowdApp) field.ErrorList pattern

📝 Validation Logic

  • Duplicate Name Prevention: Prevents ClowdApps with the same name in the same ClowdEnvironment
  • Cross-Environment Support: Allows same names across different ClowdEnvironments
  • Smart Update Logic: Skips duplicate validation on updates when name unchanged
  • Error Handling: Graceful handling of API server issues with logging

🧪 Test Coverage

  • Unit Tests: 6 comprehensive webhook suite tests covering:
    • Unique name creation (should pass)
    • Same name in different ClowdEnvironments (should pass)
    • Duplicate name in same ClowdEnvironment (should fail)
    • Update without name change (should pass)
    • Database and sidecar validation (should fail appropriately)
  • Integration Tests: Kuttl test suite with proper error validation pattern

📚 Documentation Updates

  • Updated docs/clowder-design.md with validation section
  • Documented all validation types: duplicate name, database config, sidecars, deployment strategy
  • Clarified ClowdEnvironment-scoped uniqueness requirement

Technical Details

Validation Functions

All validation functions now use the same signature for consistency:

func validateX(ctx context.Context, c client.Client, r *ClowdApp) field.ErrorList

Field Indexing

Added spec.envName field indexing in SetupWebhookWithManager to enable efficient queries:

mgr.GetFieldIndexer().IndexField(context.TODO(), &ClowdApp{}, "spec.envName", ...)

Webhook Logic

The webhook validates on both CREATE and UPDATE operations:

  • CREATE: Always runs duplicate name validation + default validations
  • UPDATE: Only runs duplicate name validation if the name changed

Business Logic

  • ✅ Allowed: ClowdApps with same name in different ClowdEnvironments
  • ❌ Prevented: ClowdApps with same name in same ClowdEnvironment (even across namespaces)
  • 💡 Rationale: ClowdEnvironment is the logical grouping unit, not Kubernetes namespace

Files Modified

  • apis/cloud.redhat.com/v1alpha1/clowdapp_webhook.go - Main webhook implementation
  • apis/cloud.redhat.com/v1alpha1/webhook_suite_test.go - Unit tests
  • tests/kuttl/test-duplicate-name-validation/ - Integration tests (4 files)
  • docs/clowder-design.md - Documentation updates

Testing

  • ✅ All existing tests pass
  • ✅ New webhook suite tests pass (6/6)
  • ✅ Kuttl integration tests validate proper error handling
  • make test passes without errors

Breaking Changes

None - this is purely additive validation that prevents invalid configurations.

  • AI (Cursor) assisted in webhook
  • AI (Cursor) authored kuttl test
  • AI (Cursor) PR summary

@maknop maknop self-requested a review July 8, 2025 14:40
@psav psav force-pushed the psav/fix-dup-name branch from 02bf8dc to 04bd6d6 Compare July 8, 2025 14:49
@maknop
Copy link
Contributor

maknop commented Jul 8, 2025

/retest

- AI (Cursor) assisted in webhook
- AI (Cursor) authored kuttl test
@psav psav force-pushed the psav/fix-dup-name branch from 04bd6d6 to f23123d Compare July 8, 2025 15:58
maknop
maknop previously approved these changes Jul 15, 2025
@maknop
Copy link
Contributor

maknop commented Jul 22, 2025

/retest

@maknop maknop self-requested a review August 12, 2025 19:05
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.

2 participants