Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make update project settings endpoint call URL with template payload + add a labels blacklist #99

Merged
merged 27 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ func (s *APITestSuite) SetupTest() {
Mlflow: &config.MlflowConfig{
TrackingURL: "http://mlflow:5000",
},
UpdateProjectConfig: &config.UpdateProjectConfig{
Endpoint: "",
PayloadTemplate: "template-payload",
ResponseTemplate: "template-response",
LabelsBlacklist: []string{
"label1",
"label2",
},
},
DefaultSecretStorage: &config.SecretStorage{
Name: "vault",
Type: string(models.VaultSecretStorageType),
Expand Down
8 changes: 6 additions & 2 deletions api/api/projects_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,17 @@ func (c *ProjectsController) UpdateProject(r *http.Request, vars map[string]stri
project.Team = newProject.Team
project.Stream = newProject.Stream
project.Labels = newProject.Labels
project, err = c.ProjectsService.UpdateProject(r.Context(), project)
updatedProject, response, err := c.ProjectsService.UpdateProject(r.Context(), project)
if err != nil {
log.Errorf("error updating project %s: %s", project.Name, err)
return FromError(err)
}

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

return Ok(updatedProject)
jonathanv28 marked this conversation as resolved.
Show resolved Hide resolved
}

func (c *ProjectsController) GetProject(r *http.Request, vars map[string]string, body interface{}) *Response {
Expand Down
172 changes: 156 additions & 16 deletions api/api/projects_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/jinzhu/gorm"
"github.com/stretchr/testify/assert"

"github.com/caraml-dev/mlp/api/config"
"github.com/caraml-dev/mlp/api/it/database"
"github.com/caraml-dev/mlp/api/models"
"github.com/caraml-dev/mlp/api/repository"
Expand Down Expand Up @@ -206,7 +207,10 @@ func TestCreateProject(t *testing.T) {
_, err := prjRepository.Save(tC.existingProject)
assert.NoError(t, err)
}
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
projectService, err := service.NewProjectsService(
mlflowTrackingURL, prjRepository, nil, false, nil,
config.UpdateProjectConfig{},
)
assert.NoError(t, err)

appCtx := &AppContext{
Expand Down Expand Up @@ -316,7 +320,10 @@ func TestListProjects(t *testing.T) {
assert.NoError(t, err)
}
}
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
projectService, err := service.NewProjectsService(
mlflowTrackingURL, prjRepository, nil, false, nil,
config.UpdateProjectConfig{},
)
assert.NoError(t, err)

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

func TestUpdateProject(t *testing.T) {
testCases := []struct {
desc string
projectID models.ID
existingProject *models.Project
expectedResponse *Response
body interface{}
desc string
projectID models.ID
existingProject *models.Project
expectedResponse *Response
body interface{}
updateProjectConfig config.UpdateProjectConfig
}{
{
desc: "Should success",
desc: "Should success with update project config",
jonathanv28 marked this conversation as resolved.
Show resolved Hide resolved
projectID: models.ID(1),
existingProject: &models.Project{
ID: models.ID(1),
Name: "Project1",
MLFlowTrackingURL: "http://mlflow.com",
Administrators: []string{adminUser},
Team: "dsp",
Stream: "dsp",
CreatedUpdated: models.CreatedUpdated{
CreatedAt: now,
UpdatedAt: now,
},
},
body: &models.Project{
Name: "Project1",
Team: "merlin",
Stream: "dsp",
Administrators: []string{adminUser},
},
expectedResponse: &Response{
code: 200,
data: map[string]interface{}{
"status": "success",
"message": "Project updated successfully",
},
},
updateProjectConfig: config.UpdateProjectConfig{
Endpoint: "url",
PayloadTemplate: `{
"project": "{{.Name}}",
"administrators": "{{.Administrators}}",
"readers": "{{.Readers}}",
"team": "{{.Team}}",
"stream": "{{.Stream}}"
}`,
ResponseTemplate: `{
"status": "{{.status}}",
"message": "{{.message}}"
}`,
LabelsBlacklist: []string{
"label1",
"label2",
},
},
},
{
desc: "Should success without update project config",
projectID: models.ID(1),
existingProject: &models.Project{
ID: models.ID(1),
Expand Down Expand Up @@ -411,6 +466,7 @@ func TestUpdateProject(t *testing.T) {
},
},
},
updateProjectConfig: config.UpdateProjectConfig{},
},
{
desc: "Should failed when name is not specified",
Expand Down Expand Up @@ -438,6 +494,7 @@ func TestUpdateProject(t *testing.T) {
Message: "Name is required",
},
},
updateProjectConfig: config.UpdateProjectConfig{},
},
{
desc: "Should failed when name project id is not found",
Expand Down Expand Up @@ -466,6 +523,52 @@ func TestUpdateProject(t *testing.T) {
Message: "project with ID 2 not found",
},
},
updateProjectConfig: config.UpdateProjectConfig{},
},
{
desc: "Should fail when label in blacklist",
projectID: models.ID(1),
existingProject: &models.Project{
ID: models.ID(1),
Name: "Project1",
MLFlowTrackingURL: "http://mlflow.com",
Administrators: []string{adminUser},
Team: "dsp",
Stream: "dsp",
Labels: models.Labels{
{
Key: "my-label",
Value: "my-value",
},
},
CreatedUpdated: models.CreatedUpdated{
CreatedAt: now,
UpdatedAt: now,
},
},
body: &models.Project{
Name: "Project1",
Team: "merlin",
Stream: "dsp",
Labels: models.Labels{
{
Key: "my-label",
Value: "my-new-value",
},
},
Administrators: []string{adminUser},
},
expectedResponse: &Response{
code: 500,
data: ErrorMessage{
Message: "one or more labels are blacklisted or have been removed or changed values and cannot be updated",
},
},
updateProjectConfig: config.UpdateProjectConfig{
LabelsBlacklist: []string{
"my-label",
},
},
},
}
for _, tC := range testCases {
Expand All @@ -476,7 +579,31 @@ func TestUpdateProject(t *testing.T) {
_, err := prjRepository.Save(tC.existingProject)
assert.NoError(t, err)
}
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)

var server *httptest.Server
if tC.updateProjectConfig.Endpoint != "" {
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var payload map[string]interface{}
err := json.NewDecoder(r.Body).Decode(&payload)
assert.NoError(t, err)

w.WriteHeader(http.StatusOK)
response := map[string]string{
"status": "success",
"message": "Project updated successfully",
}
err = json.NewEncoder(w).Encode(response)
assert.NoError(t, err)
}))
defer server.Close()

