Skip to content

Commit d33b34f

Browse files
patrykw-splunkPatryk Wasielewski
andauthored
Add comprehensive webhook validations for CRD configuration (#1762)
* Add validation webhook for appRepo fields - Add appsRepoPollInterval validation: - Default: 0 (disabled) - Minimum: 0, Maximum: 86400 (1 day) - Values between 1-59 are rejected (must be 0 or >= 60) - Add appSources uniqueness validation: - Location + Scope combination must be unique across appSources - Uses defaults.scope when scope is not specified in appSource - volumeName is NOT part of uniqueness check - Add unit tests for all new validations * Add premiumAppsProps validation for appSources - Validate that premiumAppsProps.type is required when scope is 'premiumApps' - Check both source-level and defaults-level premiumAppsProps.type - Add unit tests for premiumAppsProps validation scenarios * Add extraEnv uniqueness validation - Validate that environment variable names in spec.extraEnv are unique - Report duplicate names with reference to the first occurrence - Add unit tests for extraEnv uniqueness validation * Add imagePullSecrets uniqueness validation - Validate that secret names in spec.imagePullSecrets are unique - Report duplicate names with reference to the first occurrence - Add unit tests for imagePullSecrets uniqueness validation * Add probe fields validation - Validate livenessProbe, readinessProbe, and startupProbe configurations - initialDelaySeconds: minimum 0 - timeoutSeconds: minimum 1 - periodSeconds: minimum 1 - failureThreshold: minimum 1 - Add unit tests for probe validation scenarios * Add resource requirements validation - Validate that memory request does not exceed memory limit - Validate that cpu request does not exceed cpu limit - Add unit tests for resource requirements validation * Add SmartStore validation for volumes and indexes - Validate that indexes require at least one volume to be configured - Validate that index volumeName references an existing volume in volumes list - Validate that index has volumeName or defaults.volumeName provided - Update and add unit tests for SmartStore validation * Add StorageClassSpec ephemeralStorage mutual exclusivity validation - Validate that ephemeralStorage is mutually exclusive with storageClassName - Validate that ephemeralStorage is mutually exclusive with storageCapacity - Add unit tests for ephemeralStorage mutual exclusivity scenarios * feat(validation): add imagePullSecrets existence validation - Add ValidationContext to carry Kubernetes client for resource lookups - Extend Validator interface with context-aware validation methods - Implement ValidateImagePullSecretsExistence to verify secrets exist - Update all CRD validators with context-aware validation functions - Pass manager client to webhook server for API access - Update ValidationWebhook.md documentation with new validation rules - Add unit tests for imagePullSecrets existence validation This enables the webhook to reject CRs that reference non-existent secrets in spec.imagePullSecrets, providing early feedback to users. --------- Co-authored-by: Patryk Wasielewski <pwasiele@splunk.com>
1 parent 80bafa5 commit d33b34f

15 files changed

Lines changed: 1341 additions & 44 deletions

cmd/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ func main() {
303303
Validators: validation.DefaultValidators,
304304
ReadTimeout: readTimeout,
305305
WriteTimeout: writeTimeout,
306+
Client: mgr.GetClient(),
306307
})
307308

308309
// Add webhook server as a runnable to the manager

docs/ValidationWebhook.md

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,16 @@ The webhook validates the following spec fields:
8383
| `spec.varVolumeStorageConfig.storageCapacity` | Must match format `^[0-9]+Gi$` | must be in Gi format (e.g., '10Gi', '100Gi') |
8484
| `spec.etcVolumeStorageConfig.storageClassName` | Required when `ephemeralStorage=false` and `storageCapacity` is set | storageClassName is required when using persistent storage |
8585
| `spec.varVolumeStorageConfig.storageClassName` | Required when `ephemeralStorage=false` and `storageCapacity` is set | storageClassName is required when using persistent storage |
86+
| `spec.etcVolumeStorageConfig.ephemeralStorage` | Mutually exclusive with `storageClassName` and `storageCapacity` | storageClassName/storageCapacity cannot be set when ephemeralStorage is true |
87+
| `spec.varVolumeStorageConfig.ephemeralStorage` | Mutually exclusive with `storageClassName` and `storageCapacity` | storageClassName/storageCapacity cannot be set when ephemeralStorage is true |
88+
| `spec.extraEnv[*].name` | Must be unique across all entries | duplicate environment variable |
89+
| `spec.imagePullSecrets[*].name` | Must be unique across all entries | duplicate secret reference |
90+
| `spec.imagePullSecrets[*].name` | Must reference an existing Secret in the namespace | not found |
91+
| `spec.livenessProbe.initialDelaySeconds` | Must be ≥ 0 | must be non-negative |
92+
| `spec.readinessProbe.initialDelaySeconds` | Must be ≥ 0 | must be non-negative |
93+
| `spec.startupProbe.initialDelaySeconds` | Must be ≥ 0 | must be non-negative |
94+
| `spec.resources.requests.cpu` | Must be ≤ `limits.cpu` | request must be less than or equal to limit |
95+
| `spec.resources.requests.memory` | Must be ≤ `limits.memory` | request must be less than or equal to limit |
8696

