Browse code

add master-config validation that complains about using troublesome admission chains

deads2k authored on 2016/07/25 23:22:58
Showing 2 changed files
... ...
@@ -181,6 +181,10 @@ func ValidateMasterConfig(config *api.MasterConfig, fldPath *field.Path) Validat
181 181
 
182 182
 	if config.AdmissionConfig.PluginConfig != nil {
183 183
 		validationResults.AddErrors(ValidateAdmissionPluginConfig(config.AdmissionConfig.PluginConfig, fldPath.Child("admissionConfig", "pluginConfig"))...)
184
+		validationResults.Append(ValidateAdmissionPluginConfigConflicts(config))
185
+	}
186
+	if len(config.AdmissionConfig.PluginOrderOverride) != 0 {
187
+		validationResults.AddWarnings(field.Invalid(fldPath.Child("admissionConfig", "pluginOrderOverride"), config.AdmissionConfig.PluginOrderOverride, "specified admission ordering is being phased out.  Convert to DefaultAdmissionConfig in admissionConfig.pluginConfig."))
184 188
 	}
185 189
 
186 190
 	validationResults.Append(ValidateControllerConfig(config.ControllerConfig, fldPath.Child("controllerConfig")))
... ...
@@ -505,8 +509,11 @@ func ValidateKubernetesMasterConfig(config *api.KubernetesMasterConfig, fldPath
505 505
 	if config.AdmissionConfig.PluginConfig != nil {
506 506
 		validationResults.AddErrors(ValidateAdmissionPluginConfig(config.AdmissionConfig.PluginConfig, fldPath.Child("admissionConfig", "pluginConfig"))...)
507 507
 	}
508
+	if len(config.AdmissionConfig.PluginOrderOverride) != 0 {
509
+		validationResults.AddWarnings(field.Invalid(fldPath.Child("admissionConfig", "pluginOrderOverride"), config.AdmissionConfig.PluginOrderOverride, "specified admission ordering is being phased out.  Convert to DefaultAdmissionConfig in admissionConfig.pluginConfig."))
510
+	}
508 511
 
509
-	validationResults.AddErrors(ValidateAPIServerExtendedArguments(config.APIServerArguments, fldPath.Child("apiServerArguments"))...)
512
+	validationResults.Append(ValidateAPIServerExtendedArguments(config.APIServerArguments, fldPath.Child("apiServerArguments")))
510 513
 	validationResults.AddErrors(ValidateControllerExtendedArguments(config.ControllerArguments, fldPath.Child("controllerArguments"))...)
511 514
 
512 515
 	return validationResults
... ...
@@ -581,8 +588,19 @@ func ValidateRoutingConfig(config api.RoutingConfig, fldPath *field.Path) field.
581 581
 	return allErrs
582 582
 }
583 583
 
