From 108b78d485be2dbe0b8e69c3f2fb3633eeb8b243 Mon Sep 17 00:00:00 2001
From: Kristine Kunapuli <kristine.kunapuli@hpe.com>
Date: Thu, 5 Sep 2024 14:32:59 -0400
Subject: [PATCH 1/4] chore: add unmarshal functions for task config policies

---
 master/internal/api_config_policies.go        |  41 +--
 .../configpolicy/task_config_policy.go        |   4 +-
 master/internal/configpolicy/utils.go         |  60 ++++
 master/internal/configpolicy/utils_test.go    | 299 ++++++++++++++++++
 4 files changed, 370 insertions(+), 34 deletions(-)
 create mode 100644 master/internal/configpolicy/utils.go
 create mode 100644 master/internal/configpolicy/utils_test.go

diff --git a/master/internal/api_config_policies.go b/master/internal/api_config_policies.go
index 140f786330a..d41744caa20 100644
--- a/master/internal/api_config_policies.go
+++ b/master/internal/api_config_policies.go
@@ -7,31 +7,13 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 	"gopkg.in/yaml.v3"
 
+	"github.com/determined-ai/determined/master/internal/configpolicy"
 	"github.com/determined-ai/determined/proto/pkg/apiv1"
 )
 
-// workload types are used to distinguish configuration for experiments and NTSCs.
-const (
-	// Should not be used.
-	WorkloadTypeUnspecified = "UNKNOWN"
-	// Configuration policies for experiments.
-	WorkloadTypeExperiment = "EXPERIMENT"
-	// Configuration policies for NTSC.
-	WorkloadTypeNTSC = "NTSC"
-)
-
-func validWorkloadEnum(val string) bool {
-	switch val {
-	case WorkloadTypeExperiment, WorkloadTypeNTSC:
-		return true
-	default:
-		return false
-	}
-}
-
 func stubData() (*structpb.Struct, error) {
 	const yamlString = `
-invariant_configs:
+invariant_config:
   description: "test"
 constraints:
   resources:
@@ -44,19 +26,14 @@ constraints:
 		return nil, fmt.Errorf("unable to unmarshal yaml: %w", err)
 	}
 
-	// convert map to protobuf struct
-	yamlStruct, err := structpb.NewStruct(yamlMap)
-	if err != nil {
-		return nil, fmt.Errorf("unable to convert map to protobuf struct: %w", err)
-	}
-	return yamlStruct, nil
+	return configpolicy.MarshalConfigPolicy(yamlMap), nil
 }
 
 // Add or update workspace task config policies.
 func (*apiServer) PutWorkspaceConfigPolicies(
 	ctx context.Context, req *apiv1.PutWorkspaceConfigPoliciesRequest,
 ) (*apiv1.PutWorkspaceConfigPoliciesResponse, error) {
-	if !validWorkloadEnum(req.WorkloadType) {
+	if !configpolicy.ValidWorkloadType(req.WorkloadType) {
 		return nil, fmt.Errorf("invalid workload type: %s", req.WorkloadType)
 	}
 	data, err := stubData()
@@ -67,7 +44,7 @@ func (*apiServer) PutWorkspaceConfigPolicies(
 func (*apiServer) PutGlobalConfigPolicies(
 	ctx context.Context, req *apiv1.PutGlobalConfigPoliciesRequest,
 ) (*apiv1.PutGlobalConfigPoliciesResponse, error) {
-	if !validWorkloadEnum(req.WorkloadType) {
+	if !configpolicy.ValidWorkloadType(req.WorkloadType) {
 		return nil, fmt.Errorf("invalid workload type: %s", req.WorkloadType)
 	}
 	data, err := stubData()
@@ -78,7 +55,7 @@ func (*apiServer) PutGlobalConfigPolicies(
 func (*apiServer) GetWorkspaceConfigPolicies(
 	ctx context.Context, req *apiv1.GetWorkspaceConfigPoliciesRequest,
 ) (*apiv1.GetWorkspaceConfigPoliciesResponse, error) {
-	if !validWorkloadEnum(req.WorkloadType) {
+	if !configpolicy.ValidWorkloadType(req.WorkloadType) {
 		return nil, fmt.Errorf("invalid workload type: %s", req.WorkloadType)
 	}
 	data, err := stubData()
@@ -89,7 +66,7 @@ func (*apiServer) GetWorkspaceConfigPolicies(
 func (*apiServer) GetGlobalConfigPolicies(
 	ctx context.Context, req *apiv1.GetGlobalConfigPoliciesRequest,
 ) (*apiv1.GetGlobalConfigPoliciesResponse, error) {
-	if !validWorkloadEnum(req.WorkloadType) {
+	if !configpolicy.ValidWorkloadType(req.WorkloadType) {
 		return nil, fmt.Errorf("invalid workload type: %s", req.WorkloadType)
 	}
 	data, err := stubData()
@@ -100,7 +77,7 @@ func (*apiServer) GetGlobalConfigPolicies(
 func (*apiServer) DeleteWorkspaceConfigPolicies(
 	ctx context.Context, req *apiv1.DeleteWorkspaceConfigPoliciesRequest,
 ) (*apiv1.DeleteWorkspaceConfigPoliciesResponse, error) {
-	if !validWorkloadEnum(req.WorkloadType) {
+	if !configpolicy.ValidWorkloadType(req.WorkloadType) {
 		return nil, fmt.Errorf("invalid workload type: %s", req.WorkloadType)
 	}
 	return &apiv1.DeleteWorkspaceConfigPoliciesResponse{}, nil
@@ -110,7 +87,7 @@ func (*apiServer) DeleteWorkspaceConfigPolicies(
 func (*apiServer) DeleteGlobalConfigPolicies(
 	ctx context.Context, req *apiv1.DeleteGlobalConfigPoliciesRequest,
 ) (*apiv1.DeleteGlobalConfigPoliciesResponse, error) {
-	if !validWorkloadEnum(req.WorkloadType) {
+	if !configpolicy.ValidWorkloadType(req.WorkloadType) {
 		return nil, fmt.Errorf("invalid workload type: %s", req.WorkloadType)
 	}
 	return &apiv1.DeleteGlobalConfigPoliciesResponse{}, nil
diff --git a/master/internal/configpolicy/task_config_policy.go b/master/internal/configpolicy/task_config_policy.go
index 2167fb6155a..90a74192e8f 100644
--- a/master/internal/configpolicy/task_config_policy.go
+++ b/master/internal/configpolicy/task_config_policy.go
@@ -26,7 +26,7 @@ type Constraints struct {
 // Submitted experiments whose constraint fields vary from the respective Constraint fields set
 // within a given scope are rejected.
 type ExperimentConfigPolicy struct {
-	InvariantConfig *expconf.ExperimentConfig `json:"config"`
+	InvariantConfig *expconf.ExperimentConfig `json:"invariant_config"`
 	Constraints     *Constraints              `json:"constraints"`
 }
 
@@ -36,6 +36,6 @@ type ExperimentConfigPolicy struct {
 // Submitted NTSC tasks whose constraint fields vary from the respective Constraint fields set
 // within a given scope are rejected.
 type NTSCConfigPolicy struct {
-	InvariantConfig *model.CommandConfig `json:"config"`
+	InvariantConfig *model.CommandConfig `json:"invariant_config"`
 	Constraints     *Constraints         `json:"constraints"`
 }
diff --git a/master/internal/configpolicy/utils.go b/master/internal/configpolicy/utils.go
new file mode 100644
index 00000000000..0ce50f2c53a
--- /dev/null
+++ b/master/internal/configpolicy/utils.go
@@ -0,0 +1,60 @@
+package configpolicy
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/determined-ai/determined/master/pkg/model"
+	"github.com/determined-ai/determined/master/pkg/protoutils"
+	"github.com/ghodss/yaml"
+	"google.golang.org/protobuf/types/known/structpb"
+)
+
+// ValidWorkloadType checks if the string is an accepted WorkloadType
+func ValidWorkloadType(val string) bool {
+	switch val {
+	case string(model.ExperimentType), string(model.NTSCType):
+		return true
+	default:
+		return false
+	}
+}
+
+func UnmarshalExperimentConfigPolicy(str string) (*ExperimentConfigPolicy, error) {
+	var expConfigPolicy ExperimentConfigPolicy
+	var err error
+
+	if err = json.Unmarshal([]byte(str), &expConfigPolicy); err == nil {
+		// valid JSON input
+		return &expConfigPolicy, nil
+	}
+
+	if err = yaml.Unmarshal([]byte(str), &expConfigPolicy, yaml.DisallowUnknownFields); err == nil {
+		// valid Yaml input
+		return &expConfigPolicy, nil
+	}
+	// invalid JSON & invalid Yaml input
+	return nil, fmt.Errorf("invalid ExperimentConfigPolicy input: %w", err)
+}
+
+func UnmarshalNTSCConfigPolicy(str string) (*NTSCConfigPolicy, error) {
+	var ntscConfigPolicy NTSCConfigPolicy
+	var err error
+
+	if err = json.Unmarshal([]byte(str), &ntscConfigPolicy); err == nil {
+		// valid JSON input
+		return &ntscConfigPolicy, nil
+	}
+
+	if err = yaml.Unmarshal([]byte(str), &ntscConfigPolicy, yaml.DisallowUnknownFields); err == nil {
+		// valid Yaml input
+		return &ntscConfigPolicy, nil
+	}
+	// invalid JSON & Yaml input
+	return nil, fmt.Errorf("invalid ExperimentConfigPolicy input: %w", err)
+}
+
+func MarshalConfigPolicy(configPolicy interface{}) *structpb.Struct {
+	return protoutils.ToStruct(configPolicy)
+
+}
diff --git a/master/internal/configpolicy/utils_test.go b/master/internal/configpolicy/utils_test.go
new file mode 100644
index 00000000000..99a45f66453
--- /dev/null
+++ b/master/internal/configpolicy/utils_test.go
@@ -0,0 +1,299 @@
+package configpolicy
+
+import (
+	"testing"
+
+	"github.com/determined-ai/determined/master/pkg/model"
+	"github.com/determined-ai/determined/master/pkg/schemas/expconf"
+	"github.com/stretchr/testify/require"
+	"gotest.tools/assert"
+)
+
+const yamlConstraints = `
+constraints:
+  resources:
+    max_slots: 4
+  priority_limit: 10
+`
+const yamlExperiment = `
+invariant_config:
+  description: "test\nspecial\tchar"
+  environment:
+    force_pull_image: false
+    add_capabilities:
+      - "cap1"
+      - "cap2"
+  resources:
+    slots: 1
+`
+
+func TestUnmarshalYamlExperiment(t *testing.T) {
+	description := "test\nspecial\tchar"
+	slots := 1
+	forcePull := false
+	maxSlots := 4
+	priorityLimit := 10
+
+	structExperiment := ExperimentConfigPolicy{
+		InvariantConfig: &expconf.ExperimentConfig{
+			RawDescription: &description,
+			RawResources: &expconf.ResourcesConfig{
+				RawSlots: &slots,
+			},
+			RawEnvironment: &expconf.EnvironmentConfigV0{
+				RawForcePullImage:  &forcePull,
+				RawAddCapabilities: []string{"cap1", "cap2"},
+			},
+		},
+		Constraints: &Constraints{
+			ResourceConstraints: &ResourceConstraints{
+				MaxSlots: &maxSlots,
+			},
+			PriorityLimit: &priorityLimit,
+		},
+	}
+
+	justConfig := structExperiment
+	justConfig.Constraints = nil
+	justConstraints := structExperiment
+	justConstraints.InvariantConfig = nil
+
+	var testCases = []struct {
+		name   string
+		input  string
+		noErr  bool
+		output *ExperimentConfigPolicy
+	}{
+		{"valid yaml", yamlExperiment + yamlConstraints, true, &structExperiment},
+		{"just config", yamlExperiment, true, &justConfig},
+		{"just constraints", yamlConstraints, true, &justConstraints},
+		{"extra fields", yamlExperiment + `  extra_field: "string"` + yamlConstraints, false, nil},
+		{"invalid fields", yamlExperiment + "  debug true\n", false, nil},
+		{"empty input", "", true, &ExperimentConfigPolicy{}},
+		{"null/empty fields", yamlExperiment + `  debug:\n`, false, nil},
+		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			config, err := UnmarshalExperimentConfigPolicy(tt.input)
+			require.Equal(t, tt.noErr, err == nil)
+			if !tt.noErr {
+				assert.DeepEqual(t, tt.output, config)
+			}
+		})
+	}
+}
+
+const yamlNTSC = `
+invariant_config:
+  description: "test\nspecial\tchar"
+  environment:
+    force_pull_image: false
+    add_capabilities:
+      - "cap1"
+      - "cap2"
+  resources:
+    slots: 1
+`
+
+func TestUnmarshalYamlNTSC(t *testing.T) {
+	maxSlots := 4
+	priorityLimit := 10
+	structNTSC := NTSCConfigPolicy{
+		InvariantConfig: &model.CommandConfig{
+			Description: "test\nspecial\tchar",
+			Resources: model.ResourcesConfig{
+				Slots: 10,
+			},
+			Environment: model.Environment{
+				ForcePullImage:  false,
+				AddCapabilities: []string{"cap1", "cap2"},
+			},
+		},
+		Constraints: &Constraints{
+			ResourceConstraints: &ResourceConstraints{
+				MaxSlots: &maxSlots,
+			},
+			PriorityLimit: &priorityLimit,
+		},
+	}
+
+	justConfig := structNTSC
+	justConfig.Constraints = nil
+	justConstraints := structNTSC
+	justConstraints.InvariantConfig = nil
+
+	var testCases = []struct {
+		name   string
+		input  string
+		noErr  bool
+		output *NTSCConfigPolicy
+	}{
+		{"valid yaml", yamlNTSC + yamlConstraints, true, &structNTSC},
+		{"just config", yamlNTSC, true, &justConfig},
+		{"just constraints", yamlConstraints, true, &justConstraints},
+		{"extra fields", yamlNTSC + `  extra_field: "string"` + yamlConstraints, false, nil},
+		{"invalid fields", yamlNTSC + "  debug true\n", false, nil},
+		{"empty input", "", true, &NTSCConfigPolicy{}},
+		{"null/empty fields", yamlNTSC + `  debug:\n`, false, nil},
+		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			config, err := UnmarshalNTSCConfigPolicy(tt.input)
+			require.Equal(t, tt.noErr, err == nil)
+			if !tt.noErr {
+				assert.DeepEqual(t, tt.output, config)
+			}
+		})
+	}
+}
+
+const jsonConstraints = `
+constraints: {
+  resources: {
+    max_slots: 4
+  },
+  priority_limit: 10
+}
+`
+
+const jsonExperiment = `
+invariant_config: {
+  description: "test\nspecial\tchar",
+  environment: {
+    force_pull_image: false,
+    add_capabilities: ["cap1", "cap2"]
+  },
+  resources: {
+    slots: 1,
+  }
+}
+`
+
+func TestUnmarshalJSONExperiment(t *testing.T) {
+	description := "test\nspecial\tchar"
+	slots := 1
+	forcePull := false
+	maxSlots := 4
+	priorityLimit := 10
+
+	structExperiment := ExperimentConfigPolicy{
+		InvariantConfig: &expconf.ExperimentConfig{
+			RawDescription: &description,
+			RawResources: &expconf.ResourcesConfig{
+				RawSlots: &slots,
+			},
+			RawEnvironment: &expconf.EnvironmentConfigV0{
+				RawForcePullImage:  &forcePull,
+				RawAddCapabilities: []string{"cap1", "cap2"},
+			},
+		},
+		Constraints: &Constraints{
+			ResourceConstraints: &ResourceConstraints{
+				MaxSlots: &maxSlots,
+			},
+			PriorityLimit: &priorityLimit,
+		},
+	}
+
+	justConfig := structExperiment
+	justConfig.Constraints = nil
+	justConstraints := structExperiment
+	justConstraints.InvariantConfig = nil
+
+	var testCases = []struct {
+		name   string
+		input  string
+		noErr  bool
+		output *ExperimentConfigPolicy
+	}{
+		{"valid json", jsonExperiment + jsonConstraints, true, &structExperiment},
+		{"just config", jsonExperiment, true, &justConfig},
+		{"just constraints", jsonConstraints, true, &justConstraints},
+		{"extra fields", jsonExperiment + `  extra_field: "string"` + jsonConstraints, false, nil},
+		{"invalid fields", jsonExperiment + "  debug{ true }\n", false, nil},
+		{"empty input", "", true, &ExperimentConfigPolicy{}},
+		{"null/empty fields", jsonExperiment + "  debug:\n", false, nil},
+		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			config, err := UnmarshalExperimentConfigPolicy(tt.input)
+			require.Equal(t, tt.noErr, err == nil)
+			if !tt.noErr {
+				assert.DeepEqual(t, tt.output, config)
+			}
+		})
+	}
+}
+
+const jsonNTSC = `
+invariant_config: {
+  description: "test\nspecial\tchar",
+  environment: {
+    force_pull_image: false,
+    add_capabilities: ["cap1", "cap2"]
+  },
+  resources: {
+    slots: 1,
+  }
+}
+`
+
+func TestUnmarshalJSONNTSC(t *testing.T) {
+	maxSlots := 4
+	priorityLimit := 10
+	structNTSC := NTSCConfigPolicy{
+		InvariantConfig: &model.CommandConfig{
+			Description: "test\nspecial\tchar",
+			Resources: model.ResourcesConfig{
+				Slots: 10,
+			},
+			Environment: model.Environment{
+				ForcePullImage:  false,
+				AddCapabilities: []string{"cap1", "cap2"},
+			},
+		},
+		Constraints: &Constraints{
+			ResourceConstraints: &ResourceConstraints{
+				MaxSlots: &maxSlots,
+			},
+			PriorityLimit: &priorityLimit,
+		},
+	}
+
+	justConfig := structNTSC
+	justConfig.Constraints = nil
+	justConstraints := structNTSC
+	justConstraints.InvariantConfig = nil
+
+	var testCases = []struct {
+		name   string
+		input  string
+		noErr  bool
+		output *NTSCConfigPolicy
+	}{
+		{"valid json", jsonNTSC + jsonConstraints, true, &structNTSC},
+		{"just config", jsonNTSC, true, &justConfig},
+		{"just constraints", jsonConstraints, true, &justConstraints},
+		{"extra fields", jsonNTSC + `  extra_field: "string"` + jsonConstraints, false, nil},
+		{"invalid fields", jsonNTSC + "  debug{ true }\n", false, nil},
+		{"empty input", "", true, &NTSCConfigPolicy{}},
+		{"null/empty fields", jsonNTSC + "  debug:\n", false, nil},
+		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			config, err := UnmarshalNTSCConfigPolicy(tt.input)
+			require.Equal(t, tt.noErr, err == nil)
+			if !tt.noErr {
+				assert.DeepEqual(t, tt.output, config)
+			}
+		})
+	}
+}

From f075dd1ca668fac37504a292789c231ee52b2e2c Mon Sep 17 00:00:00 2001
From: Kristine Kunapuli <kristine.kunapuli@hpe.com>
Date: Thu, 5 Sep 2024 14:42:32 -0400
Subject: [PATCH 2/4] clean up API descriptions

---
 harness/determined/common/api/bindings.py     | 19 +++----
 master/internal/configpolicy/utils.go         |  1 -
 proto/pkg/apiv1/config_policies.pb.go         | 23 ++++----
 .../determined/api/v1/config_policies.proto   | 23 ++++----
 webui/react/src/services/api-ts-sdk/api.ts    | 52 +++++++++----------
 5 files changed, 54 insertions(+), 64 deletions(-)

diff --git a/harness/determined/common/api/bindings.py b/harness/determined/common/api/bindings.py
index a208d9138aa..f2ff9e34230 100644
--- a/harness/determined/common/api/bindings.py
+++ b/harness/determined/common/api/bindings.py
@@ -7144,7 +7144,7 @@ def to_json(self, omit_unset: bool = False) -> typing.Dict[str, typing.Any]:
         return out
 
 class v1GetWorkspaceConfigPoliciesResponse(Printable):
-    """Response to GetWorkspaceConfigPolicies request."""
+    """Response to GetWorkspaceConfigPoliciesRequest."""
     configPolicies: "typing.Optional[typing.Dict[str, typing.Any]]" = None
 
     def __init__(
@@ -12127,7 +12127,7 @@ def to_json(self, omit_unset: bool = False) -> typing.Dict[str, typing.Any]:
         return out
 
 class v1PutGlobalConfigPoliciesResponse(Printable):
-    """Response to PutGlobalConfigPolicies request."""
+    """Response to PutGlobalConfigPoliciesRequest."""
     configPolicies: "typing.Optional[typing.Dict[str, typing.Any]]" = None
 
     def __init__(
@@ -12365,7 +12365,7 @@ def to_json(self, omit_unset: bool = False) -> typing.Dict[str, typing.Any]:
         return out
 
 class v1PutWorkspaceConfigPoliciesResponse(Printable):
-    """Response to PutWorkspaceConfigPolicies request."""
+    """Response to PutWorkspaceConfigPoliciesRequest."""
     configPolicies: "typing.Optional[typing.Dict[str, typing.Any]]" = None
 
     def __init__(
@@ -18431,8 +18431,7 @@ def delete_DeleteGlobalConfigPolicies(
 ) -> None:
     """Delete global task config policies.
 
-    - workloadType: Specifies which workload type the config policies apply to: EXPERIMENT or
-NTSC.
+    - workloadType: The workload type the config policies apply to: EXPERIMENT or NTSC.
     """
     _params = None
     if type(workloadType) == str:
@@ -18681,7 +18680,7 @@ def delete_DeleteWorkspaceConfigPolicies(
     """Delete workspace task config policies.
 
     - workloadType: The workload type the config policies apply to: EXPERIMENT or NTSC.
-    - workspaceId: Specifies which workspace the config policies apply to. Use global API for
+    - workspaceId: The workspace the config policies apply to. Use global API for
 global config policies.
     """
     _params = None
@@ -19565,8 +19564,7 @@ def get_GetGlobalConfigPolicies(
 ) -> "v1GetGlobalConfigPoliciesResponse":
     """Get global task config policies.
 
-    - workloadType: Specifies which workload type the config policies apply to: EXPERIMENT or
-NTSC.
+    - workloadType: The workload type the config policies apply to: EXPERIMENT or NTSC.
     """
     _params = None
     if type(workloadType) == str:
@@ -21670,7 +21668,7 @@ def get_GetWorkspaceConfigPolicies(
     """Get workspace task config policies.
 
     - workloadType: The workload type the config policies apply to: EXPERIMENT or NTSC.
-    - workspaceId: Specifies which workspace the config policies apply to. Use global API for
+    - workspaceId: The workspace the config policies apply to. Use global API for
 global config policies.
     """
     _params = None
@@ -23727,8 +23725,7 @@ def put_PutWorkspaceConfigPolicies(
 ) -> "v1PutWorkspaceConfigPoliciesResponse":
     """Add or update workspace task config policies.
 
-    - workloadType: Specifies which workload type the config policies apply to: EXPERIMENT or
-NTSC.
+    - workloadType: The workload type the config policies apply to: EXPERIMENT or NTSC.
     - workspaceId: The workspace the config policies apply to. Use global API for
 global config policies.
     """
diff --git a/master/internal/configpolicy/utils.go b/master/internal/configpolicy/utils.go
index 0ce50f2c53a..5cad0e5fafc 100644
--- a/master/internal/configpolicy/utils.go
+++ b/master/internal/configpolicy/utils.go
@@ -56,5 +56,4 @@ func UnmarshalNTSCConfigPolicy(str string) (*NTSCConfigPolicy, error) {
 
 func MarshalConfigPolicy(configPolicy interface{}) *structpb.Struct {
 	return protoutils.ToStruct(configPolicy)
-
 }
diff --git a/proto/pkg/apiv1/config_policies.pb.go b/proto/pkg/apiv1/config_policies.pb.go
index 1cad4531241..7f4721dbd9c 100644
--- a/proto/pkg/apiv1/config_policies.pb.go
+++ b/proto/pkg/apiv1/config_policies.pb.go
@@ -29,8 +29,7 @@ type PutWorkspaceConfigPoliciesRequest struct {
 	// The workspace the config policies apply to. Use global API for
 	// global config policies.
 	WorkspaceId int32 `protobuf:"varint,1,opt,name=workspace_id,json=workspaceId,proto3" json:"workspace_id,omitempty"`
-	// Specifies which workload type the config policies apply to: EXPERIMENT or
-	// NTSC.
+	// The workload type the config policies apply to: EXPERIMENT or NTSC.
 	WorkloadType string `protobuf:"bytes,2,opt,name=workload_type,json=workloadType,proto3" json:"workload_type,omitempty"`
 	// The config policies to use. Contains both invariant configs and constraints
 	// in yaml or json format.
@@ -90,7 +89,7 @@ func (x *PutWorkspaceConfigPoliciesRequest) GetConfigPolicies() string {
 	return ""
 }
 
-// Response to PutWorkspaceConfigPolicies request.
+// Response to PutWorkspaceConfigPoliciesRequest.
 type PutWorkspaceConfigPoliciesResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
@@ -200,7 +199,7 @@ func (x *PutGlobalConfigPoliciesRequest) GetConfigPolicies() string {
 	return ""
 }
 
-// Response to PutGlobalConfigPolicies request.
+// Response to PutGlobalConfigPoliciesRequest.
 type PutGlobalConfigPoliciesResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
@@ -257,7 +256,7 @@ type GetWorkspaceConfigPoliciesRequest struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	// Specifies which workspace the config policies apply to. Use global API for
+	// The workspace the config policies apply to. Use global API for
 	// global config policies.
 	WorkspaceId int32 `protobuf:"varint,1,opt,name=workspace_id,json=workspaceId,proto3" json:"workspace_id,omitempty"`
 	// The workload type the config policies apply to: EXPERIMENT or NTSC.
@@ -310,7 +309,7 @@ func (x *GetWorkspaceConfigPoliciesRequest) GetWorkloadType() string {
 	return ""
 }
 
-// Response to GetWorkspaceConfigPolicies request.
+// Response to GetWorkspaceConfigPoliciesRequest.
 type GetWorkspaceConfigPoliciesResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
@@ -367,8 +366,7 @@ type GetGlobalConfigPoliciesRequest struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	// Specifies which workload type the config policies apply to: EXPERIMENT or
-	// NTSC.
+	// The workload type the config policies apply to: EXPERIMENT or NTSC.
 	WorkloadType string `protobuf:"bytes,1,opt,name=workload_type,json=workloadType,proto3" json:"workload_type,omitempty"`
 }
 
@@ -468,7 +466,7 @@ type DeleteWorkspaceConfigPoliciesRequest struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	// Specifies which workspace the config policies apply to. Use global API for
+	// The workspace the config policies apply to. Use global API for
 	// global config policies.
 	WorkspaceId int32 `protobuf:"varint,1,opt,name=workspace_id,json=workspaceId,proto3" json:"workspace_id,omitempty"`
 	// The workload type the config policies apply to: EXPERIMENT or NTSC.
@@ -521,7 +519,7 @@ func (x *DeleteWorkspaceConfigPoliciesRequest) GetWorkloadType() string {
 	return ""
 }
 
-// Response to DeleteWorkspaceConfigPolicies request.
+// Response to DeleteWorkspaceConfigPoliciesRequest.
 type DeleteWorkspaceConfigPoliciesResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
@@ -567,8 +565,7 @@ type DeleteGlobalConfigPoliciesRequest struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	// Specifies which workload type the config policies apply to: EXPERIMENT or
-	// NTSC.
+	// The workload type the config policies apply to: EXPERIMENT or NTSC.
 	WorkloadType string `protobuf:"bytes,1,opt,name=workload_type,json=workloadType,proto3" json:"workload_type,omitempty"`
 }
 
@@ -611,7 +608,7 @@ func (x *DeleteGlobalConfigPoliciesRequest) GetWorkloadType() string {
 	return ""
 }
 
-// Response to DeleteGlobalConfigPolicies request.
+// Response to DeleteGlobalConfigPoliciesRequest.
 type DeleteGlobalConfigPoliciesResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
diff --git a/proto/src/determined/api/v1/config_policies.proto b/proto/src/determined/api/v1/config_policies.proto
index a1d70c2137a..36855d5109b 100644
--- a/proto/src/determined/api/v1/config_policies.proto
+++ b/proto/src/determined/api/v1/config_policies.proto
@@ -11,15 +11,14 @@ message PutWorkspaceConfigPoliciesRequest {
   // The workspace the config policies apply to. Use global API for
   // global config policies.
   int32 workspace_id = 1;
-  // Specifies which workload type the config policies apply to: EXPERIMENT or
-  // NTSC.
+  // The workload type the config policies apply to: EXPERIMENT or NTSC.
   string workload_type = 2;
   // The config policies to use. Contains both invariant configs and constraints
   // in yaml or json format.
   string config_policies = 3;
 }
 
-// Response to PutWorkspaceConfigPolicies request.
+// Response to PutWorkspaceConfigPoliciesRequest.
 message PutWorkspaceConfigPoliciesResponse {
   // The config policies saved. Contains both invariant configs and constraints
   // in yaml or json format.
@@ -36,7 +35,7 @@ message PutGlobalConfigPoliciesRequest {
   string config_policies = 2;
 }
 
-// Response to PutGlobalConfigPolicies request.
+// Response to PutGlobalConfigPoliciesRequest.
 message PutGlobalConfigPoliciesResponse {
   // The config policies saved. Contains both invariant configs and constraints
   // in yaml or json format.
@@ -46,14 +45,14 @@ message PutGlobalConfigPoliciesResponse {
 // GetWorkspaceConfigPoliciesRequest lists task config policies
 // for a given workspace and workload type.
 message GetWorkspaceConfigPoliciesRequest {
-  // Specifies which workspace the config policies apply to. Use global API for
+  // The workspace the config policies apply to. Use global API for
   // global config policies.
   int32 workspace_id = 1;
   //  The workload type the config policies apply to: EXPERIMENT or NTSC.
   string workload_type = 2;
 }
 
-// Response to GetWorkspaceConfigPolicies request.
+// Response to GetWorkspaceConfigPoliciesRequest.
 message GetWorkspaceConfigPoliciesResponse {
   // The current config policies saved for the workspace. Contains both
   // invariant configs and constraints in yaml or json format.
@@ -63,8 +62,7 @@ message GetWorkspaceConfigPoliciesResponse {
 // GetGlobalConfigPoliciesRequest lists global task config
 // policies for a given workload type.
 message GetGlobalConfigPoliciesRequest {
-  // Specifies which workload type the config policies apply to: EXPERIMENT or
-  // NTSC.
+  //  The workload type the config policies apply to: EXPERIMENT or NTSC.
   string workload_type = 1;
 }
 
@@ -78,23 +76,22 @@ message GetGlobalConfigPoliciesResponse {
 // DeleteWorkspaceConfigPoliciesRequest is used to delete all task config
 // policies for the workspace and workload type.
 message DeleteWorkspaceConfigPoliciesRequest {
-  // Specifies which workspace the config policies apply to. Use global API for
+  // The workspace the config policies apply to. Use global API for
   // global config policies.
   int32 workspace_id = 1;
   // The workload type the config policies apply to: EXPERIMENT or NTSC.
   string workload_type = 2;
 }
 
-// Response to DeleteWorkspaceConfigPolicies request.
+// Response to DeleteWorkspaceConfigPoliciesRequest.
 message DeleteWorkspaceConfigPoliciesResponse {}
 
 // DeleteGlobalConfigPoliciesRequest is used to delete all global task config
 // policies for the workload type.
 message DeleteGlobalConfigPoliciesRequest {
-  // Specifies which workload type the config policies apply to: EXPERIMENT or
-  // NTSC.
+  // The workload type the config policies apply to: EXPERIMENT or NTSC.
   string workload_type = 1;
 }
 
-// Response to DeleteGlobalConfigPolicies request.
+// Response to DeleteGlobalConfigPoliciesRequest.
 message DeleteGlobalConfigPoliciesResponse {}
diff --git a/webui/react/src/services/api-ts-sdk/api.ts b/webui/react/src/services/api-ts-sdk/api.ts
index 8a451341d38..7d12ca28df4 100644
--- a/webui/react/src/services/api-ts-sdk/api.ts
+++ b/webui/react/src/services/api-ts-sdk/api.ts
@@ -2651,7 +2651,7 @@ export interface V1DeleteExperimentsResponse {
     results: Array<V1ExperimentActionResult>;
 }
 /**
- * Response to DeleteGlobalConfigPolicies request.
+ * Response to DeleteGlobalConfigPoliciesRequest.
  * @export
  * @interface V1DeleteGlobalConfigPoliciesResponse
  */
@@ -2751,7 +2751,7 @@ export interface V1DeleteTensorboardFilesResponse {
 export interface V1DeleteWebhookResponse {
 }
 /**
- * Response to DeleteWorkspaceConfigPolicies request.
+ * Response to DeleteWorkspaceConfigPoliciesRequest.
  * @export
  * @interface V1DeleteWorkspaceConfigPoliciesResponse
  */
@@ -5280,7 +5280,7 @@ export interface V1GetWebhooksResponse {
     webhooks: Array<V1Webhook>;
 }
 /**
- * Response to GetWorkspaceConfigPolicies request.
+ * Response to GetWorkspaceConfigPoliciesRequest.
  * @export
  * @interface V1GetWorkspaceConfigPoliciesResponse
  */
@@ -8817,7 +8817,7 @@ export interface V1PutExperimentsRetainLogsResponse {
     results: Array<V1ExperimentActionResult>;
 }
 /**
- * Response to PutGlobalConfigPolicies request.
+ * Response to PutGlobalConfigPoliciesRequest.
  * @export
  * @interface V1PutGlobalConfigPoliciesResponse
  */
@@ -8945,7 +8945,7 @@ export interface V1PutWorkspaceConfigPoliciesRequest {
      */
     workspaceId?: number;
     /**
-     * Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+     * The workload type the config policies apply to: EXPERIMENT or NTSC.
      * @type {string}
      * @memberof V1PutWorkspaceConfigPoliciesRequest
      */
@@ -8958,7 +8958,7 @@ export interface V1PutWorkspaceConfigPoliciesRequest {
     configPolicies?: string;
 }
 /**
- * Response to PutWorkspaceConfigPolicies request.
+ * Response to PutWorkspaceConfigPoliciesRequest.
  * @export
  * @interface V1PutWorkspaceConfigPoliciesResponse
  */
@@ -12752,7 +12752,7 @@ export const AlphaApiFetchParamCreator = function (configuration?: Configuration
         /**
          * 
          * @summary Delete global task config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
          */
@@ -12788,7 +12788,7 @@ export const AlphaApiFetchParamCreator = function (configuration?: Configuration
         /**
          * 
          * @summary Delete workspace task config policies.
-         * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+         * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
          * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -12830,7 +12830,7 @@ export const AlphaApiFetchParamCreator = function (configuration?: Configuration
         /**
          * 
          * @summary Get global task config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
          */
@@ -12866,7 +12866,7 @@ export const AlphaApiFetchParamCreator = function (configuration?: Configuration
         /**
          * 
          * @summary Get workspace task config policies.
-         * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+         * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
          * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -12945,7 +12945,7 @@ export const AlphaApiFetchParamCreator = function (configuration?: Configuration
          * 
          * @summary Add or update workspace task config policies.
          * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {V1PutWorkspaceConfigPoliciesRequest} body
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13003,7 +13003,7 @@ export const AlphaApiFp = function (configuration?: Configuration) {
         /**
          * 
          * @summary Delete global task config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
          */
@@ -13022,7 +13022,7 @@ export const AlphaApiFp = function (configuration?: Configuration) {
         /**
          * 
          * @summary Delete workspace task config policies.
-         * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+         * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
          * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13042,7 +13042,7 @@ export const AlphaApiFp = function (configuration?: Configuration) {
         /**
          * 
          * @summary Get global task config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
          */
@@ -13061,7 +13061,7 @@ export const AlphaApiFp = function (configuration?: Configuration) {
         /**
          * 
          * @summary Get workspace task config policies.
-         * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+         * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
          * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13101,7 +13101,7 @@ export const AlphaApiFp = function (configuration?: Configuration) {
          * 
          * @summary Add or update workspace task config policies.
          * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {V1PutWorkspaceConfigPoliciesRequest} body
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13130,7 +13130,7 @@ export const AlphaApiFactory = function (configuration?: Configuration, fetch?:
         /**
          * 
          * @summary Delete global task config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
          */
@@ -13140,7 +13140,7 @@ export const AlphaApiFactory = function (configuration?: Configuration, fetch?:
         /**
          * 
          * @summary Delete workspace task config policies.
-         * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+         * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
          * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13151,7 +13151,7 @@ export const AlphaApiFactory = function (configuration?: Configuration, fetch?:
         /**
          * 
          * @summary Get global task config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
          */
@@ -13161,7 +13161,7 @@ export const AlphaApiFactory = function (configuration?: Configuration, fetch?:
         /**
          * 
          * @summary Get workspace task config policies.
-         * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+         * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
          * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13183,7 +13183,7 @@ export const AlphaApiFactory = function (configuration?: Configuration, fetch?:
          * 
          * @summary Add or update workspace task config policies.
          * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
-         * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+         * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
          * @param {V1PutWorkspaceConfigPoliciesRequest} body
          * @param {*} [options] Override http request option.
          * @throws {RequiredError}
@@ -13204,7 +13204,7 @@ export class AlphaApi extends BaseAPI {
     /**
      * 
      * @summary Delete global task config policies.
-     * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+     * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
      * @memberof AlphaApi
@@ -13216,7 +13216,7 @@ export class AlphaApi extends BaseAPI {
     /**
      * 
      * @summary Delete workspace task config policies.
-     * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+     * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
      * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
@@ -13229,7 +13229,7 @@ export class AlphaApi extends BaseAPI {
     /**
      * 
      * @summary Get global task config policies.
-     * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+     * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
      * @memberof AlphaApi
@@ -13241,7 +13241,7 @@ export class AlphaApi extends BaseAPI {
     /**
      * 
      * @summary Get workspace task config policies.
-     * @param {number} workspaceId Specifies which workspace the config policies apply to. Use global API for global config policies.
+     * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
      * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
@@ -13267,7 +13267,7 @@ export class AlphaApi extends BaseAPI {
      * 
      * @summary Add or update workspace task config policies.
      * @param {number} workspaceId The workspace the config policies apply to. Use global API for global config policies.
-     * @param {string} workloadType Specifies which workload type the config policies apply to: EXPERIMENT or NTSC.
+     * @param {string} workloadType The workload type the config policies apply to: EXPERIMENT or NTSC.
      * @param {V1PutWorkspaceConfigPoliciesRequest} body
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}

From 3903fa0a13b521c799d60f3a18c4426a38877653 Mon Sep 17 00:00:00 2001
From: Kristine Kunapuli <kristine.kunapuli@hpe.com>
Date: Thu, 5 Sep 2024 15:20:47 -0400
Subject: [PATCH 3/4] reduce duplicate code; add comments to exported functions

---
 master/internal/configpolicy/utils.go      |  10 +-
 master/internal/configpolicy/utils_test.go | 142 +++++++--------------
 2 files changed, 56 insertions(+), 96 deletions(-)

diff --git a/master/internal/configpolicy/utils.go b/master/internal/configpolicy/utils.go
index 5cad0e5fafc..2004f14769c 100644
--- a/master/internal/configpolicy/utils.go
+++ b/master/internal/configpolicy/utils.go
@@ -4,13 +4,14 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/determined-ai/determined/master/pkg/model"
-	"github.com/determined-ai/determined/master/pkg/protoutils"
 	"github.com/ghodss/yaml"
 	"google.golang.org/protobuf/types/known/structpb"
+
+	"github.com/determined-ai/determined/master/pkg/model"
+	"github.com/determined-ai/determined/master/pkg/protoutils"
 )
 
-// ValidWorkloadType checks if the string is an accepted WorkloadType
+// ValidWorkloadType checks if the string is an accepted WorkloadType.
 func ValidWorkloadType(val string) bool {
 	switch val {
 	case string(model.ExperimentType), string(model.NTSCType):
@@ -20,6 +21,7 @@ func ValidWorkloadType(val string) bool {
 	}
 }
 
+// UnmarshalExperimentConfigPolicy unpacks a string into ExperimentConfigPolicy struct.
 func UnmarshalExperimentConfigPolicy(str string) (*ExperimentConfigPolicy, error) {
 	var expConfigPolicy ExperimentConfigPolicy
 	var err error
@@ -37,6 +39,7 @@ func UnmarshalExperimentConfigPolicy(str string) (*ExperimentConfigPolicy, error
 	return nil, fmt.Errorf("invalid ExperimentConfigPolicy input: %w", err)
 }
 
+// UnmarshalNTSCConfigPolicy unpacks a string into NTSCConfigPolicy struct.
 func UnmarshalNTSCConfigPolicy(str string) (*NTSCConfigPolicy, error) {
 	var ntscConfigPolicy NTSCConfigPolicy
 	var err error
@@ -54,6 +57,7 @@ func UnmarshalNTSCConfigPolicy(str string) (*NTSCConfigPolicy, error) {
 	return nil, fmt.Errorf("invalid ExperimentConfigPolicy input: %w", err)
 }
 
+// MarshalConfigPolicy packs a config policy into a proto struct.
 func MarshalConfigPolicy(configPolicy interface{}) *structpb.Struct {
 	return protoutils.ToStruct(configPolicy)
 }
diff --git a/master/internal/configpolicy/utils_test.go b/master/internal/configpolicy/utils_test.go
index 99a45f66453..9891ea607fa 100644
--- a/master/internal/configpolicy/utils_test.go
+++ b/master/internal/configpolicy/utils_test.go
@@ -3,10 +3,11 @@ package configpolicy
 import (
 	"testing"
 
-	"github.com/determined-ai/determined/master/pkg/model"
-	"github.com/determined-ai/determined/master/pkg/schemas/expconf"
 	"github.com/stretchr/testify/require"
 	"gotest.tools/assert"
+
+	"github.com/determined-ai/determined/master/pkg/model"
+	"github.com/determined-ai/determined/master/pkg/schemas/expconf"
 )
 
 const yamlConstraints = `
@@ -15,6 +16,7 @@ constraints:
     max_slots: 4
   priority_limit: 10
 `
+
 const yamlExperiment = `
 invariant_config:
   description: "test\nspecial\tchar"
@@ -27,38 +29,40 @@ invariant_config:
     slots: 1
 `
 
-func TestUnmarshalYamlExperiment(t *testing.T) {
-	description := "test\nspecial\tchar"
-	slots := 1
-	forcePull := false
-	maxSlots := 4
-	priorityLimit := 10
+var (
+	description   = "test\nspecial\tchar"
+	slots         = 1
+	forcePull     = false
+	maxSlots      = 4
+	priorityLimit = 10
+)
 
-	structExperiment := ExperimentConfigPolicy{
-		InvariantConfig: &expconf.ExperimentConfig{
-			RawDescription: &description,
-			RawResources: &expconf.ResourcesConfig{
-				RawSlots: &slots,
-			},
-			RawEnvironment: &expconf.EnvironmentConfigV0{
-				RawForcePullImage:  &forcePull,
-				RawAddCapabilities: []string{"cap1", "cap2"},
-			},
+var structExperiment = ExperimentConfigPolicy{
+	InvariantConfig: &expconf.ExperimentConfig{
+		RawDescription: &description,
+		RawResources: &expconf.ResourcesConfig{
+			RawSlots: &slots,
 		},
-		Constraints: &Constraints{
-			ResourceConstraints: &ResourceConstraints{
-				MaxSlots: &maxSlots,
-			},
-			PriorityLimit: &priorityLimit,
+		RawEnvironment: &expconf.EnvironmentConfigV0{
+			RawForcePullImage:  &forcePull,
+			RawAddCapabilities: []string{"cap1", "cap2"},
 		},
-	}
+	},
+	Constraints: &Constraints{
+		ResourceConstraints: &ResourceConstraints{
+			MaxSlots: &maxSlots,
+		},
+		PriorityLimit: &priorityLimit,
+	},
+}
 
+func TestUnmarshalYamlExperiment(t *testing.T) {
 	justConfig := structExperiment
 	justConfig.Constraints = nil
 	justConstraints := structExperiment
 	justConstraints.InvariantConfig = nil
 
-	var testCases = []struct {
+	testCases := []struct {
 		name   string
 		input  string
 		noErr  bool
@@ -97,34 +101,32 @@ invariant_config:
     slots: 1
 `
 
-func TestUnmarshalYamlNTSC(t *testing.T) {
-	maxSlots := 4
-	priorityLimit := 10
-	structNTSC := NTSCConfigPolicy{
-		InvariantConfig: &model.CommandConfig{
-			Description: "test\nspecial\tchar",
-			Resources: model.ResourcesConfig{
-				Slots: 10,
-			},
-			Environment: model.Environment{
-				ForcePullImage:  false,
-				AddCapabilities: []string{"cap1", "cap2"},
-			},
+var structNTSC = NTSCConfigPolicy{
+	InvariantConfig: &model.CommandConfig{
+		Description: "test\nspecial\tchar",
+		Resources: model.ResourcesConfig{
+			Slots: 10,
 		},
-		Constraints: &Constraints{
-			ResourceConstraints: &ResourceConstraints{
-				MaxSlots: &maxSlots,
-			},
-			PriorityLimit: &priorityLimit,
+		Environment: model.Environment{
+			ForcePullImage:  false,
+			AddCapabilities: []string{"cap1", "cap2"},
 		},
-	}
+	},
+	Constraints: &Constraints{
+		ResourceConstraints: &ResourceConstraints{
+			MaxSlots: &maxSlots,
+		},
+		PriorityLimit: &priorityLimit,
+	},
+}
 
+func TestUnmarshalYamlNTSC(t *testing.T) {
 	justConfig := structNTSC
 	justConfig.Constraints = nil
 	justConstraints := structNTSC
 	justConstraints.InvariantConfig = nil
 
-	var testCases = []struct {
+	testCases := []struct {
 		name   string
 		input  string
 		noErr  bool
@@ -174,37 +176,12 @@ invariant_config: {
 `
 
 func TestUnmarshalJSONExperiment(t *testing.T) {
-	description := "test\nspecial\tchar"
-	slots := 1
-	forcePull := false
-	maxSlots := 4
-	priorityLimit := 10
-
-	structExperiment := ExperimentConfigPolicy{
-		InvariantConfig: &expconf.ExperimentConfig{
-			RawDescription: &description,
-			RawResources: &expconf.ResourcesConfig{
-				RawSlots: &slots,
-			},
-			RawEnvironment: &expconf.EnvironmentConfigV0{
-				RawForcePullImage:  &forcePull,
-				RawAddCapabilities: []string{"cap1", "cap2"},
-			},
-		},
-		Constraints: &Constraints{
-			ResourceConstraints: &ResourceConstraints{
-				MaxSlots: &maxSlots,
-			},
-			PriorityLimit: &priorityLimit,
-		},
-	}
-
 	justConfig := structExperiment
 	justConfig.Constraints = nil
 	justConstraints := structExperiment
 	justConstraints.InvariantConfig = nil
 
-	var testCases = []struct {
+	testCases := []struct {
 		name   string
 		input  string
 		noErr  bool
@@ -245,33 +222,12 @@ invariant_config: {
 `
 
 func TestUnmarshalJSONNTSC(t *testing.T) {
-	maxSlots := 4
-	priorityLimit := 10
-	structNTSC := NTSCConfigPolicy{
-		InvariantConfig: &model.CommandConfig{
-			Description: "test\nspecial\tchar",
-			Resources: model.ResourcesConfig{
-				Slots: 10,
-			},
-			Environment: model.Environment{
-				ForcePullImage:  false,
-				AddCapabilities: []string{"cap1", "cap2"},
-			},
-		},
-		Constraints: &Constraints{
-			ResourceConstraints: &ResourceConstraints{
-				MaxSlots: &maxSlots,
-			},
-			PriorityLimit: &priorityLimit,
-		},
-	}
-
 	justConfig := structNTSC
 	justConfig.Constraints = nil
 	justConstraints := structNTSC
 	justConstraints.InvariantConfig = nil
 
-	var testCases = []struct {
+	testCases := []struct {
 		name   string
 		input  string
 		noErr  bool

From a0e8a1a7164980b4524daffb9e7d82ec8f4f38f5 Mon Sep 17 00:00:00 2001
From: Kristine Kunapuli <kristine.kunapuli@hpe.com>
Date: Fri, 6 Sep 2024 11:18:33 -0400
Subject: [PATCH 4/4] fix json tests; add valid type test

---
 master/internal/configpolicy/utils.go      |   9 +-
 master/internal/configpolicy/utils_test.go | 142 ++++++++++++++-------
 2 files changed, 106 insertions(+), 45 deletions(-)

diff --git a/master/internal/configpolicy/utils.go b/master/internal/configpolicy/utils.go
index 2004f14769c..ca54fd3202e 100644
--- a/master/internal/configpolicy/utils.go
+++ b/master/internal/configpolicy/utils.go
@@ -1,6 +1,7 @@
 package configpolicy
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 
@@ -26,7 +27,9 @@ func UnmarshalExperimentConfigPolicy(str string) (*ExperimentConfigPolicy, error
 	var expConfigPolicy ExperimentConfigPolicy
 	var err error
 
-	if err = json.Unmarshal([]byte(str), &expConfigPolicy); err == nil {
+	dec := json.NewDecoder(bytes.NewReader([]byte(str)))
+	dec.DisallowUnknownFields()
+	if err = dec.Decode(&expConfigPolicy); err == nil {
 		// valid JSON input
 		return &expConfigPolicy, nil
 	}
@@ -44,7 +47,9 @@ func UnmarshalNTSCConfigPolicy(str string) (*NTSCConfigPolicy, error) {
 	var ntscConfigPolicy NTSCConfigPolicy
 	var err error
 
-	if err = json.Unmarshal([]byte(str), &ntscConfigPolicy); err == nil {
+	dec := json.NewDecoder(bytes.NewReader([]byte(str)))
+	dec.DisallowUnknownFields()
+	if err = dec.Decode(&ntscConfigPolicy); err == nil {
 		// valid JSON input
 		return &ntscConfigPolicy, nil
 	}
diff --git a/master/internal/configpolicy/utils_test.go b/master/internal/configpolicy/utils_test.go
index 9891ea607fa..bd418072d12 100644
--- a/master/internal/configpolicy/utils_test.go
+++ b/master/internal/configpolicy/utils_test.go
@@ -10,6 +10,26 @@ import (
 	"github.com/determined-ai/determined/master/pkg/schemas/expconf"
 )
 
+func TestValidWorkloadType(t *testing.T) {
+	testCases := []struct {
+		name  string
+		input string
+		valid bool
+	}{
+		{"valid experiment type", "EXPERIMENT", true},
+		{"valid ntsc type", "NTSC", true},
+		{"invalid type", "EXPERIMENTS", false},
+		{"lowercase", "experiment", false},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			valid := ValidWorkloadType(tt.input)
+			require.Equal(t, tt.valid, valid)
+		})
+	}
+}
+
 const yamlConstraints = `
 constraints:
   resources:
@@ -74,8 +94,8 @@ func TestUnmarshalYamlExperiment(t *testing.T) {
 		{"extra fields", yamlExperiment + `  extra_field: "string"` + yamlConstraints, false, nil},
 		{"invalid fields", yamlExperiment + "  debug true\n", false, nil},
 		{"empty input", "", true, &ExperimentConfigPolicy{}},
-		{"null/empty fields", yamlExperiment + `  debug:\n`, false, nil},
-		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+		{"null/empty fields", yamlExperiment + "  debug:\n", true, &structExperiment},
+		{"wrong field type", "invariant_config:\n  debug: 3\n", false, nil},
 	}
 
 	for _, tt := range testCases {
@@ -138,8 +158,8 @@ func TestUnmarshalYamlNTSC(t *testing.T) {
 		{"extra fields", yamlNTSC + `  extra_field: "string"` + yamlConstraints, false, nil},
 		{"invalid fields", yamlNTSC + "  debug true\n", false, nil},
 		{"empty input", "", true, &NTSCConfigPolicy{}},
-		{"null/empty fields", yamlNTSC + `  debug:\n`, false, nil},
-		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+		{"null/empty fields", yamlNTSC + "  debug:\n", true, &structNTSC}, // empty fields unmarshal to default value
+		{"wrong field type", "invariant_config:\n  debug: 3\n", false, nil},
 	}
 
 	for _, tt := range testCases {
@@ -154,27 +174,63 @@ func TestUnmarshalYamlNTSC(t *testing.T) {
 }
 
 const jsonConstraints = `
-constraints: {
-  resources: {
-    max_slots: 4
-  },
-  priority_limit: 10
-}
+    "constraints": {
+        "resources": {
+            "max_slots": 4
+        },
+        "priority_limit": 10
+    }
 `
 
 const jsonExperiment = `
-invariant_config: {
-  description: "test\nspecial\tchar",
-  environment: {
-    force_pull_image: false,
-    add_capabilities: ["cap1", "cap2"]
-  },
-  resources: {
-    slots: 1,
-  }
+    "invariant_config": {
+        "description": "test\nspecial\tchar",
+        "environment": {
+            "force_pull_image": false,
+            "add_capabilities": ["cap1", "cap2"]
+        },
+        "resources": {
+            "slots": 1
+        }
+    }
+`
+
+const jsonExtraField = `{
+    "invariant_config": {
+        "description": "test\nspecial\tchar",
+        "extra_field": "test"
+    }
 }
 `
 
+const jsonInvalidField = `{
+    "invariant_config": {
+        "description": "test\nspecial\tchar",
+        "debug"=true
+    }
+}`
+
+const jsonEmptyField = `{
+    "invariant_config": {
+        "description": "test\nspecial\tchar",
+        "debug":,
+        "environment": {
+            "force_pull_image": false,
+            "add_capabilities": ["cap1", "cap2"]
+        },
+        "resources": {
+            "slots": 1
+        }
+    }
+}`
+
+const jsonWrongFieldType = `{
+    "invariant_config": {
+        "description": "test\nspecial\tchar",
+        "debug": 4
+    }
+}`
+
 func TestUnmarshalJSONExperiment(t *testing.T) {
 	justConfig := structExperiment
 	justConfig.Constraints = nil
@@ -187,14 +243,14 @@ func TestUnmarshalJSONExperiment(t *testing.T) {
 		noErr  bool
 		output *ExperimentConfigPolicy
 	}{
-		{"valid json", jsonExperiment + jsonConstraints, true, &structExperiment},
-		{"just config", jsonExperiment, true, &justConfig},
-		{"just constraints", jsonConstraints, true, &justConstraints},
-		{"extra fields", jsonExperiment + `  extra_field: "string"` + jsonConstraints, false, nil},
-		{"invalid fields", jsonExperiment + "  debug{ true }\n", false, nil},
+		{"valid json", `{` + jsonExperiment + `,` + jsonConstraints + `}`, true, &structExperiment},
+		{"just config", `{` + jsonExperiment + `}`, true, &justConfig},
+		{"just constraints", `{` + jsonConstraints + `}`, true, &justConstraints},
+		{"extra fields", jsonExtraField, false, nil},
+		{"invalid fields", jsonInvalidField, false, nil},
 		{"empty input", "", true, &ExperimentConfigPolicy{}},
-		{"null/empty fields", jsonExperiment + "  debug:\n", false, nil},
-		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+		{"null/empty fields", jsonEmptyField, true, &structExperiment}, // empty fields unmarshal to default value
+		{"wrong field type", jsonWrongFieldType, false, nil},
 	}
 
 	for _, tt := range testCases {
@@ -209,16 +265,16 @@ func TestUnmarshalJSONExperiment(t *testing.T) {
 }
 
 const jsonNTSC = `
-invariant_config: {
-  description: "test\nspecial\tchar",
-  environment: {
-    force_pull_image: false,
-    add_capabilities: ["cap1", "cap2"]
-  },
-  resources: {
-    slots: 1,
-  }
-}
+    "invariant_config": {
+        "description": "test\nspecial\tchar",
+        "environment": {
+            "force_pull_image": false,
+            "add_capabilities": ["cap1", "cap2"]
+        },
+        "resources": {
+            "slots": 1
+        }
+    }
 `
 
 func TestUnmarshalJSONNTSC(t *testing.T) {
@@ -233,14 +289,14 @@ func TestUnmarshalJSONNTSC(t *testing.T) {
 		noErr  bool
 		output *NTSCConfigPolicy
 	}{
-		{"valid json", jsonNTSC + jsonConstraints, true, &structNTSC},
-		{"just config", jsonNTSC, true, &justConfig},
-		{"just constraints", jsonConstraints, true, &justConstraints},
-		{"extra fields", jsonNTSC + `  extra_field: "string"` + jsonConstraints, false, nil},
-		{"invalid fields", jsonNTSC + "  debug{ true }\n", false, nil},
+		{"valid json", `{` + jsonNTSC + `,` + jsonConstraints + `}`, true, &structNTSC},
+		{"just config", `{` + jsonNTSC + `}`, true, &justConfig},
+		{"just constraints", `{` + jsonConstraints + `}`, true, &justConstraints},
+		{"extra fields", jsonExtraField, false, nil},
+		{"invalid fields", jsonInvalidField, false, nil},
 		{"empty input", "", true, &NTSCConfigPolicy{}},
-		{"null/empty fields", jsonNTSC + "  debug:\n", false, nil},
-		{"wrong field type", "invariant_configs:\n  description: 3\n", false, nil},
+		{"null/empty fields", jsonEmptyField, true, &structNTSC}, // empty fields unmarshal to default value
+		{"wrong field type", jsonWrongFieldType, false, nil},
 	}
 
 	for _, tt := range testCases {