8797
### CRD-Specific Fields
8898

@@ -111,6 +121,9 @@ AppFramework configuration is validated only when provided:
111121
|-------|-----------------|
112122
| `spec.appRepo.appSources[*].name` | Required (non-empty) |
113123
| `spec.appRepo.appSources[*].location` | Required (non-empty) |
124+
| `spec.appRepo.appSources[*]` | Combination of `location` + `scope` must be unique across all appSources |
125+
| `spec.appRepo.appSources[*].premiumAppsProps` | Required when `scope=premiumApps` |
126+
| `spec.appRepo.appsRepoPollIntervalSeconds` | Must be ≥ 0 |
114127
| `spec.appRepo.volumes[*].name` | Required (non-empty) |
115128

116129
## Example Validation Errors
@@ -167,6 +180,42 @@ Error:
167180
The Standalone "example" is invalid: spec.smartstore.volumes[0].name: Required value: volume name is required
168181
```
169182
183+
### Non-Existent ImagePullSecret
184+
185+
```yaml
186+
apiVersion: enterprise.splunk.com/v4
187+
kind: Standalone
188+
metadata:
189+
name: example
190+
namespace: splunk
191+
spec:
192+
imagePullSecrets:
193+
- name: my-registry-secret # Invalid: secret does not exist in namespace
194+
```
195+
196+
Error:
197+
```
198+
The Standalone "example" is invalid: spec.imagePullSecrets[0].name: Not found: "my-registry-secret"
199+
```
200+
201+
### Ephemeral Storage with StorageClassName
202+
203+
```yaml
204+
apiVersion: enterprise.splunk.com/v4
205+
kind: Standalone
206+
metadata:
207+
name: example
208+
spec:
209+
etcVolumeStorageConfig:
210+
ephemeralStorage: true
211+
storageClassName: "standard" # Invalid: cannot set with ephemeralStorage=true
212+
```
213+
214+
Error:
215+
```
216+
The Standalone "example" is invalid: spec.etcVolumeStorageConfig.storageClassName: Invalid value: "standard": storageClassName cannot be set when ephemeralStorage is true
217+
```
218+
170219
## Verifying Webhook Deployment
171220
172221
### Check Webhook Pod is Running
@@ -264,6 +313,121 @@ The validation webhook consists of:
264313
5. Webhook returns Allowed/Denied response
265314
6. If allowed, the resource is persisted; if denied, user receives error
266315

316+
## Adding a New CRD to the Webhook
317+
318+
To add validation for a new CRD, follow these steps:
319+
320+
### 1. Create Validation Functions
321+
322+
Create a new file `pkg/splunk/enterprise/validation/<crd>_validation.go`:
323+
324+
```go
325+
package validation
326+
327+
import (
328+
"k8s.io/apimachinery/pkg/util/validation/field"
329+
enterpriseApi "github.com/splunk/splunk-operator/api/v4"
330+
)
331+
332+
// Validate<CRD>Create validates a <CRD> on CREATE
333+
func Validate<CRD>Create(obj *enterpriseApi.<CRD>) field.ErrorList {
334+
var allErrs field.ErrorList
335+
// Add validation logic
336+
allErrs = append(allErrs, validateCommonSplunkSpec(&obj.Spec.CommonSplunkSpec, field.NewPath("spec"))...)
337+
return allErrs
338+
}
339+
340+
// Validate<CRD>CreateWithContext validates with access to Kubernetes API
341+
// Use this for validations that need to check if resources exist (e.g., Secrets)
342+
func Validate<CRD>CreateWithContext(obj *enterpriseApi.<CRD>, vc *ValidationContext) field.ErrorList {
343+
allErrs := Validate<CRD>Create(obj)
344+
if len(obj.Spec.ImagePullSecrets) > 0 {
345+
allErrs = append(allErrs, ValidateImagePullSecretsExistence(
346+
obj.Spec.ImagePullSecrets, vc, field.NewPath("spec").Child("imagePullSecrets"))...)
347+
}
348+
return allErrs
349+
}
350+
351+
// Validate<CRD>Update validates a <CRD> on UPDATE
352+
func Validate<CRD>Update(obj, oldObj *enterpriseApi.<CRD>) field.ErrorList {
353+
return Validate<CRD>Create(obj)
354+
}
355+
356+
// Validate<CRD>UpdateWithContext validates on UPDATE with Kubernetes API access
357+
func Validate<CRD>UpdateWithContext(obj, oldObj *enterpriseApi.<CRD>, vc *ValidationContext) field.ErrorList {
358+
return Validate<CRD>CreateWithContext(obj, vc)
359+
}
360+
361+
// Get<CRD>WarningsOnCreate returns warnings for CREATE
362+
func Get<CRD>WarningsOnCreate(obj *enterpriseApi.<CRD>) []string {
363+
return getCommonWarnings(&obj.Spec.CommonSplunkSpec)
364+
}
365+
366+
// Get<CRD>WarningsOnUpdate returns warnings for UPDATE
367+
func Get<CRD>WarningsOnUpdate(obj, oldObj *enterpriseApi.<CRD>) []string {
368+
return Get<CRD>WarningsOnCreate(obj)
369+
}
370+
```
371+
372+
### 2. Register the Validator
373+
374+
Add the GVR and validator to `pkg/splunk/enterprise/validation/registry.go`:
375+
376+
```go
377+
// Add GVR constant
378+
var <CRD>GVR = schema.GroupVersionResource{
379+
Group: "enterprise.splunk.com",
380+
Version: "v4",
381+
Resource: "<crd>s", // plural, lowercase
382+
}
383+
384+
// Add to DefaultValidators map
385+
var DefaultValidators = map[schema.GroupVersionResource]Validator{
386+
// ... existing validators ...
387+
388+
<CRD>GVR: &GenericValidator[*enterpriseApi.<CRD>]{
389+
ValidateCreateFunc: Validate<CRD>Create,
390+
ValidateUpdateFunc: Validate<CRD>Update,
391+
ValidateCreateWithContextFunc: Validate<CRD>CreateWithContext, // Optional: for resource lookups
392+
ValidateUpdateWithContextFunc: Validate<CRD>UpdateWithContext, // Optional: for resource lookups
393+
WarningsOnCreateFunc: Get<CRD>WarningsOnCreate,
394+
WarningsOnUpdateFunc: Get<CRD>WarningsOnUpdate,
395+
GroupKind: schema.GroupKind{
396+
Group: "enterprise.splunk.com",
397+
Kind: "<CRD>",
398+
},
399+
},
400+
}
401+
```
402+
403+
### 3. Add Unit Tests
404+
405+
Create `pkg/splunk/enterprise/validation/<crd>_validation_test.go` with test cases.
406+
407+
### 4. Update ValidatingWebhookConfiguration
408+
409+
Add the new resource to `config/webhook/manifests.yaml`:
410+
411+
```yaml
412+
webhooks:
413+
- name: validate.enterprise.splunk.com
414+
rules:
415+
- apiGroups: ["enterprise.splunk.com"]
416+
apiVersions: ["v4"]
417+
operations: ["CREATE", "UPDATE"]
418+
resources:
419+
- standalones
420+
- indexerclusters
421+
- <crd>s # Add new resource here
422+
```
423+
424+
### Context-Aware vs Basic Validation
425+
426+
- **Basic validation** (`ValidateCreateFunc`): For validations that only need the CR itself (field formats, required fields, cross-field rules)
427+
- **Context-aware validation** (`ValidateCreateWithContextFunc`): For validations that need to query the Kubernetes API (checking if Secrets, ConfigMaps, or other resources exist)
428+
429+
If your CRD doesn't need context-aware validation, you can omit `ValidateCreateWithContextFunc` and `ValidateUpdateWithContextFunc` - the webhook will automatically fall back to the basic validation functions.
430+
267431
## Disabling the Webhook
268432

269433
To disable the webhook after it has been enabled:

pkg/splunk/enterprise/validation/clustermanager_validation.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,27 @@ func ValidateClusterManagerCreate(obj *enterpriseApi.ClusterManager) field.Error
4242
return allErrs
4343
}
4444

45+
// ValidateClusterManagerCreateWithContext validates a ClusterManager on CREATE with ValidationContext
46+
func ValidateClusterManagerCreateWithContext(obj *enterpriseApi.ClusterManager, vc *ValidationContext) field.ErrorList {
47+
allErrs := ValidateClusterManagerCreate(obj)
48+
if len(obj.Spec.ImagePullSecrets) > 0 {
49+
allErrs = append(allErrs, ValidateImagePullSecretsExistence(
50+
obj.Spec.ImagePullSecrets, vc, field.NewPath("spec").Child("imagePullSecrets"))...)
51+
}
52+
return allErrs
53+
}
54+
4555
// ValidateClusterManagerUpdate validates a ClusterManager on UPDATE
4656
// TODO: Add immutable field validation here (e.g., compare obj vs oldObj for fields that cannot change after creation)
4757
func ValidateClusterManagerUpdate(obj, oldObj *enterpriseApi.ClusterManager) field.ErrorList {
4858
return ValidateClusterManagerCreate(obj)
4959
}
5060

61+
// ValidateClusterManagerUpdateWithContext validates a ClusterManager on UPDATE with ValidationContext
62+
func ValidateClusterManagerUpdateWithContext(obj, oldObj *enterpriseApi.ClusterManager, vc *ValidationContext) field.ErrorList {
63+
return ValidateClusterManagerCreateWithContext(obj, vc)
64+
}
65+
5166
// GetClusterManagerWarningsOnCreate returns warnings for ClusterManager CREATE
5267
func GetClusterManagerWarningsOnCreate(obj *enterpriseApi.ClusterManager) []string {
5368
return getCommonWarnings(&obj.Spec.CommonSplunkSpec)

0 commit comments

Comments
 (0)