584
-func ValidateAPIServerExtendedArguments(config api.ExtendedArguments, fldPath *field.Path) field.ErrorList {
585
-	return ValidateExtendedArguments(config, apiserveroptions.NewAPIServer().AddFlags, fldPath)
584
+func ValidateAPIServerExtendedArguments(config api.ExtendedArguments, fldPath *field.Path) ValidationResults {
585
+	validationResults := ValidationResults{}
586
+
587
+	validationResults.AddErrors(ValidateExtendedArguments(config, apiserveroptions.NewAPIServer().AddFlags, fldPath)...)
588
+
589
+	if len(config["admission-control"]) > 0 {
590
+		validationResults.AddWarnings(field.Invalid(fldPath.Key("admission-control"), config["admission-control"], "specified admission ordering is being phased out.  Convert to DefaultAdmissionConfig in admissionConfig.pluginConfig."))
591
+	}
592
+	if len(config["admission-control-config-file"]) > 0 {
593
+		validationResults.AddWarnings(field.Invalid(fldPath.Key("admission-control-config-file"), config["admission-control-config-file"], "specify a single admission control config file is being phased out.  Convert to admissionConfig.pluginConfig, one file per plugin."))
594
+	}
595
+
596
+	return validationResults
586 597
 }
587 598
 
588 599
 func ValidateControllerExtendedArguments(config api.ExtendedArguments, fldPath *field.Path) field.ErrorList {
... ...
@@ -602,3 +620,18 @@ func ValidateAdmissionPluginConfig(pluginConfig map[string]api.AdmissionPluginCo
602 602
 	return allErrs
603 603
 
604 604
 }
605
+
606
+func ValidateAdmissionPluginConfigConflicts(masterConfig *api.MasterConfig) ValidationResults {
607
+	validationResults := ValidationResults{}
608
+
609
+	if masterConfig.KubernetesMasterConfig != nil {
610
+		// check for collisions between openshift and kube plugin config
611
+		for pluginName, kubeConfig := range masterConfig.KubernetesMasterConfig.AdmissionConfig.PluginConfig {
612
+			if openshiftConfig, exists := masterConfig.AdmissionConfig.PluginConfig[pluginName]; exists && !reflect.DeepEqual(kubeConfig, openshiftConfig) {
613
+				validationResults.AddWarnings(field.Invalid(field.NewPath("kubernetesMasterConfig", "admissionConfig", "pluginConfig").Key(pluginName), masterConfig.AdmissionConfig.PluginConfig[pluginName], "conflicts with kubernetesMasterConfig.admissionConfig.pluginConfig.  Separate admission chains are being phased out.  Convert to admissionConfig.pluginConfig."))
614
+			}
615
+		}
616
+	}
617
+
618
+	return validationResults
619
+}
... ...
@@ -5,6 +5,7 @@ import (
5 5
 
6 6
 	kapi "k8s.io/kubernetes/pkg/api"
7 7
 	"k8s.io/kubernetes/pkg/util/diff"
8
+	"k8s.io/kubernetes/pkg/util/sets"
8 9
 	"k8s.io/kubernetes/pkg/util/validation/field"
9 10
 
10 11
 	"github.com/openshift/origin/pkg/cmd/server/api"
... ...
@@ -18,7 +19,8 @@ func TestFailingAPIServerArgs(t *testing.T) {
18 18
 
19 19
 	// [port: invalid value '[invalid-value]': could not be set: strconv.ParseUint: parsing "invalid-value": invalid syntax flag: invalid value 'missing-key': is not a valid flag]
20 20
 
21
-	errs := ValidateAPIServerExtendedArguments(args, nil)
21
+	validationResults := ValidateAPIServerExtendedArguments(args, nil)
22
+	errs := validationResults.Errors
22 23
 
23 24
 	if len(errs) != 2 {
24 25
 		t.Fatalf("expected 2 errors, not %v", errs)
... ...
@@ -248,3 +250,174 @@ func TestValidateAdmissionPluginConfig(t *testing.T) {
248 248
 		}
249 249
 	}
250 250
 }
251
+
252
+func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
253
+	testCases := []struct {
254
+		name    string
255
+		options configapi.MasterConfig
256
+
257
+		warningFields []string
258
+	}{
259
+		{
260
+			name: "stock everything",
261
+		},
262
+		{
263
+			name: "specified kube admission order 01",
264
+			options: configapi.MasterConfig{
265
+				KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
266
+					AdmissionConfig: configapi.AdmissionConfig{
267
+						PluginOrderOverride: []string{"foo"},
268
+					},
269
+				},
270
+			},
271
+			warningFields: []string{"kubernetesMasterConfig.admissionConfig.pluginOrderOverride"},
272
+		},
273
+		{
274
+			name: "specified kube admission order 02",
275
+			options: configapi.MasterConfig{
276
+				KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
277
+					APIServerArguments: configapi.ExtendedArguments{
278
+						"admission-control": []string{"foo"},
279
+					},
280
+				},
281
+			},
282
+			warningFields: []string{"kubernetesMasterConfig.apiServerArguments[admission-control]"},
283
+		},
284
+		{
285
+			name: "specified origin admission order",
286
+			options: configapi.MasterConfig{
287
+				AdmissionConfig: configapi.AdmissionConfig{
288
+					PluginOrderOverride: []string{"foo"},
289
+				},
290
+			},
291
+			warningFields: []string{"admissionConfig.pluginOrderOverride"},
292
+		},
293
+		{
294
+			name: "specified kube admission config file",
295
+			options: configapi.MasterConfig{
296
+				KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
297
+					APIServerArguments: configapi.ExtendedArguments{
298
+						"admission-control-config-file": []string{"foo"},
299
+					},
300
+				},
301
+			},
302
+			warningFields: []string{"kubernetesMasterConfig.apiServerArguments[admission-control-config-file]"},
303
+		},
304
+		{
305
+			name: "specified, non-conflicting plugin configs 01",
306
+			options: configapi.MasterConfig{
307
+				AdmissionConfig: configapi.AdmissionConfig{
308
+					PluginConfig: map[string]configapi.AdmissionPluginConfig{
309
+						"foo": {
310
+							Location: "bar",
311
+						},
312
+					},
313
+				},
314
+			},
315
+		},
316
+		{
317
+			name: "specified, non-conflicting plugin configs 02",
318
+			options: configapi.MasterConfig{
319
+				KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
320
+					AdmissionConfig: configapi.AdmissionConfig{
321
+						PluginConfig: map[string]configapi.AdmissionPluginConfig{
322
+							"foo": {
323
+								Location: "bar",
324
+							},
325
+							"third": {
326
+								Location: "bar",
327
+							},
328
+						},
329
+					},
330
+				},
331
+				AdmissionConfig: configapi.AdmissionConfig{
332
+					PluginConfig: map[string]configapi.AdmissionPluginConfig{
333
+						"foo": {
334
+							Location: "bar",
335
+						},
336
+					},
337
+				},
338
+			},
339
+		},
340
+		{
341
+			name: "specified, non-conflicting plugin configs 03",
342
+			options: configapi.MasterConfig{
343
+				KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
344
+					AdmissionConfig: configapi.AdmissionConfig{
345
+						PluginConfig: map[string]configapi.AdmissionPluginConfig{
346
+							"foo": {
347
+								Location: "bar",
348
+							},
349
+							"third": {
350
+								Location: "bar",
351
+							},
352
+						},
353
+					},
354
+				},
355
+			},
356
+		},
357
+		{
358
+			name: "specified conflicting plugin configs 01",
359
+			options: configapi.MasterConfig{
360
+				KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
361
+					AdmissionConfig: configapi.AdmissionConfig{
362
+						PluginConfig: map[string]configapi.AdmissionPluginConfig{
363
+							"foo": {
364
+								Location: "different",
365
+							},
366
+						},
367
+					},
368
+				},
369
+				AdmissionConfig: configapi.AdmissionConfig{
370
+					PluginConfig: map[string]configapi.AdmissionPluginConfig{
371
+						"foo": {
372
+							Location: "bar",
373
+						},
374
+					},
375
+				},
376
+			},
377
+			warningFields: []string{"kubernetesMasterConfig.admissionConfig.pluginConfig[foo]"},
378
+		},
379
+	}
380
+
381
+	// these fields have warnings in the empty case
382
+	defaultWarningFields := sets.NewString(
383
+		"serviceAccountConfig.managedNames", "serviceAccountConfig.publicKeyFiles", "serviceAccountConfig.privateKeyFile", "serviceAccountConfig.masterCA",
384
+		"projectConfig.securityAllocator", "kubernetesMasterConfig.proxyClientInfo")
385
+
386
+	for _, tc := range testCases {
387
+		results := ValidateMasterConfig(&tc.options, nil)
388
+
389
+		for _, result := range results.Warnings {
390
+			if defaultWarningFields.Has(result.Field) {
391
+				continue
392
+			}
393
+
394
+			found := false
395
+			for _, expectedField := range tc.warningFields {
396
+				if result.Field == expectedField {
397
+					found = true
398
+					break
399
+				}
400
+			}
401
+
402
+			if !found {
403
+				t.Errorf("%s: didn't expect %q", tc.name, result.Field)
404
+			}
405
+		}
406
+
407
+		for _, expectedField := range tc.warningFields {
408
+			found := false
409
+			for _, result := range results.Warnings {
410
+				if result.Field == expectedField {
411
+					found = true
412
+					break
413
+				}
414
+			}
415
+
416
+			if !found {
417
+				t.Errorf("%s: didn't find %q", tc.name, expectedField)
418
+			}
419
+		}
420
+	}
421
+}