Enable Validation and Acceptance of New Parameter Configuration (#912)

* Init

* Enable Validation and Acceptance of New Parameter Configuration

Resolves #3584

- Allows for the supplying of configuration files for the daemon and
api-config subcommands.
- Properly validates the schema and contents of the supplied config file
- Enables default parameters to be disabled, allows integration tests to
  run with all parameters enabled

* Removed reflect deep equal for unit test

* Changed expectError to a string from a boolean

* Added checking of supplied file with enable all params

api/disabled_parameters.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"fmt"
+	"io/ioutil"
 	"net/http"
 	"strings"
 
@@ -10,6 +11,13 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+const (
+	disabledStatusStr    = "disabled"
+	enabledStatusStr     = "enabled"
+	requiredParameterStr = "required"
+	optionalParameterStr = "optional"
+)
+
 // DisplayDisabledMap is a struct that contains the necessary information
 // to output the current config to the screen
 type DisplayDisabledMap struct {
@@ -60,6 +68,125 @@ func (ddm *DisplayDisabledMap) addEntry(restPath string, requiredOrOptional stri
 	ddm.Data[restPath][requiredOrOptional] = append(ddm.Data[restPath][requiredOrOptional], mapEntry)
 }
 
+// validateSchema takes in a newly loaded DisplayDisabledMap and validates that all the
+// "strings" are the correct values.  For instance, it says that the sub-config is either
+// "required" or "optional" as well as making sure the string values for the parameters
+// are either "enabled" or "disabled".
+//
+// However, it does not validate whether the values actually exist.  That comes with the "Validate"
+// function on a DisabledMapConfig
+func (ddm *DisplayDisabledMap) validateSchema() error {
+	type innerStruct struct {
+		// IllegalParamTypes: list of the mis-spelled parameter types (i.e. not required or optional)
+		IllegalParamTypes []string
+		// IllegalParamStatus: list of parameter names with mis-spelled parameter status combined as a string
+		IllegalParamStatus []string
+	}
+
+	illegalSchema := make(map[string]innerStruct)
+
+	for restPath, entries := range ddm.Data {
+		tmp := innerStruct{}
+		for requiredOrOptional, paramList := range entries {
+
+			if requiredOrOptional != requiredParameterStr && requiredOrOptional != optionalParameterStr {
+				tmp.IllegalParamTypes = append(tmp.IllegalParamTypes, requiredOrOptional)
+			}
+
+			for _, paramDict := range paramList {
+				for paramName, paramStatus := range paramDict {
+					if paramStatus != disabledStatusStr && paramStatus != enabledStatusStr {
+						errorStr := fmt.Sprintf("%s : %s", paramName, paramStatus)
+						tmp.IllegalParamStatus = append(tmp.IllegalParamStatus, errorStr)
+					}
+				}
+			}
+
+			if len(tmp.IllegalParamTypes) != 0 || len(tmp.IllegalParamStatus) != 0 {
+				illegalSchema[restPath] = tmp
+			}
+		}
+	}
+
+	// No error if there are no entries
+	if len(illegalSchema) == 0 {
+		return nil
+	}
+
+	var sb strings.Builder
+
+	for restPath, iStruct := range illegalSchema {
+		_, _ = sb.WriteString(fmt.Sprintf("REST Path %s contained the following errors:\n", restPath))
+		if len(iStruct.IllegalParamTypes) != 0 {
+			_, _ = sb.WriteString(fmt.Sprintf("  -> Illegal Parameter Types: %v\n", iStruct.IllegalParamTypes))
+		}
+		if len(iStruct.IllegalParamStatus) != 0 {
+			_, _ = sb.WriteString(fmt.Sprintf("  -> Illegal Parameter Status: %v\n", iStruct.IllegalParamStatus))
+		}
+	}
+
+	return fmt.Errorf(sb.String())
+}
+
+// toDisabledMapConfig creates a disabled map config from a display disabled map.  If the swag pointer
+// is nil then no validation is performed on the disabled map config.  This is useful for unit tests
+func (ddm *DisplayDisabledMap) toDisabledMapConfig(swag *openapi3.Swagger) (*DisabledMapConfig, error) {
+	// Check that all the "strings" are valid
+	err := ddm.validateSchema()
+	if err != nil {
+		return nil, err
+	}
+
+	// We now should have a correctly formed DisplayDisabledMap.
+	// Let's turn that into a config
+	dmc := MakeDisabledMapConfig()
+
+	for restPath, entries := range ddm.Data {
+		var disabledParams []string
+		for _, paramList := range entries {
+			// We don't care if they are required or optional, only if the are disabled
+			for _, paramDict := range paramList {
+				for paramName, paramStatus := range paramDict {
+					if paramStatus != disabledStatusStr {
+						continue
+					}
+					disabledParams = append(disabledParams, paramName)
+				}
+			}
+		}
+
+		// Default to just get for now
+		dmc.addEntry(restPath, http.MethodGet, disabledParams)
+	}
+
+	if swag != nil {
+		err = dmc.validate(swag)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return dmc, nil
+}
+
+// MakeDisabledMapConfigFromFile loads a file containing a disabled map configuration.
+func MakeDisabledMapConfigFromFile(swag *openapi3.Swagger, filePath string) (*DisabledMapConfig, error) {
+	// First load the file...
+	f, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return nil, err
+	}
+
+	ddm := makeDisplayDisabledMap()
+
+	err = yaml.Unmarshal(f, &ddm.Data)
+	if err != nil {
+		return nil, err
+	}
+
+	return ddm.toDisabledMapConfig(swag)
+}
+
 // MakeDisplayDisabledMapFromConfig will make a DisplayDisabledMap that takes into account the DisabledMapConfig.
 // If limited is set to true, then only disabled parameters will be added to the DisplayDisabledMap
 func MakeDisplayDisabledMapFromConfig(swag *openapi3.Swagger, mapConfig *DisabledMapConfig, limited bool) *DisplayDisabledMap {
@@ -84,16 +211,16 @@ func MakeDisplayDisabledMapFromConfig(swag *openapi3.Swagger, mapConfig *Disable
 				var statusStr string
 
 				if parameterIsDisabled {
-					statusStr = "disabled"
+					statusStr = disabledStatusStr
 				} else {
-					statusStr = "enabled"
+					statusStr = enabledStatusStr
 				}
 
 				if pref.Value.Required {
-					rval.addEntry(restPath, "required", paramName, statusStr)
+					rval.addEntry(restPath, requiredParameterStr, paramName, statusStr)
 				} else {
 					// If the optional parameter is disabled, add it to the map
-					rval.addEntry(restPath, "optional", paramName, statusStr)
+					rval.addEntry(restPath, optionalParameterStr, paramName, statusStr)
 				}
 			}
 
@@ -173,7 +300,7 @@ func (dmc *DisabledMapConfig) addEntry(restPath string, operationName string, pa
 	dmc.Data[restPath][operationName] = parameterNames
 }
 
-// MakeDisabledMapConfig creates a new disabled map configuration
+// MakeDisabledMapConfig creates a new disabled map configuration with everything enabled
 func MakeDisabledMapConfig() *DisabledMapConfig {
 	return &DisabledMapConfig{
 		Data: make(map[string]map[string][]string),
@@ -211,14 +338,14 @@ func (edmc *ErrDisabledMapConfig) Error() string {
 	var sb strings.Builder
 	for k, v := range edmc.BadEntries {
 
-		// If the length of the list is zero then it is a mis-spelled REST path
+		// If the length of the list is zero then it is an unknown REST path
 		if len(v) == 0 {
-			_, _ = sb.WriteString(fmt.Sprintf("Mis-spelled REST Path: %s\n", k))
+			_, _ = sb.WriteString(fmt.Sprintf("Unknown REST Path: %s\n", k))
 			continue
 		}
 
 		for op, param := range v {
-			_, _ = sb.WriteString(fmt.Sprintf("REST Path %s (Operation: %s) contains mis-spelled parameters: %s\n", k, op, strings.Join(param, ",")))
+			_, _ = sb.WriteString(fmt.Sprintf("REST Path %s (Operation: %s) contains unknown parameters: %s\n", k, op, strings.Join(param, ",")))
 		}
 	}
 	return sb.String()

api/disabled_parameters_test.go

@@ -4,6 +4,8 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -14,6 +16,166 @@ import (
 	"github.com/algorand/indexer/api/generated/v2"
 )
 
+func TestToDisabledMapConfigFromFile(t *testing.T) {
+	expectedValue := DisabledMapConfig{Data: map[string]map[string][]string{
+		"/sampleEndpoint": {http.MethodGet: {"p2"}},
+	}}
+
+	configFile := filepath.Join("test_resources", "mock_disabled_map_config.yaml")
+
+	// Nil pointer for openapi3.swagger because we don't want any validation
+	// to be run on the config (they are made up endpoints)
+	loadedConfig, err := MakeDisabledMapConfigFromFile(nil, configFile)
+	require.NoError(t, err)
+	require.NotNil(t, loadedConfig)
+	require.Equal(t, expectedValue, *loadedConfig)
+}
+
+func TestToDisabledMapConfig(t *testing.T) {
+	type testingStruct struct {
+		name        string
+		ddm         *DisplayDisabledMap
+		dmc         *DisabledMapConfig
+		expectError string
+	}
+
+	tests := []testingStruct{
+		{"test 1",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required": {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional": {{"p3": "enabled"}},
+				}}},
+			&DisabledMapConfig{Data: map[string]map[string][]string{
+				"/sampleEndpoint": {http.MethodGet: {"p2"}},
+			}},
+
+			"",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+
+			// Nil pointer for openapi3.swagger because we don't want any validation
+			// to be run on the config
+			dmc, err := test.ddm.toDisabledMapConfig(nil)
+
+			if test.expectError != "" {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), test.expectError)
+			} else {
+				require.NoError(t, err)
+				require.True(t, reflect.DeepEqual(*dmc, *test.dmc))
+			}
+		})
+	}
+
+}
+
+func TestSchemaCheck(t *testing.T) {
+	type testingStruct struct {
+		name        string
+		ddm         *DisplayDisabledMap
+		expectError string
+	}
+	tests := []testingStruct{
+		{"test param types - good",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required": {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional": {{"p3": "enabled"}},
+				}},
+			},
+			"",
+		},
+
+		{"test param types - bad required",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required-FAKE": {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional":      {{"p3": "enabled"}},
+				}},
+			},
+			"required-FAKE",
+		},
+
+		{"test param types - bad optional",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required":      {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional-FAKE": {{"p3": "enabled"}},
+				}},
+			},
+			"optional-FAKE",
+		},
+
+		{"test param types - bad both",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required-FAKE": {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional-FAKE": {{"p3": "enabled"}},
+				}},
+			},
+			"required-FAKE optional-FAKE",
+		},
+
+		{"test param status - good",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required": {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional": {{"p3": "enabled"}},
+				}},
+			},
+			"",
+		},
+
+		{"test param status - bad required",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required": {{"p1": "enabled"}, {"p2": "disabled-FAKE"}},
+					"optional": {{"p3": "enabled"}},
+				}},
+			},
+			"p2",
+		},
+
+		{"test param status - bad optional",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required": {{"p1": "enabled"}, {"p2": "disabled"}},
+					"optional": {{"p3": "enabled-FAKE"}},
+				}},
+			},
+			"p3",
+		},
+
+		{"test param status - bad both",
+			&DisplayDisabledMap{Data: map[string]map[string][]map[string]string{
+				"/sampleEndpoint": {
+					"required": {{"p1": "enabled-FAKE"}, {"p2": "disabled"}},
+					"optional": {{"p3": "enabled-FAKE"}},
+				}},
+			},
+			"p1",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := test.ddm.validateSchema()
+
+			if test.expectError != "" {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), test.expectError)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+
+}
+
 func TestValidate(t *testing.T) {
 	// Validates that the default config is correctly spelled
 	dmc := GetDefaultDisabledMapConfigForPostgres()

api/test_resources/mock_disabled_map_config.yaml

@@ -0,0 +1,6 @@
+/sampleEndpoint:
+  required:
+    - p1 : enabled
+    - p2 : disabled
+  optional:
+    - p3 : enabled