Apply new Standard Ensembler routing policy with Custom Experiment En…

…gines (#237)

* Add route_name_path to standard ensembler config

* Update ensembler struct in test data to reflect latest schema

* Add additional validation tests to ensure that experiment mappings and route name path are not both set

* Add additional test case to verify the route name path is valid

* Refactor request parsing test

* Remove redundant regex route path name check

* Update regex expression to allow empty route_name_path string

* Remove required tag from EnsemblerStandardConfig.RouteNamePath

* Add e2e tests for std ensemblers with route_name_path

* Add additional conditional to e2e tests to check for ensembler pods only when docker/pyfunc types are used

* Fix e2e test data

* Add positive test to verify valid router config creation

* Fix router api test data

* Add additional unit tests for router service creation with std ensemblers with route name path

* Replace deprecated ioutil functions with corresponding io or os ones

* Fix lint comments on indenting conditional statements

* Fix e2e tests after changes from rebasing

* Fix bug in e2e test for std ensembler with route name path

* Fix sdk e2e tests

* Remove experiment mappings as a required field and update regex for route name path

* Add custom validator for standard ensembler config

* Remove standard ensembler config schema checks from request verification layer

* Make standard ensembler only parse non nil configs to fiber config

* Remove unnecessary route name path field in json request

* Remove duplicated error messages from EnsemblerStandardConfig custom validation checks

* Remove regex expression for route_name_path

api/api/openapi.bundle.yaml

@@ -1980,6 +1980,7 @@ components:
             timeout: timeout
           updated_at: 2000-01-23T04:56:07.000+00:00
           standard_config:
+            route_name_path: route_name_path
             experiment_mappings:
             - treatment: treatment-1
               route: route-1
@@ -2275,6 +2276,7 @@ components:
           timeout: timeout
         updated_at: 2000-01-23T04:56:07.000+00:00
         standard_config:
+          route_name_path: route_name_path
           experiment_mappings:
           - treatment: treatment-1
             route: route-1
@@ -2335,6 +2337,7 @@ components:
     EnsemblerStandardConfig:
       description: ensembler config when ensembler type is standard
       example:
+        route_name_path: route_name_path
         experiment_mappings:
         - treatment: treatment-1
           route: route-1
@@ -2348,8 +2351,8 @@ components:
           items:
             $ref: '#/components/schemas/EnsemblerStandardConfig_experiment_mappings'
           type: array
-      required:
-      - experiment_mappings
+        route_name_path:
+          type: string
       type: object
     EnsemblerDockerConfig:
       description: ensembler config when ensembler type is docker
@@ -2626,6 +2629,7 @@ components:
               timeout: timeout
             updated_at: 2000-01-23T04:56:07.000+00:00
             standard_config:
+              route_name_path: route_name_path
               experiment_mappings:
               - treatment: treatment-1
                 route: route-1
@@ -2781,6 +2785,7 @@ components:
             timeout: timeout
           updated_at: 2000-01-23T04:56:07.000+00:00
           standard_config:
+            route_name_path: route_name_path
             experiment_mappings:
             - treatment: treatment-1
               route: route-1

api/api/specs/routers.yaml

@@ -794,8 +794,6 @@ components:
       description: "ensembler config when ensembler type is standard"
       type: "object"
       nullable: true
-      required:
-        - "experiment_mappings"
       properties:
         experiment_mappings:
           type: "array"
@@ -818,6 +816,8 @@ components:
                 description: "route id of the routes configured in the router"
                 type: "string"
                 example: "route-1"
+        route_name_path:
+          type: "string"
 
     EnsemblerDockerConfig:
       description: "ensembler config when ensembler type is docker"

api/e2e/test/00_all_e2e_test.go

@@ -79,6 +79,10 @@ func TestEndToEnd(t *testing.T) {
 		t.Parallel()
 		t.Run("DeployRouter", TestDeployRouterWithTrafficRules)
 	})
+	t.Run("TestStandardEnsemblerWithRouteNamePath", func(t *testing.T) {
+		t.Parallel()
+		t.Run("DeployRouter", TestDeployRouterWithRouteNamePathInStandardEnsembler)
+	})
 }
 
 func TestMain(m *testing.M) {

api/e2e/test/01_create_router_test.go

@@ -49,20 +49,21 @@ func TestCreateRouter(t *testing.T) {
 						response.StatusCode, string(responsePayload))
 					actualResponse := gjson.GetBytes(responsePayload, "response").String()
 					expectedResponse := `{
-					  "experiment": {
-						"configuration": {
-							"foo":"bar"
-						}
-					  },
-					  "route_responses": [
-						{
-						  "data": {
-							"version": "control"
-						  },
-						  "is_default": false,
-						  "route": "control"
-						}
-					  ]
+						"experiment": {
+							"configuration": {
+								"foo":"bar",
+								"route_name":"treatment-a"
+							}
+						},
+						"route_responses": [
+							{
+								"data": {
+									"version": "control"
+								},
+								"is_default": false,
+								"route": "control"
+							}
+					  	]
 					}`
 					assert.JSONEq(t, expectedResponse, actualResponse)
 				})