tC.updateProjectConfig.Endpoint = server.URL
}

projectService, err := service.NewProjectsService(
mlflowTrackingURL, prjRepository, nil, false, nil,
tC.updateProjectConfig,
)
assert.NoError(t, err)

appCtx := &AppContext{
Expand Down Expand Up @@ -506,14 +633,24 @@ 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)
switch tC.expectedResponse.data.(type) {
case *models.Project:
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)
case map[string]interface{}:
var responseMessage map[string]interface{}
err = json.Unmarshal(rr.Body.Bytes(), &responseMessage)
assert.NoError(t, err)
assert.Equal(t, tC.expectedResponse.data, responseMessage)
default:
t.Fatal("unexpected type for expectedResponse.data")
}
jonathanv28 marked this conversation as resolved.
Show resolved Hide resolved
} else {
e := ErrorMessage{}
err = json.Unmarshal(rr.Body.Bytes(), &e)
Expand Down Expand Up @@ -595,7 +732,10 @@ func TestGetProject(t *testing.T) {
_, err := prjRepository.Save(tC.existingProject)
assert.NoError(t, err)
}
projectService, err := service.NewProjectsService(mlflowTrackingURL, prjRepository, nil, false, nil)
projectService, err := service.NewProjectsService(
mlflowTrackingURL, prjRepository, nil, false, nil,
config.UpdateProjectConfig{},
)
assert.NoError(t, err)

appCtx := &AppContext{
Expand Down
3 changes: 2 additions & 1 deletion api/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ func NewAppContext(db *gorm.DB, cfg *config.Config) (ctx *AppContext, err error)
cfg.Mlflow.TrackingURL,
repository.NewProjectRepository(db),
authEnforcer,
cfg.Authorization.Enabled, projectsWebhookManager)
cfg.Authorization.Enabled, projectsWebhookManager,
*cfg.UpdateProjectConfig)

if err != nil {
return nil, fmt.Errorf("failed to initialize projects service: %v", err)
Expand Down
4 changes: 3 additions & 1 deletion api/cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ func startServer(cfg *config.Config) {
Docs: cfg.Docs,
MaxAuthzCacheExpiryMinutes: fmt.Sprintf("%.0f",
math.Ceil((time.Duration(enforcer.MaxKeyExpirySeconds) * time.Second).Minutes())),
UIConfig: cfg.UI,
LabelsBlacklist: cfg.UpdateProjectConfig.LabelsBlacklist,
UIConfig: cfg.UI,
}

router.Methods("GET").Path("/env.js").HandlerFunc(uiEnv.handler)
Expand Down Expand Up @@ -114,6 +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"`
}

func (h uiEnvHandler) handler(w http.ResponseWriter, r *http.Request) {
Expand Down
14 changes: 14 additions & 0 deletions api/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Config struct {
DefaultSecretStorage *SecretStorage `validate:"required"`
UI *UIConfig
Webhooks *webhooks.Config
UpdateProjectConfig *UpdateProjectConfig
}

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

type UpdateProjectConfig struct {
// endpoint to be called when the update projects config endpoint is called
Endpoint string `validate:"omitempty,url"`
// payload template to define the payload in a JSON template to be sent to the endpoint
PayloadTemplate string
// response template to define the response in the JSON payload given by the endpoint
// through the template that should be sent back to the user
ResponseTemplate string
// labels blacklist that hides/prevents labels contained within to not be modifiable
jonathanv28 marked this conversation as resolved.
Show resolved Hide resolved
LabelsBlacklist []string
}

// Transform env variables to the format consumed by koanf.
// The variable key is split by the double underscore ('__') sequence,
// which separates nested config variables, and then each config key is
Expand Down Expand Up @@ -224,6 +237,7 @@ var defaultConfig = &Config{
AllowCustomTeam: true,
AllowCustomStream: true,
},
UpdateProjectConfig: &UpdateProjectConfig{},
DefaultSecretStorage: &SecretStorage{
Name: "internal",
Type: "internal",
Expand Down
Loading
Loading