Skip to content

Commit 8a02334

Browse files
jonathanv28Jonathan Victor GoklasJonathan Victor Goklas
authored
Make update project settings endpoint call URL with template payload + add a labels blacklist (caraml-dev#99)
* first initialization * add update project settings configs * fix limit and package errors * fix limit, package, and fmt errors * fix naming errors * fix typo * fix error * try error handling when update project not specified * try fix * try fix error * add labels blacklist config to UI * fix error * fix as from pr comments, add label blacklist & ui toast, no unit tests updated * gofmt-ed files * handle labels blacklist, handle webhook scenarios, handle endpoint nil, update unit tests * fix test-api * fix api errors * rename config, add fail scenario test * add yaml file * add fix for ui blacklist, refactor update project service layer * fix lint code * fix error code * refactor labels blacklist to list * fix lint code * refactor initialisation * rename map * fix ui --------- Co-authored-by: Jonathan Victor Goklas <[email protected]> Co-authored-by: Jonathan Victor Goklas <[email protected]>
1 parent 688c2fc commit 8a02334

File tree

13 files changed

+887
-97
lines changed

13 files changed

+887
-97
lines changed

‎api/api/api_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,15 @@ func (s *APITestSuite) SetupTest() {
5858
Mlflow: &config.MlflowConfig{
5959
TrackingURL: "http://mlflow:5000",
6060
},
61+
UpdateProjectConfig: &config.UpdateProjectConfig{
62+
Endpoint: "",
63+
PayloadTemplate: "template-payload",
64+
ResponseTemplate: "template-response",
65+
LabelsBlacklist: []string{
66+
"label1",
67+
"label2",
68+
},
69+
},
6170
DefaultSecretStorage: &config.SecretStorage{
6271
Name: "vault",
6372
Type: string(models.VaultSecretStorageType),

‎api/api/projects_api.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,17 @@ func (c *ProjectsController) UpdateProject(r *http.Request, vars map[string]stri
7878
project.Team = newProject.Team
7979
project.Stream = newProject.Stream
8080
project.Labels = newProject.Labels
81-
project, err = c.ProjectsService.UpdateProject(r.Context(), project)
81+
updatedProject, response, err := c.ProjectsService.UpdateProject(r.Context(), project)
8282
if err != nil {
8383
log.Errorf("error updating project %s: %s", project.Name, err)
8484
return FromError(err)
8585
}
8686

87-
return Ok(project)
87+
if response != nil {
88+
return Ok(response)
89+
}
90+
91+
return Ok(updatedProject)
8892
}
8993

9094
func (c *ProjectsController) GetProject(r *http.Request, vars map[string]string, body interface{}) *Response {

‎api/api/projects_api_test.go

Lines changed: 156 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/jinzhu/gorm"
1616
"github.com/stretchr/testify/assert"
1717

18+
"github.com/caraml-dev/mlp/api/config"
1819
"github.com/caraml-dev/mlp/api/it/database"
1920
"github.com/caraml-dev/mlp/api/models"
2021
"github.com/caraml-dev/mlp/api/repository"
@@ -206,7 +207,10 @@ func TestCreateProject(t *testing.T) {
206207
_, err := prjRepository.Save(tC.existingProject)
207208
assert.NoError(t, err)
208209
}
209-
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
210+
projectService, err := service.NewProjectsService(
211+
mlflowTrackingURL, prjRepository, nil, false, nil,
212+
config.UpdateProjectConfig{},
213+
)
210214
assert.NoError(t, err)
211215

212216
appCtx := &AppContext{
@@ -316,7 +320,10 @@ func TestListProjects(t *testing.T) {
316320
assert.NoError(t, err)
317321
}
318322
}
319-
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
323+
projectService, err := service.NewProjectsService(
324+
mlflowTrackingURL, prjRepository, nil, false, nil,
325+
config.UpdateProjectConfig{},
326+
)
320327
assert.NoError(t, err)
321328

322329
appCtx := &AppContext{
@@ -369,14 +376,62 @@ func TestListProjects(t *testing.T) {
369376

370377
func TestUpdateProject(t *testing.T) {
371378
testCases := []struct {
372-
desc string
373-
projectID models.ID
374-
existingProject *models.Project
375-
expectedResponse *Response
376-
body interface{}
379+
desc string
380+
projectID models.ID
381+
existingProject *models.Project
382+
expectedResponse *Response
383+
body interface{}
384+
updateProjectConfig config.UpdateProjectConfig
377385
}{
378386
{
379-
desc: "Should success",
387+
desc: "Should success with update project config",
388+
projectID: models.ID(1),
389+
existingProject: &models.Project{
390+
ID: models.ID(1),
391+
Name: "Project1",
392+
MLFlowTrackingURL: "http://mlflow.com",
393+
Administrators: []string{adminUser},
394+
Team: "dsp",
395+
Stream: "dsp",
396+
CreatedUpdated: models.CreatedUpdated{
397+
CreatedAt: now,
398+
UpdatedAt: now,
399+
},
400+
},
401+
body: &models.Project{
402+
Name: "Project1",
403+
Team: "merlin",
404+
Stream: "dsp",
405+
Administrators: []string{adminUser},
406+
},
407+
expectedResponse: &Response{
408+
code: 200,
409+
data: map[string]interface{}{
410+
"status": "success",
411+
"message": "Project updated successfully",
412+
},
413+
},
414+
updateProjectConfig: config.UpdateProjectConfig{
415+
Endpoint: "url",
416+
PayloadTemplate: `{
417+
"project": "{{.Name}}",
418+
"administrators": "{{.Administrators}}",
419+
"readers": "{{.Readers}}",
420+
"team": "{{.Team}}",
421+
"stream": "{{.Stream}}"
422+
}`,
423+
ResponseTemplate: `{
424+
"status": "{{.status}}",
425+
"message": "{{.message}}"
426+
}`,
427+
LabelsBlacklist: []string{
428+
"label1",
429+
"label2",
430+
},
431+
},
432+
},
433+
{
434+
desc: "Should success without update project config",
380435
projectID: models.ID(1),
381436
existingProject: &models.Project{
382437
ID: models.ID(1),
@@ -411,6 +466,7 @@ func TestUpdateProject(t *testing.T) {
411466
},
412467
},
413468
},
469+
updateProjectConfig: config.UpdateProjectConfig{},
414470
},
415471
{
416472
desc: "Should failed when name is not specified",
@@ -438,6 +494,7 @@ func TestUpdateProject(t *testing.T) {
438494
Message: "Name is required",
439495
},
440496
},
497+
updateProjectConfig: config.UpdateProjectConfig{},
441498
},
442499
{
443500
desc: "Should failed when name project id is not found",
@@ -466,6 +523,52 @@ func TestUpdateProject(t *testing.T) {
466523
Message: "project with ID 2 not found",
467524
},
468525
},
526+
updateProjectConfig: config.UpdateProjectConfig{},
527+
},
528+
{
529+
desc: "Should fail when label in blacklist",
530+
projectID: models.ID(1),
531+
existingProject: &models.Project{
532+
ID: models.ID(1),
533+
Name: "Project1",
534+
MLFlowTrackingURL: "http://mlflow.com",
535+
Administrators: []string{adminUser},
536+
Team: "dsp",
537+
Stream: "dsp",
538+
Labels: models.Labels{
539+
{
540+
Key: "my-label",
541+
Value: "my-value",
542+
},
543+
},
544+
CreatedUpdated: models.CreatedUpdated{
545+
CreatedAt: now,
546+
UpdatedAt: now,
547+
},
548+
},
549+
body: &models.Project{
550+
Name: "Project1",
551+
Team: "merlin",
552+
Stream: "dsp",
553+
Labels: models.Labels{
554+
{
555+
Key: "my-label",
556+
Value: "my-new-value",
557+
},
558+
},
559+
Administrators: []string{adminUser},
560+
},
561+
expectedResponse: &Response{
562+
code: 500,
563+
data: ErrorMessage{
564+
Message: "one or more labels are blacklisted or have been removed or changed values and cannot be updated",
565+
},
566+
},
567+
updateProjectConfig: config.UpdateProjectConfig{
568+
LabelsBlacklist: []string{
569+
"my-label",
570+
},
571+
},
469572
},
470573
}
471574
for _, tC := range testCases {
@@ -476,7 +579,31 @@ func TestUpdateProject(t *testing.T) {
476579
_, err := prjRepository.Save(tC.existingProject)
477580
assert.NoError(t, err)
478581
}
479-
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
582+
583+
var server *httptest.Server
584+
if tC.updateProjectConfig.Endpoint != "" {
585+
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
586+
var payload map[string]interface{}
587+
err := json.NewDecoder(r.Body).Decode(&payload)
588+
assert.NoError(t, err)
589+
590+
w.WriteHeader(http.StatusOK)
591+
response := map[string]string{
592+
"status": "success",
593+
"message": "Project updated successfully",
594+
}
595+
err = json.NewEncoder(w).Encode(response)
596+
assert.NoError(t, err)
597+
}))
598+
defer server.Close()
599+
600+
tC.updateProjectConfig.Endpoint = server.URL
601+
}
602+
603+
projectService, err := service.NewProjectsService(
604+
mlflowTrackingURL, prjRepository, nil, false, nil,
605+
tC.updateProjectConfig,
606+
)
480607
assert.NoError(t, err)
481608

482609
appCtx := &AppContext{
@@ -506,14 +633,24 @@ func TestUpdateProject(t *testing.T) {
506633

507634
assert.Equal(t, tC.expectedResponse.code, rr.Code)
508635
if tC.expectedResponse.code >= 200 && tC.expectedResponse.code < 300 {
509-
project := &models.Project{}
510-
err = json.Unmarshal(rr.Body.Bytes(), &project)
511-
assert.NoError(t, err)
636+
switch tC.expectedResponse.data.(type) {
637+
case *models.Project:
638+
project := &models.Project{}
639+
err = json.Unmarshal(rr.Body.Bytes(), &project)
640+
assert.NoError(t, err)
512641

513-
project.CreatedAt = now
514-
project.UpdatedAt = now
642+
project.CreatedAt = now
643+
project.UpdatedAt = now
515644

516-
assert.Equal(t, tC.expectedResponse.data, project)
645+
assert.Equal(t, tC.expectedResponse.data, project)
646+
case map[string]interface{}:
647+
var responseMessage map[string]interface{}
648+
err = json.Unmarshal(rr.Body.Bytes(), &responseMessage)
649+
assert.NoError(t, err)
650+
assert.Equal(t, tC.expectedResponse.data, responseMessage)
651+
default:
652+
t.Fatal("unexpected type for expectedResponse.data")
653+
}
517654
} else {
518655
e := ErrorMessage{}
519656
err = json.Unmarshal(rr.Body.Bytes(), &e)
@@ -595,7 +732,10 @@ func TestGetProject(t *testing.T) {
595732
_, err := prjRepository.Save(tC.existingProject)
596733
assert.NoError(t, err)
597734
}
598-
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
735+
projectService, err := service.NewProjectsService(
736+
mlflowTrackingURL, prjRepository, nil, false, nil,
737+
config.UpdateProjectConfig{},
738+
)
599739
assert.NoError(t, err)
600740

601741
appCtx := &AppContext{

‎api/api/router.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ func NewAppContext(db *gorm.DB, cfg *config.Config) (ctx *AppContext, err error)
7474
cfg.Mlflow.TrackingURL,
7575
repository.NewProjectRepository(db),
7676
authEnforcer,
77-
cfg.Authorization.Enabled, projectsWebhookManager)
77+
cfg.Authorization.Enabled, projectsWebhookManager,
78+
*cfg.UpdateProjectConfig)
7879

7980
if err != nil {
8081
return nil, fmt.Errorf("failed to initialize projects service: %v", err)

‎api/cmd/serve.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ func startServer(cfg *config.Config) {
8383
Docs: cfg.Docs,
8484
MaxAuthzCacheExpiryMinutes: fmt.Sprintf("%.0f",
8585
math.Ceil((time.Duration(enforcer.MaxKeyExpirySeconds) * time.Second).Minutes())),
86-
UIConfig: cfg.UI,
86+
LabelsBlacklist: cfg.UpdateProjectConfig.LabelsBlacklist,
87+
UIConfig: cfg.UI,
8788
}
8889

8990
router.Methods("GET").Path("/env.js").HandlerFunc(uiEnv.handler)
@@ -114,6 +115,7 @@ type uiEnvHandler struct {
114115
Streams config.Streams `json:"REACT_APP_STREAMS"`
115116
Docs config.Documentations `json:"REACT_APP_DOC_LINKS"`
116117
MaxAuthzCacheExpiryMinutes string `json:"REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES"`
118+
LabelsBlacklist []string `json:"REACT_APP_LABELS_BLACKLIST"`
117119
}
118120

119121
func (h uiEnvHandler) handler(w http.ResponseWriter, r *http.Request) {

‎api/config/config.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ type Config struct {
3535
DefaultSecretStorage *SecretStorage `validate:"required"`
3636
UI *UIConfig
3737
Webhooks *webhooks.Config
38+
UpdateProjectConfig *UpdateProjectConfig
3839
}
3940

4041
// SecretStorage represents the configuration for a secret storage.
@@ -126,6 +127,18 @@ type UIConfig struct {
126127
ProjectInfoUpdateEnabled bool `json:"REACT_APP_PROJECT_INFO_UPDATE_ENABLED"`
127128
}
128129

130+
type UpdateProjectConfig struct {
131+
// endpoint to be called when the update projects config endpoint is called
132+
Endpoint string `validate:"omitempty,url"`
133+
// payload template to define the payload in a JSON template to be sent to the endpoint
134+
PayloadTemplate string
135+
// response template to define the response in the JSON payload given by the endpoint
136+
// through the template that should be sent back to the user
137+
ResponseTemplate string
138+
// labels blacklist that hides/prevents labels contained within to not be modifiable
139+
LabelsBlacklist []string
140+
}
141+
129142
// Transform env variables to the format consumed by koanf.
130143
// The variable key is split by the double underscore ('__') sequence,
131144
// which separates nested config variables, and then each config key is
@@ -224,6 +237,7 @@ var defaultConfig = &Config{
224237
AllowCustomTeam: true,
225238
AllowCustomStream: true,
226239
},
240+
UpdateProjectConfig: &UpdateProjectConfig{},
227241
DefaultSecretStorage: &SecretStorage{
228242
Name: "internal",
229243
Type: "internal",

0 commit comments

Comments
 (0)