Skip to content

Commit

Permalink
handle labels blacklist, handle webhook scenarios, handle endpoint ni…
Browse files Browse the repository at this point in the history
…l, update unit tests
  • Loading branch information
Jonathan Victor Goklas authored and Jonathan Victor Goklas committed Jun 13, 2024
1 parent 5652b96 commit 02f36ce
Show file tree
Hide file tree
Showing 12 changed files with 453 additions and 248 deletions.
10 changes: 5 additions & 5 deletions api/api/projects_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ func (c *ProjectsController) UpdateProject(r *http.Request, vars map[string]stri
log.Errorf("error updating project %s: %s", project.Name, err)
return FromError(err)
}
if response == nil {
log.Errorf("error updating project, update project config has not been completely specified")
return FromError(err)
}

return Ok(response)
if response != nil {
return Ok(response)
} else {

Check warning on line 93 in api/api/projects_api.go

View workflow job for this annotation

GitHub Actions / test-api

indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
return Ok(project)
}
}

func (c *ProjectsController) GetProject(r *http.Request, vars map[string]string, body interface{}) *Response {
Expand Down
52 changes: 31 additions & 21 deletions api/api/projects_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,12 @@ func TestUpdateProject(t *testing.T) {
projectID models.ID
existingProject *models.Project
expectedResponse *Response
expectedMessage map[string]interface{}
body interface{}
updateProjectEndpoint string
updateProjectPayload string
updateProjectResponse string
labelsBlacklist map[string]bool
}{
{
desc: "Should success with update project config",
Expand All @@ -406,20 +408,10 @@ func TestUpdateProject(t *testing.T) {
Stream: "dsp",
Administrators: []string{adminUser},
},
expectedResponse: &Response{
code: 200,
data: &models.Project{
ID: models.ID(1),
Name: "Project1",
MLFlowTrackingURL: "http://mlflow.com",
Administrators: []string{adminUser},
Team: "merlin",
Stream: "dsp",
CreatedUpdated: models.CreatedUpdated{
CreatedAt: now,
UpdatedAt: now,
},
},
expectedResponse: nil,
expectedMessage: map[string]interface{}{
"status": "success",
"message": "Project updated successfully",
},
updateProjectEndpoint: "url",
updateProjectPayload: `{
Expand All @@ -433,6 +425,10 @@ func TestUpdateProject(t *testing.T) {
"status": "{{.status}}",
"message": "{{.message}}"
}`,
labelsBlacklist: map[string]bool{
"label1": true,
"label2": true,
},
},
{
desc: "Should success without update project config",
Expand Down Expand Up @@ -470,9 +466,11 @@ func TestUpdateProject(t *testing.T) {
},
},
},
expectedMessage: nil,
updateProjectEndpoint: "",
updateProjectPayload: "",
updateProjectResponse: "",
labelsBlacklist: map[string]bool{},
},
{
desc: "Should failed when name is not specified",
Expand Down Expand Up @@ -500,9 +498,11 @@ func TestUpdateProject(t *testing.T) {
Message: "Name is required",
},
},
expectedMessage: nil,
updateProjectEndpoint: "",
updateProjectPayload: "",
updateProjectResponse: "",
labelsBlacklist: map[string]bool{},
},
{
desc: "Should failed when name project id is not found",
Expand Down Expand Up @@ -531,9 +531,11 @@ func TestUpdateProject(t *testing.T) {
Message: "project with ID 2 not found",
},
},
expectedMessage: nil,
updateProjectEndpoint: "",
updateProjectPayload: "",
updateProjectResponse: "",
labelsBlacklist: map[string]bool{},
},
}
for _, tC := range testCases {
Expand Down Expand Up @@ -566,11 +568,12 @@ func TestUpdateProject(t *testing.T) {
}

projectService, err := service.NewProjectsService(
mlflowTrackingURL, prjRepository, nil, false, nil
mlflowTrackingURL, prjRepository, nil, false, nil,
config.UpdateProjectConfig{
Endpoint: tC.updateProjectEndpoint,
PayloadTemplate: tC.updateProjectPayload,
ResponseTemplate: tC.updateProjectResponse,
LabelsBlacklist: tC.labelsBlacklist,
},
)
assert.NoError(t, err)
Expand Down Expand Up @@ -602,14 +605,21 @@ func TestUpdateProject(t *testing.T) {

assert.Equal(t, tC.expectedResponse.code, rr.Code)
if tC.expectedResponse.code >= 200 && tC.expectedResponse.code < 300 {
project := &models.Project{}
err = json.Unmarshal(rr.Body.Bytes(), &project)
assert.NoError(t, err)
if tC.expectedResponse != nil {
project := &models.Project{}
err = json.Unmarshal(rr.Body.Bytes(), &project)
assert.NoError(t, err)

project.CreatedAt = now
project.UpdatedAt = now
project.CreatedAt = now
project.UpdatedAt = now

assert.Equal(t, tC.expectedResponse.data, project)
assert.Equal(t, tC.expectedResponse.data, project)
} else {
var responseMessage map[string]interface{}
err = json.Unmarshal(rr.Body.Bytes(), &responseMessage)
assert.NoError(t, err)
assert.Equal(t, tC.expectedMessage, responseMessage)
}
} else {
e := ErrorMessage{}
err = json.Unmarshal(rr.Body.Bytes(), &e)
Expand Down
7 changes: 1 addition & 6 deletions api/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,12 @@ func NewAppContext(db *gorm.DB, cfg *config.Config) (ctx *AppContext, err error)
}
}

if cfg.UpdateProject == nil {
return nil, fmt.Errorf("update project config is nil")
}
updateProjectConfig := *cfg.UpdateProject

projectsService, err := service.NewProjectsService(
cfg.Mlflow.TrackingURL,
repository.NewProjectRepository(db),
authEnforcer,
cfg.Authorization.Enabled, projectsWebhookManager,
updateProjectConfig)
*cfg.UpdateProject)