@@ -94,7 +95,8 @@ func TestCreateRouter(t *testing.T) {
 								"response": {
 									"experiment": {
 										"configuration": {
-											"foo": "bar"
+											"foo": "bar",
+											"route_name":"treatment-a"
 										}
 									},
 									"route_responses": [
@@ -120,7 +122,8 @@ func TestCreateRouter(t *testing.T) {
 								"response": {
 									"experiment": {
 										"configuration": {
-											"bar": "baz"
+											"bar": "baz",
+											"route_name":"control"
 										}
 									},
 									"route_responses": [

api/e2e/test/02_update_router_invalid_test.go

@@ -145,7 +145,8 @@ func TestUpdateRouterInvalidConfig(t *testing.T) {
 			expectedResponse := `{
 			  "experiment": {
 				"configuration": {
-					"foo":"bar"
+					"foo":"bar",
+					"route_name":"treatment-a"
 				}
 			  },
 			  "route_responses": [

api/e2e/test/03_create_router_version_test.go

@@ -130,20 +130,21 @@ func TestCreateRouterVersion(t *testing.T) {
 				response.StatusCode, string(responsePayload))
 			actualResponse := gjson.GetBytes(responsePayload, "response").String()
 			expectedResponse := `{
-			  "experiment": {
-				"configuration": {
-					"foo":"bar"
-				}
-			  },
-			  "route_responses": [
-				{
-				  "data": {
-					"version": "control"
-				  },
-				  "is_default": false,
-				  "route": "control"
-				}
-			  ]
+				"experiment": {
+					"configuration": {
+						"foo":"bar",
+						"route_name":"treatment-a"
+					}
+				},
+				"route_responses": [
+					{
+						"data": {
+							"version": "control"
+						},
+						"is_default": false,
+						"route": "control"
+					}
+				]
 			}`
 			assert.JSONEq(t, expectedResponse, actualResponse)
 		})

api/e2e/test/04_undeploy_router_test.go

@@ -5,7 +5,7 @@ package e2e
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"testing"
 	"time"
@@ -42,7 +42,7 @@ func TestUndeployRouter(t *testing.T) {
 	response, err := globalTestContext.httpClient.Do(req)
 	require.NoError(t, err)
 	assert.Equal(t, http.StatusOK, response.StatusCode)
-	responseBody, err := ioutil.ReadAll(response.Body)
+	responseBody, err := io.ReadAll(response.Body)
 	defer response.Body.Close()
 	require.NoError(t, err)
 	t.Log("Undeploy Response:", string(responseBody))

api/e2e/test/06_deploy_valid_config_test.go

@@ -5,7 +5,7 @@ package e2e
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"testing"
 
@@ -42,7 +42,7 @@ func TestDeployValidConfig(t *testing.T) {
 	response, err := globalTestContext.httpClient.Do(req)
 	require.NoError(t, err)
 	assert.Equal(t, http.StatusAccepted, response.StatusCode, readBody(t, response))
-	responseBody, err := ioutil.ReadAll(response.Body)
+	responseBody, err := io.ReadAll(response.Body)
 	require.NoError(t, err)
 	t.Log("Deploy Response:", string(responseBody))
 

api/e2e/test/09_deploy_router_with_route_name_path_std_ensembler_test.go

@@ -0,0 +1,55 @@
+//go:build e2e
+
+package e2e
+
+import (
+	"net/http"
+	"path/filepath"
+	"testing"
+
+	"github.com/caraml-dev/turing/api/turing/models"
+	"github.com/stretchr/testify/assert"
+)
+
+/*
+Steps:
+Create a new router with valid config for the router.
+a. Test GET router immediately > empty config
+b. Wait for success response from deployment
+c. Test GET router version > status shows "deployed"
+d. Test GET router > config section shows version 1, status "deployed"
+e. Test cluster that deployments exist
+f. Make a request to the router, validate the response.
+*/
+func TestDeployRouterWithRouteNamePathInStandardEnsembler(t *testing.T) {
+	// Create router
+	t.Log("Creating router")
+	data := makeRouterPayload(
+		filepath.Join("testdata", "create_router_std_ensembler_route_name_path.json.tmpl"),
+		globalTestContext)
+
+	withDeployedRouter(t, data,
+		func(router *models.Router) {
+			t.Log("Testing router endpoint: POST " + router.Endpoint)
+			withRouterResponse(t,
+				http.MethodPost,
+				router.Endpoint,
+				http.Header{
+					"Content-Type":  {"application/json"},
+					"X-Mirror-Body": {"true"},
+				},
+				`{"client": {"id": 4}}`,
+				func(response *http.Response, responsePayload []byte) {
+					assert.Equal(t, http.StatusOK, response.StatusCode,
+						"Unexpected response (code %d): %s",
+						response.StatusCode, string(responsePayload))
+					actualResponse := string(responsePayload)
+					expectedResponse := `{
+					  "version" : "treatment-a"
+					}`
+					assert.JSONEq(t, expectedResponse, actualResponse)
+				})
+		},
+		nil,
+	)
+}

api/e2e/test/helpers_api_test.go

@@ -6,7 +6,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"testing"
 	"time"
@@ -34,7 +34,7 @@ func getRouter(
 	}
 	defer resp.Body.Close()
 
-	respBytes, err := ioutil.ReadAll(resp.Body)
+	respBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, err
 	}
@@ -61,7 +61,7 @@ func getRouterByName(
 	defer resp.Body.Close()
 
 	routers := make([]*models.Router, 0)
-	data, err := ioutil.ReadAll(resp.Body)
+	data, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, err
 	}
@@ -96,7 +96,7 @@ func getRouterVersion(
 	}
 	defer resp.Body.Close()
 
-	respBytes, err := ioutil.ReadAll(resp.Body)
+	respBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, err
 	}
@@ -143,7 +143,7 @@ func waitDeployVersion(
 }
 
 func getPodLogs(t *testing.T, resp *http.Response) []service.PodLog {
-	data, err := ioutil.ReadAll(resp.Body)
+	data, err := io.ReadAll(resp.Body)
 	if err != nil {
 		t.Error(err)
 	}
@@ -175,7 +175,7 @@ func withDeployedRouter(
 	require.NoError(t, err)
 	defer resp.Body.Close()
 
-	responsePayload, err := ioutil.ReadAll(resp.Body)
+	responsePayload, err := io.ReadAll(resp.Body)
 	require.NoError(t, err)
 
 	created := models.Router{}
@@ -275,7 +275,8 @@ func withDeployedRouter(
 			globalTestContext.ProjectName,
 			fmt.Sprintf("%s-turing-enricher-%d-0-deployment", router.Name, routerVersion.Version)))
 	}
-	if router.CurrRouterVersion.Ensembler != nil {
+	if router.CurrRouterVersion.Ensembler != nil &&
+		router.CurrRouterVersion.Ensembler.Type != models.EnsemblerStandardType {
 		require.True(t, isDeploymentExists(
 			globalTestContext.clusterClients,
 			globalTestContext.ProjectName,

api/e2e/test/helpers_test.go

@@ -4,8 +4,9 @@ package e2e
 
 import (
 	"bytes"
-	"io/ioutil"
+	"io"
 	"net/http"
+	"os"
 	"testing"
 	"text/template"
 
@@ -16,7 +17,7 @@ func readBody(t *testing.T, resp *http.Response) string {
 	if resp.Body == nil {
 		return ""
 	}
-	data, err := ioutil.ReadAll(resp.Body)
+	data, err := io.ReadAll(resp.Body)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -25,7 +26,7 @@ func readBody(t *testing.T, resp *http.Response) string {
 
 // make payload to create new router from a template file
 func makeRouterPayload(payloadTemplateFile string, args TestContext) []byte {
-	data, err := ioutil.ReadFile(payloadTemplateFile)
+	data, err := os.ReadFile(payloadTemplateFile)
 	if err != nil {
 		panic(err)
 	}
@@ -52,7 +53,7 @@ func withRouterResponse(
 	body string,
 	assertion func(response *http.Response, responsePayload []byte)) {
 
-	req, err := http.NewRequest(method, url, ioutil.NopCloser(bytes.NewReader([]byte(body))))
+	req, err := http.NewRequest(method, url, io.NopCloser(bytes.NewReader([]byte(body))))
 	require.NoError(t, err)
 
 	req.Header = headers
@@ -61,7 +62,7 @@ func withRouterResponse(
 	require.NoError(t, err)
 	defer resp.Body.Close()
 
-	responseBytes, err := ioutil.ReadAll(resp.Body)
+	responseBytes, err := io.ReadAll(resp.Body)
 	require.NoError(t, err)
 
 	assertion(resp, responseBytes)