diff --git a/api/turing/api/request/request.go b/api/turing/api/request/request.go index 93cbf1195..96cce2585 100644 --- a/api/turing/api/request/request.go +++ b/api/turing/api/request/request.go @@ -107,58 +107,55 @@ func (r CreateOrUpdateRouterRequest) BuildRouter(projectID models.ID) *models.Ro } // BuildRouterVersion builds the router version model from the entire request payload -func (r CreateOrUpdateRouterRequest) BuildRouterVersion( +func (r RouterConfig) BuildRouterVersion( router *models.Router, defaults *config.RouterDefaults, cryptoSvc service.CryptoService, expSvc service.ExperimentsService, ensemblersSvc service.EnsemblersService, ) (rv *models.RouterVersion, err error) { - if r.Config == nil { - return nil, errors.New("router config is empty") - } rv = &models.RouterVersion{ RouterID: router.ID, Router: router, Image: defaults.Image, Status: models.RouterVersionStatusPending, - Routes: r.Config.Routes, - DefaultRouteID: r.Config.DefaultRouteID, - TrafficRules: r.Config.TrafficRules, + Routes: r.Routes, + DefaultRouteID: r.DefaultRouteID, + TrafficRules: r.TrafficRules, ExperimentEngine: &models.ExperimentEngine{ - Type: r.Config.ExperimentEngine.Type, + Type: r.ExperimentEngine.Type, }, - ResourceRequest: r.Config.ResourceRequest, - Timeout: r.Config.Timeout, + ResourceRequest: r.ResourceRequest, + Timeout: r.Timeout, LogConfig: &models.LogConfig{ LogLevel: routercfg.LogLevel(defaults.LogLevel), CustomMetricsEnabled: defaults.CustomMetricsEnabled, FiberDebugLogEnabled: defaults.FiberDebugLogEnabled, JaegerEnabled: defaults.JaegerEnabled, - ResultLoggerType: r.Config.LogConfig.ResultLoggerType, + ResultLoggerType: r.LogConfig.ResultLoggerType, }, } - if r.Config.Enricher != nil { - rv.Enricher = r.Config.Enricher.BuildEnricher() + if r.Enricher != nil { + rv.Enricher = r.Enricher.BuildEnricher() } - if r.Config.Ensembler != nil { + if r.Ensembler != nil { // Ensure ensembler config is set based on the ensembler type - if r.Config.Ensembler.Type == models.EnsemblerDockerType && r.Config.Ensembler.DockerConfig == nil { + if r.Ensembler.Type == models.EnsemblerDockerType && r.Ensembler.DockerConfig == nil { return nil, errors.New("missing ensembler docker config") } - if r.Config.Ensembler.Type == models.EnsemblerStandardType && r.Config.Ensembler.StandardConfig == nil { + if r.Ensembler.Type == models.EnsemblerStandardType && r.Ensembler.StandardConfig == nil { return nil, errors.New("missing ensembler standard config") } - if r.Config.Ensembler.Type == models.EnsemblerPyFuncType { - if r.Config.Ensembler.PyfuncConfig == nil { + if r.Ensembler.Type == models.EnsemblerPyFuncType { + if r.Ensembler.PyfuncConfig == nil { return nil, errors.New("missing ensembler pyfunc reference config") } // Verify if the ensembler given by its ProjectID and EnsemblerID exist ensembler, err := ensemblersSvc.FindByID( - *r.Config.Ensembler.PyfuncConfig.EnsemblerID, + *r.Ensembler.PyfuncConfig.EnsemblerID, service.EnsemblersFindByIDOptions{ - ProjectID: r.Config.Ensembler.PyfuncConfig.ProjectID, + ProjectID: r.Ensembler.PyfuncConfig.ProjectID, }) if err != nil { return nil, fmt.Errorf("failed to find specified ensembler: %w", err) @@ -172,20 +169,20 @@ func (r CreateOrUpdateRouterRequest) BuildRouterVersion( return nil, fmt.Errorf("only pyfunc ensemblers allowed; ensembler type given: %T", v) } } - rv.Ensembler = r.Config.Ensembler + rv.Ensembler = r.Ensembler } switch rv.LogConfig.ResultLoggerType { case models.BigQueryLogger: rv.LogConfig.BigQueryConfig = &models.BigQueryConfig{ - Table: r.Config.LogConfig.BigQueryConfig.Table, - ServiceAccountSecret: r.Config.LogConfig.BigQueryConfig.ServiceAccountSecret, + Table: r.LogConfig.BigQueryConfig.Table, + ServiceAccountSecret: r.LogConfig.BigQueryConfig.ServiceAccountSecret, BatchLoad: true, // default for now } case models.KafkaLogger: rv.LogConfig.KafkaConfig = &models.KafkaConfig{ - Brokers: r.Config.LogConfig.KafkaConfig.Brokers, - Topic: r.Config.LogConfig.KafkaConfig.Topic, - SerializationFormat: r.Config.LogConfig.KafkaConfig.SerializationFormat, + Brokers: r.LogConfig.KafkaConfig.Brokers, + Topic: r.LogConfig.KafkaConfig.Topic, + SerializationFormat: r.LogConfig.KafkaConfig.SerializationFormat, } } if rv.ExperimentEngine.Type != models.ExperimentEngineTypeNop { @@ -203,15 +200,15 @@ func (r CreateOrUpdateRouterRequest) BuildRouterVersion( } // BuildExperimentEngineConfig creates the Experiment config from the given input properties -func (r CreateOrUpdateRouterRequest) BuildExperimentEngineConfig( +func (r RouterConfig) BuildExperimentEngineConfig( router *models.Router, cryptoSvc service.CryptoService, expSvc service.ExperimentsService, ) (json.RawMessage, error) { - rawExpConfig := r.Config.ExperimentEngine.Config + rawExpConfig := r.ExperimentEngine.Config // Handle missing passkey / encrypt it in Standard experiment config - if expSvc.IsStandardExperimentManager(r.Config.ExperimentEngine.Type) { + if expSvc.IsStandardExperimentManager(r.ExperimentEngine.Type) { // Convert the new config to the standard type expConfig, err := manager.ParseStandardExperimentConfig(rawExpConfig) if err != nil { @@ -221,7 +218,7 @@ func (r CreateOrUpdateRouterRequest) BuildExperimentEngineConfig( if expConfig.Client.Passkey == "" { // Extract existing router version config if router.CurrRouterVersion != nil && - router.CurrRouterVersion.ExperimentEngine.Type == r.Config.ExperimentEngine.Type { + router.CurrRouterVersion.ExperimentEngine.Type == r.ExperimentEngine.Type { currVerExpConfig, err := manager.ParseStandardExperimentConfig(router.CurrRouterVersion.ExperimentEngine.Config) if err != nil { return nil, fmt.Errorf("Error parsing existing experiment config: %v", err) diff --git a/api/turing/api/request/request_test.go b/api/turing/api/request/request_test.go index b46e53ac4..bfc12c100 100644 --- a/api/turing/api/request/request_test.go +++ b/api/turing/api/request/request_test.go @@ -65,23 +65,40 @@ func makeTuringExperimentConfig(clientPasskey string) json.RawMessage { return expEngineConfig } -var createOrUpdateRequest = CreateOrUpdateRouterRequest{ - Environment: "env", - Name: "router", - Config: &RouterConfig{ - Routes: []*models.Route{ - { - ID: "default", - Type: "PROXY", - Endpoint: "endpoint", - Timeout: "6s", - }, +var validRouterConfig = RouterConfig{ + Routes: []*models.Route{ + { + ID: "default", + Type: "PROXY", + Endpoint: "endpoint", + Timeout: "6s", }, - DefaultRouteID: "default", - ExperimentEngine: &ExperimentEngineConfig{ - Type: "standard", - Config: makeTuringExperimentConfig("dummy_passkey"), + }, + DefaultRouteID: "default", + ExperimentEngine: &ExperimentEngineConfig{ + Type: "standard", + Config: makeTuringExperimentConfig("dummy_passkey"), + }, + ResourceRequest: &models.ResourceRequest{ + MinReplica: 0, + MaxReplica: 5, + CPURequest: resource.Quantity{ + Format: "500M", + }, + MemoryRequest: resource.Quantity{ + Format: "1G", + }, + }, + Timeout: "10s", + LogConfig: &LogConfig{ + ResultLoggerType: "bigquery", + BigQueryConfig: &BigQueryConfig{ + Table: "project.dataset.table", + ServiceAccountSecret: "service_account", }, + }, + Enricher: &EnricherEnsemblerConfig{ + Image: "lala", ResourceRequest: &models.ResourceRequest{ MinReplica: 0, MaxReplica: 5, @@ -92,67 +109,63 @@ var createOrUpdateRequest = CreateOrUpdateRouterRequest{ Format: "1G", }, }, - Timeout: "10s", - LogConfig: &LogConfig{ - ResultLoggerType: "bigquery", - BigQueryConfig: &BigQueryConfig{ - Table: "project.dataset.table", - ServiceAccountSecret: "service_account", + Endpoint: "endpoint", + Timeout: "6s", + Port: 8080, + Env: []*models.EnvVar{ + { + Name: "key", + Value: "value", }, }, - Enricher: &EnricherEnsemblerConfig{ - Image: "lala", + }, + Ensembler: &models.Ensembler{ + Type: "docker", + DockerConfig: &models.EnsemblerDockerConfig{ + Image: "nginx", ResourceRequest: &models.ResourceRequest{ - MinReplica: 0, - MaxReplica: 5, - CPURequest: resource.Quantity{ - Format: "500M", - }, - MemoryRequest: resource.Quantity{ - Format: "1G", - }, - }, - Endpoint: "endpoint", - Timeout: "6s", - Port: 8080, - Env: []*models.EnvVar{ - { - Name: "key", - Value: "value", - }, - }, - }, - Ensembler: &models.Ensembler{ - Type: "docker", - DockerConfig: &models.EnsemblerDockerConfig{ - Image: "nginx", - ResourceRequest: &models.ResourceRequest{ - CPURequest: resource.Quantity{Format: "500m"}, - MemoryRequest: resource.Quantity{Format: "1Gi"}, - }, - Timeout: "5s", + CPURequest: resource.Quantity{Format: "500m"}, + MemoryRequest: resource.Quantity{Format: "1Gi"}, }, + Timeout: "5s", }, }, } -var createOrUpdateInvalidRequest = CreateOrUpdateRouterRequest{ - Environment: "env", - Name: "router", - Config: &RouterConfig{ - Routes: []*models.Route{ - { - ID: "default", - Type: "PROXY", - Endpoint: "endpoint", - Timeout: "6s", - }, +var invalidRouterConfig = RouterConfig{ + Routes: []*models.Route{ + { + ID: "default", + Type: "PROXY", + Endpoint: "endpoint", + Timeout: "6s", }, - DefaultRouteID: "default", - ExperimentEngine: &ExperimentEngineConfig{ - Type: "standard", - Config: makeTuringExperimentConfig("dummy_passkey"), + }, + DefaultRouteID: "default", + ExperimentEngine: &ExperimentEngineConfig{ + Type: "standard", + Config: makeTuringExperimentConfig("dummy_passkey"), + }, + ResourceRequest: &models.ResourceRequest{ + MinReplica: 0, + MaxReplica: 5, + CPURequest: resource.Quantity{ + Format: "500M", }, + MemoryRequest: resource.Quantity{ + Format: "1G", + }, + }, + Timeout: "10s", + LogConfig: &LogConfig{ + ResultLoggerType: "bigquery", + BigQueryConfig: &BigQueryConfig{ + Table: "project.dataset.table", + ServiceAccountSecret: "service_account", + }, + }, + Enricher: &EnricherEnsemblerConfig{ + Image: "lala", ResourceRequest: &models.ResourceRequest{ MinReplica: 0, MaxReplica: 5, @@ -163,51 +176,42 @@ var createOrUpdateInvalidRequest = CreateOrUpdateRouterRequest{ Format: "1G", }, }, - Timeout: "10s", - LogConfig: &LogConfig{ - ResultLoggerType: "bigquery", - BigQueryConfig: &BigQueryConfig{ - Table: "project.dataset.table", - ServiceAccountSecret: "service_account", + Endpoint: "endpoint", + Timeout: "6s", + Port: 8080, + Env: []*models.EnvVar{ + { + Name: "key", + Value: "value", }, }, - Enricher: &EnricherEnsemblerConfig{ - Image: "lala", + }, + Ensembler: &models.Ensembler{ + Type: "pyfunc", + PyfuncConfig: &models.EnsemblerPyfuncConfig{ + ProjectID: models.NewID(11), + EnsemblerID: models.NewID(12), ResourceRequest: &models.ResourceRequest{ - MinReplica: 0, - MaxReplica: 5, - CPURequest: resource.Quantity{ - Format: "500M", - }, - MemoryRequest: resource.Quantity{ - Format: "1G", - }, - }, - Endpoint: "endpoint", - Timeout: "6s", - Port: 8080, - Env: []*models.EnvVar{ - { - Name: "key", - Value: "value", - }, - }, - }, - Ensembler: &models.Ensembler{ - Type: "pyfunc", - PyfuncConfig: &models.EnsemblerPyfuncConfig{ - ProjectID: models.NewID(11), - EnsemblerID: models.NewID(12), - ResourceRequest: &models.ResourceRequest{ - CPURequest: resource.Quantity{Format: "500m"}, - MemoryRequest: resource.Quantity{Format: "1Gi"}, - }, - Timeout: "5s", + CPURequest: resource.Quantity{Format: "500m"}, + MemoryRequest: resource.Quantity{Format: "1Gi"}, }, + Timeout: "5s", }, }, } +var createOrUpdateRequest = CreateOrUpdateRouterRequest{ + Environment: "env", + Name: "router", + Config: &validRouterConfig, +} + +var createOrUpdateInvalidRequest = CreateOrUpdateRouterRequest{ + Environment: "env", + Name: "router", + Config: &invalidRouterConfig, +} + func TestRequestBuildRouter(t *testing.T) { projectID := models.ID(1) expected := &models.Router{ @@ -309,7 +313,7 @@ func TestRequestBuildRouterVersionLoggerConfiguration(t *testing.T) { // Set up mock Ensembler service ensemblerSvc := &mocks.EnsemblersService{} - got, err := baseRequest.BuildRouterVersion(router, &routerDefault, cryptoSvc, expSvc, ensemblerSvc) + got, err := baseRequest.Config.BuildRouterVersion(router, &routerDefault, cryptoSvc, expSvc, ensemblerSvc) assert.NoError(t, err) assert.Equal(t, got.LogConfig, tt.expectedLogConfig) }) @@ -418,7 +422,7 @@ func TestRequestBuildRouterVersionWithDefaults(t *testing.T) { // Set up mock Ensembler service ensemblerSvc := &mocks.EnsemblersService{} - got, err := createOrUpdateRequest.BuildRouterVersion(router, &defaults, cryptoSvc, expSvc, ensemblerSvc) + got, err := validRouterConfig.BuildRouterVersion(router, &defaults, cryptoSvc, expSvc, ensemblerSvc) require.NoError(t, err) expected.Model = got.Model assertgotest.DeepEqual(t, expected, *got) @@ -455,7 +459,7 @@ func TestRequestBuildRouterVersionWithUnavailablePyFuncEnsembler(t *testing.T) { ensemblerSvc.On("FindByID", mock.Anything, mock.Anything).Return(nil, errors.New("record not found")) // Update the router with an invalid request - got, err := createOrUpdateInvalidRequest.BuildRouterVersion(router, &defaults, cryptoSvc, expSvc, ensemblerSvc) + got, err := invalidRouterConfig.BuildRouterVersion(router, &defaults, cryptoSvc, expSvc, ensemblerSvc) assert.EqualError(t, err, "failed to find specified ensembler: record not found") assert.Nil(t, got) @@ -475,42 +479,36 @@ func TestBuildExperimentEngineConfig(t *testing.T) { // Define tests tests := map[string]struct { - req CreateOrUpdateRouterRequest + req RouterConfig router *models.Router expected json.RawMessage err string }{ "failure | std engine | missing curr version passkey": { - req: CreateOrUpdateRouterRequest{ - Config: &RouterConfig{ - ExperimentEngine: &ExperimentEngineConfig{ - Type: "standard-manager", - Config: json.RawMessage(`{"client": {"username": "client-name"}}`), - }, + req: RouterConfig{ + ExperimentEngine: &ExperimentEngineConfig{ + Type: "standard-manager", + Config: json.RawMessage(`{"client": {"username": "client-name"}}`), }, }, router: &models.Router{}, err: "Passkey must be configured", }, "failure | std engine | encrypt passkey error": { - req: CreateOrUpdateRouterRequest{ - Config: &RouterConfig{ - ExperimentEngine: &ExperimentEngineConfig{ - Type: "standard-manager", - Config: json.RawMessage(`{"client": {"username": "client-name", "passkey": "passkey-bad"}}`), - }, + req: RouterConfig{ + ExperimentEngine: &ExperimentEngineConfig{ + Type: "standard-manager", + Config: json.RawMessage(`{"client": {"username": "client-name", "passkey": "passkey-bad"}}`), }, }, router: &models.Router{}, err: "Passkey could not be encrypted: test-encrypt-error", }, "success | std engine | use curr version passkey": { - req: CreateOrUpdateRouterRequest{ - Config: &RouterConfig{ - ExperimentEngine: &ExperimentEngineConfig{ - Type: "standard-manager", - Config: json.RawMessage(`{"client": {"username": "client-name"}}`), - }, + req: RouterConfig{ + ExperimentEngine: &ExperimentEngineConfig{ + Type: "standard-manager", + Config: json.RawMessage(`{"client": {"username": "client-name"}}`), }, }, router: &models.Router{ @@ -528,12 +526,10 @@ func TestBuildExperimentEngineConfig(t *testing.T) { }`), }, "success | std engine | use new passkey": { - req: CreateOrUpdateRouterRequest{ - Config: &RouterConfig{ - ExperimentEngine: &ExperimentEngineConfig{ - Type: "standard-manager", - Config: json.RawMessage(`{"client": {"username": "client-name", "passkey": "passkey"}}`), - }, + req: RouterConfig{ + ExperimentEngine: &ExperimentEngineConfig{ + Type: "standard-manager", + Config: json.RawMessage(`{"client": {"username": "client-name", "passkey": "passkey"}}`), }, }, router: &models.Router{}, @@ -544,12 +540,10 @@ func TestBuildExperimentEngineConfig(t *testing.T) { }`), }, "success | custom engine": { - req: CreateOrUpdateRouterRequest{ - Config: &RouterConfig{ - ExperimentEngine: &ExperimentEngineConfig{ - Type: "custom-manager", - Config: json.RawMessage("[1, 2]"), - }, + req: RouterConfig{ + ExperimentEngine: &ExperimentEngineConfig{ + Type: "custom-manager", + Config: json.RawMessage("[1, 2]"), }, }, router: &models.Router{}, diff --git a/api/turing/api/router_versions_api.go b/api/turing/api/router_versions_api.go index 998030677..b26be6b90 100644 --- a/api/turing/api/router_versions_api.go +++ b/api/turing/api/router_versions_api.go @@ -48,24 +48,21 @@ func (c RouterVersionsController) CreateRouterVersion(r *http.Request, vars Requ return errResp } - requestConfig := body.(*request.RouterConfig) - - // This is used temporarily in order to reuse the BuildRouterVersion method - dummyRouterRequest := request.CreateOrUpdateRouterRequest{ - Environment: "", - Name: "", - Config: requestConfig, - } + request := body.(*request.RouterConfig) // Create new version var routerVersion *models.RouterVersion - rVersion, err := dummyRouterRequest.BuildRouterVersion( + if request == nil { + return InternalServerError("unable to create router version", "router config is empty") + } + + routerVersion, err := request.BuildRouterVersion( router, c.RouterDefaults, c.AppContext.CryptoService, c.AppContext.ExperimentsService, c.EnsemblersService) if err == nil { // Save router version, re-assign the value of err - rVersion.Status = models.RouterVersionStatusUndeployed - routerVersion, err = c.RouterVersionsService.Save(rVersion) + routerVersion.Status = models.RouterVersionStatusUndeployed + routerVersion, err = c.RouterVersionsService.Save(routerVersion) } if err != nil { diff --git a/api/turing/api/routers_api.go b/api/turing/api/routers_api.go index 0940a3142..21c60ecbe 100644 --- a/api/turing/api/routers_api.go +++ b/api/turing/api/routers_api.go @@ -91,7 +91,11 @@ func (c RoutersController) CreateRouter( // then create the new version var routerVersion *models.RouterVersion - rVersion, err := request.BuildRouterVersion( + if request.Config == nil { + return InternalServerError("unable to create router", "router config is empty") + } + + rVersion, err := request.Config.BuildRouterVersion( router, c.RouterDefaults, c.AppContext.CryptoService, c.AppContext.ExperimentsService, c.EnsemblersService) if err == nil { // Save router version @@ -152,7 +156,11 @@ func (c RoutersController) UpdateRouter(r *http.Request, vars RequestVars, body // Create new version var routerVersion *models.RouterVersion - rVersion, err := request.BuildRouterVersion( + if request.Config == nil { + return InternalServerError("unable to update router", "router config is empty") + } + + rVersion, err := request.Config.BuildRouterVersion( router, c.RouterDefaults, c.AppContext.CryptoService, c.AppContext.ExperimentsService, c.EnsemblersService) if err == nil { // Save router version, re-assign the value of err