if err != nil {
return nil, fmt.Errorf("failed to initialize projects service: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion api/cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ type uiEnvHandler struct {
Streams config.Streams `json:"REACT_APP_STREAMS"`
Docs config.Documentations `json:"REACT_APP_DOC_LINKS"`
MaxAuthzCacheExpiryMinutes string `json:"REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES"`
LabelsBlacklist []string `json:"REACT_APP_LABELS_BLACKLIST"`
LabelsBlacklist map[string]bool `json:"REACT_APP_LABELS_BLACKLIST"`
}

func (h uiEnvHandler) handler(w http.ResponseWriter, r *http.Request) {
Expand Down
11 changes: 3 additions & 8 deletions api/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ type UIConfig struct {
}

type UpdateProjectConfig struct {
Endpoint string `validate:"url"`
Endpoint string `validate:"omitempty,url"`
PayloadTemplate string
ResponseTemplate string
LabelsBlacklist []string `json:"REACT_APP_LABELS_BLACKLIST"`
LabelsBlacklist map[string]bool
// labels blacklist that hides/prevents labels contained within to not be modifiable
}

Expand Down Expand Up @@ -233,12 +233,7 @@ var defaultConfig = &Config{
AllowCustomTeam: true,
AllowCustomStream: true,
},
UpdateProject: &UpdateProjectConfig{
Endpoint: "http://localhost:8080",
PayloadTemplate: ``,
ResponseTemplate: ``,
LabelsBlacklist: []string{"env"},
},
UpdateProject: &UpdateProjectConfig{},
DefaultSecretStorage: &SecretStorage{
Name: "internal",
Type: "internal",
Expand Down
37 changes: 21 additions & 16 deletions api/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,14 @@ func TestLoad(t *testing.T) {
ProjectInfoUpdateEnabled: true,
},
UpdateProject: &config.UpdateProjectConfig{
Endpoint: "http://example-update-project.dev",
PayloadTemplate: "your-payload-template",
ResponseTemplate: "your-response-template",
LabelsBlacklist: []string{
"label1",
"label2",
Endpoint: "http://localhost:3030",
PayloadTemplate: `{"template": "{{.template}}"}
`,
ResponseTemplate: `{"response": "{{.response}}"}
`,
LabelsBlacklist: map[string]bool{
"label1": true,
"label2": true,
},
},
DefaultSecretStorage: &config.SecretStorage{
Expand Down Expand Up @@ -345,9 +347,9 @@ func TestValidate(t *testing.T) {
Endpoint: "http://example-update-project.dev",
PayloadTemplate: "your-payload-template",
ResponseTemplate: "your-response-template",
LabelsBlacklist: []string{
"label1",
"label2",
LabelsBlacklist: map[string]bool{
"label1": true,
"label2": true,
},
},
},
Expand Down Expand Up @@ -401,9 +403,9 @@ func TestValidate(t *testing.T) {
Endpoint: "http://example-update-project.dev",
PayloadTemplate: "your-payload-template",
ResponseTemplate: "your-response-template",
LabelsBlacklist: []string{
"label1",
"label2",
LabelsBlacklist: map[string]bool{
"label1": true,
"label2": true,
},
},
},
Expand Down Expand Up @@ -461,9 +463,9 @@ func TestValidate(t *testing.T) {
Endpoint: "http://example-update-project.dev",
PayloadTemplate: "your-payload-template",
ResponseTemplate: "your-response-template",
LabelsBlacklist: []string{
"label1",
"label2",
LabelsBlacklist: map[string]bool{
"label1": true,
"label2": true,
},
},
},
Expand Down Expand Up @@ -518,7 +520,10 @@ func TestValidate(t *testing.T) {
Endpoint: "http://example-update-project.dev",
PayloadTemplate: "your-payload-template",
ResponseTemplate: "your-response-template",
LabelsBlacklist: []string{},
LabelsBlacklist: map[string]bool{
"label1": true,
"label2": true,
},
},
},
error: errors.New(
Expand Down
12 changes: 7 additions & 5 deletions api/config/testdata/config-2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ ui:
projectinfoUpdateEnabled: true

updateProject:
endpoint: "http://example-update-project.dev"
payloadTemplate: "your-payload-template"
responseTemplate: "your-response-template"
endpoint: http://localhost:3030
payloadTemplate: |
{"template": "{{.template}}"}
responseTemplate: |
{"response": "{{.response}}"}
labelsBlacklist:
- "label1"
- "label2"
label1: true
label2: true

defaultSecretStorage:
name: default-secret-storage
Expand Down
Loading

0 comments on commit 02f36ce

Please sign in to comment.