Merge pull request #2 from mesosphere/fix-trial-metadata-substitution…

…-check

Don't check if trial's metadata in experiment parameters

pkg/controller.v1beta1/consts/const.go

@@ -179,4 +179,14 @@ var (
 	DefaultKatibDBManagerServiceIP = env.GetEnvOrDefault(DefaultKatibDBManagerServiceIPEnvName, "katib-db-manager")
 	// DefaultKatibDBManagerServicePort is the default Port of Katib DB Manager
 	DefaultKatibDBManagerServicePort = env.GetEnvOrDefault(DefaultKatibDBManagerServicePortEnvName, "6789")
+
+	// List of all valid keys of trial metadata for substitution in Trial template
+	TrialTemplateMetaKeys = []string{
+		TrialTemplateMetaKeyOfName,
+		TrialTemplateMetaKeyOfNamespace,
+		TrialTemplateMetaKeyOfKind,
+		TrialTemplateMetaKeyOfAPIVersion,
+		TrialTemplateMetaKeyOfAnnotations,
+		TrialTemplateMetaKeyOfLabels,
+	}
 )

pkg/webhook/v1beta1/experiment/validator/validator.go

@@ -311,8 +311,13 @@ func (g *DefaultValidator) validateTrialTemplate(instance *experimentsv1beta1.Ex
 
 		// Check if parameter reference exist in experiment parameters
 		if len(experimentParameterNames) > 0 {
-			if _, ok := experimentParameterNames[parameter.Reference]; !ok {
-				return fmt.Errorf("parameter reference %v does not exist in spec.parameters: %v", parameter.Reference, instance.Spec.Parameters)
+			// Check if parameter is trial metadata
+			regex := regexp.MustCompile(consts.TrialTemplateMetaReplaceFormatRegex)
+			match := regex.FindStringSubmatch(parameter.Reference)
+			if !(len(match) > 0 && contains(consts.TrialTemplateMetaKeys, match[1])) {
+				if _, ok := experimentParameterNames[parameter.Reference]; !ok {
+					return fmt.Errorf("parameter reference %v does not exist in spec.parameters: %v", parameter.Reference, instance.Spec.Parameters)
+				}
 			}
 		}
 
@@ -481,3 +486,12 @@ func (g *DefaultValidator) validateMetricsCollector(inst *experimentsv1beta1.Exp
 
 	return nil
 }
+
+func contains(slice []string, item string) bool {
+	for _, s := range slice {
+		if s == item {
+			return true
+		}
+	}
+	return false
+}

pkg/webhook/v1beta1/experiment/validator/validator_test.go

@@ -415,6 +415,7 @@ spec:
 	validTemplate4 := p.EXPECT().GetTrialTemplate(gomock.Any()).Return(validJobStr, nil)
 	validTemplate5 := p.EXPECT().GetTrialTemplate(gomock.Any()).Return(validJobStr, nil)
 	validTemplate6 := p.EXPECT().GetTrialTemplate(gomock.Any()).Return(validJobStr, nil)
+	validTemplate7 := p.EXPECT().GetTrialTemplate(gomock.Any()).Return(validJobStr, nil)
 
 	missedParameterTemplate := p.EXPECT().GetTrialTemplate(gomock.Any()).Return(missedParameterJobStr, nil)
 	oddParameterTemplate := p.EXPECT().GetTrialTemplate(gomock.Any()).Return(oddParameterJobStr, nil)
@@ -431,6 +432,7 @@ spec:
 		validTemplate4,
 		validTemplate5,
 		validTemplate6,
+		validTemplate7,
 		missedParameterTemplate,
 		oddParameterTemplate,
 		invalidParameterTemplate,
@@ -570,6 +572,16 @@ spec:
 			Err:             false,
 			testDescription: "Trial template contains Trial parameters when spec.parameters is empty",
 		},
+		// Trial template contains Trial metadata parameter substitution
+		{
+			Instance: func() *experimentsv1beta1.Experiment {
+				i := newFakeInstance()
+				i.Spec.TrialTemplate.TrialParameters[1].Reference = "${trialSpec.Name}"
+				return i
+			}(),
+			Err:             false,
+			testDescription: "Trial template contains Trial metadata reference as parameter",
+		},
 		// Trial Template doesn't contain parameter from trialParameters
 		// missedParameterTemplate case
 		{