From 463bfb6249bf77d0233607357fef833ca1ef2f49 Mon Sep 17 00:00:00 2001 From: Honnix Date: Thu, 5 Nov 2020 23:23:47 +0100 Subject: [PATCH] Archive and unarchive a project (#138) * Use state for a project to archive or activate it --- go.mod | 2 +- go.sum | 2 ++ pkg/manager/impl/project_manager_test.go | 2 ++ pkg/manager/impl/testutils/repository.go | 7 +++-- .../impl/validation/project_validator.go | 6 +++- .../impl/validation/project_validator_test.go | 22 ++++++++++++-- pkg/repositories/config/migrations.go | 18 +++++++++++ pkg/repositories/gormimpl/project_repo.go | 3 +- .../gormimpl/project_repo_test.go | 30 +++++++++++++++++-- pkg/repositories/mocks/project_repo.go | 7 ++++- pkg/repositories/models/project.go | 2 ++ pkg/repositories/transformers/project.go | 3 ++ pkg/repositories/transformers/project_test.go | 10 +++++++ 13 files changed, 104 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index 957e352b4e..324a079e97 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/jinzhu/gorm v1.9.12 github.com/kelseyhightower/envconfig v1.4.0 // indirect github.com/lib/pq v1.3.0 - github.com/lyft/flyteidl v0.18.6 + github.com/lyft/flyteidl v0.18.10 github.com/lyft/flytepropeller v0.3.17 github.com/lyft/flytestdlib v0.3.9 github.com/magiconair/properties v1.8.1 diff --git a/go.sum b/go.sum index 068f8cf07e..3461b942f2 100644 --- a/go.sum +++ b/go.sum @@ -464,6 +464,8 @@ github.com/lyft/flyteidl v0.17.0/go.mod h1:/zQXxuHO11u/saxTTZc8oYExIGEShXB+xCB1/ github.com/lyft/flyteidl v0.18.0/go.mod h1:/zQXxuHO11u/saxTTZc8oYExIGEShXB+xCB1/F1Cu20= github.com/lyft/flyteidl v0.18.6 h1:HGbxHI8avEDvoPqcO2+/BoJVcP9sjOj4qwJ/wNRWuoA= github.com/lyft/flyteidl v0.18.6/go.mod h1:/zQXxuHO11u/saxTTZc8oYExIGEShXB+xCB1/F1Cu20= +github.com/lyft/flyteidl v0.18.10 h1:kM7HKNsv0S9H0nJMPGdTbD53csjc7ueO9eXX+UtZ3fk= +github.com/lyft/flyteidl v0.18.10/go.mod h1:/zQXxuHO11u/saxTTZc8oYExIGEShXB+xCB1/F1Cu20= github.com/lyft/flyteplugins v0.5.1/go.mod h1:8zhqFG9BzbHNQGEXzGYltTJLD+KTmQZkanxXgeFI25c= github.com/lyft/flytepropeller v0.3.17 h1:a2PVqWjnn8oNEeayAqNizMAtEixl/F3S4vd8z4kbiqI= github.com/lyft/flytepropeller v0.3.17/go.mod h1:T8Utxqv7B5USAX9c/Qh0lBbKXHFSgOwwaISOd9h36P4= diff --git a/pkg/manager/impl/project_manager_test.go b/pkg/manager/impl/project_manager_test.go index c09d342ca0..95bb8b7bb8 100644 --- a/pkg/manager/impl/project_manager_test.go +++ b/pkg/manager/impl/project_manager_test.go @@ -47,11 +47,13 @@ func TestListProjects(t *testing.T) { repository := repositoryMocks.NewMockRepository() repository.ProjectRepo().(*repositoryMocks.MockProjectRepo).ListProjectsFunction = func( ctx context.Context, parameter common.SortParameter) ([]models.Project, error) { + activeState := int32(admin.Project_ACTIVE) return []models.Project{ { Identifier: "project", Name: "project", Description: "project_description", + State: &activeState, }, }, nil } diff --git a/pkg/manager/impl/testutils/repository.go b/pkg/manager/impl/testutils/repository.go index 64f46c03d0..e17d5ba2d3 100644 --- a/pkg/manager/impl/testutils/repository.go +++ b/pkg/manager/impl/testutils/repository.go @@ -4,6 +4,7 @@ import ( "context" "github.com/lyft/flyteadmin/pkg/repositories/models" + "github.com/lyft/flyteidl/gen/pb-go/flyteidl/admin" "github.com/lyft/flyteadmin/pkg/repositories" repositoryMocks "github.com/lyft/flyteadmin/pkg/repositories/mocks" @@ -13,7 +14,8 @@ func GetRepoWithDefaultProjectAndErr(err error) repositories.RepositoryInterface repo := repositoryMocks.NewMockRepository() repo.ProjectRepo().(*repositoryMocks.MockProjectRepo).GetFunction = func( ctx context.Context, projectID string) (models.Project, error) { - return models.Project{}, err + activeState := int32(admin.Project_ACTIVE) + return models.Project{State: &activeState}, err } return repo } @@ -22,7 +24,8 @@ func GetRepoWithDefaultProject() repositories.RepositoryInterface { repo := repositoryMocks.NewMockRepository() repo.ProjectRepo().(*repositoryMocks.MockProjectRepo).GetFunction = func( ctx context.Context, projectID string) (models.Project, error) { - return models.Project{}, nil + activeState := int32(admin.Project_ACTIVE) + return models.Project{State: &activeState}, nil } return repo } diff --git a/pkg/manager/impl/validation/project_validator.go b/pkg/manager/impl/validation/project_validator.go index 746160749b..00d7c3c6b6 100644 --- a/pkg/manager/impl/validation/project_validator.go +++ b/pkg/manager/impl/validation/project_validator.go @@ -53,12 +53,16 @@ func ValidateProjectLabels(request admin.Project) error { // Validates that a specified project and domain combination has been registered and exists in the db. func ValidateProjectAndDomain( ctx context.Context, db repositories.RepositoryInterface, config runtimeInterfaces.ApplicationConfiguration, projectID, domainID string) error { - _, err := db.ProjectRepo().Get(ctx, projectID) + project, err := db.ProjectRepo().Get(ctx, projectID) if err != nil { return errors.NewFlyteAdminErrorf(codes.InvalidArgument, "failed to validate that project [%s] and domain [%s] are registered, err: [%+v]", projectID, domainID, err) } + if *project.State != int32(admin.Project_ACTIVE) { + return errors.NewFlyteAdminErrorf(codes.InvalidArgument, + "project [%s] is not active", projectID) + } var validDomain bool domains := config.GetDomainsConfig() for _, domain := range *domains { diff --git a/pkg/manager/impl/validation/project_validator_test.go b/pkg/manager/impl/validation/project_validator_test.go index 0a0adb5ca9..8b98ed3c4c 100644 --- a/pkg/manager/impl/validation/project_validator_test.go +++ b/pkg/manager/impl/validation/project_validator_test.go @@ -107,18 +107,36 @@ func TestValidateProjectAndDomain(t *testing.T) { mockRepo.ProjectRepo().(*repositoryMocks.MockProjectRepo).GetFunction = func( ctx context.Context, projectID string) (models.Project, error) { assert.Equal(t, projectID, "flyte-project-id") - return models.Project{}, nil + activeState := int32(admin.Project_ACTIVE) + return models.Project{State: &activeState}, nil } err := ValidateProjectAndDomain(context.Background(), mockRepo, testutils.GetApplicationConfigWithDefaultDomains(), "flyte-project-id", "domain") assert.Nil(t, err) +} +func TestValidateProjectAndDomainArchivedProject(t *testing.T) { + mockRepo := repositoryMocks.NewMockRepository() + mockRepo.ProjectRepo().(*repositoryMocks.MockProjectRepo).GetFunction = func( + ctx context.Context, projectID string) (models.Project, error) { + archivedState := int32(admin.Project_ARCHIVED) + return models.Project{State: &archivedState}, nil + } + + err := ValidateProjectAndDomain(context.Background(), mockRepo, testutils.GetApplicationConfigWithDefaultDomains(), + "flyte-project-id", "domain") + assert.EqualError(t, err, + "project [flyte-project-id] is not active") +} + +func TestValidateProjectAndDomainError(t *testing.T) { + mockRepo := repositoryMocks.NewMockRepository() mockRepo.ProjectRepo().(*repositoryMocks.MockProjectRepo).GetFunction = func( ctx context.Context, projectID string) (models.Project, error) { return models.Project{}, errors.New("foo") } - err = ValidateProjectAndDomain(context.Background(), mockRepo, testutils.GetApplicationConfigWithDefaultDomains(), + err := ValidateProjectAndDomain(context.Background(), mockRepo, testutils.GetApplicationConfigWithDefaultDomains(), "flyte-project-id", "domain") assert.EqualError(t, err, "failed to validate that project [flyte-project-id] and domain [domain] are registered, err: [foo]") diff --git a/pkg/repositories/config/migrations.go b/pkg/repositories/config/migrations.go index 163e9d652a..433512a909 100644 --- a/pkg/repositories/config/migrations.go +++ b/pkg/repositories/config/migrations.go @@ -269,4 +269,22 @@ var Migrations = []*gormigrate.Migration{ return tx.Model(&TaskExecution{}).RemoveIndex("idx_task_executions_exec").Error }, }, + { + ID: "2020-11-03-project-state-addition", + Migrate: func(tx *gorm.DB) error { + return tx.AutoMigrate(&models.Project{}).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Model(&models.Project{}).DropColumn("state").Error + }, + }, + { + ID: "2020-11-03-project-state-default", + Migrate: func(tx *gorm.DB) error { + return tx.Exec("UPDATE projects set state = 0").Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec("UPDATE projects set state = NULL").Error + }, + }, } diff --git a/pkg/repositories/gormimpl/project_repo.go b/pkg/repositories/gormimpl/project_repo.go index fb14b49308..924ebd3528 100644 --- a/pkg/repositories/gormimpl/project_repo.go +++ b/pkg/repositories/gormimpl/project_repo.go @@ -5,6 +5,7 @@ import ( "google.golang.org/grpc/codes" + "github.com/lyft/flyteidl/gen/pb-go/flyteidl/admin" "github.com/lyft/flytestdlib/promutils" "github.com/jinzhu/gorm" @@ -50,7 +51,7 @@ func (r *ProjectRepo) Get(ctx context.Context, projectID string) (models.Project func (r *ProjectRepo) ListAll(ctx context.Context, sortParameter common.SortParameter) ([]models.Project, error) { var projects []models.Project - var tx = r.db + var tx = r.db.Where("state != ?", int32(admin.Project_ARCHIVED)) if sortParameter != nil { tx = tx.Order(sortParameter.GetGormOrderExpr()) } diff --git a/pkg/repositories/gormimpl/project_repo_test.go b/pkg/repositories/gormimpl/project_repo_test.go index 46569da725..a17fdd4265 100644 --- a/pkg/repositories/gormimpl/project_repo_test.go +++ b/pkg/repositories/gormimpl/project_repo_test.go @@ -19,12 +19,14 @@ func TestCreateProject(t *testing.T) { query := GlobalMock.NewMock() query.WithQuery( - `INSERT INTO "projects" ("created_at","updated_at","deleted_at","identifier","name","description","labels") VALUES (?,?,?,?,?,?,?)`) + `INSERT INTO "projects" ("created_at","updated_at","deleted_at","identifier","name","description","labels","state") VALUES (?,?,?,?,?,?,?,?)`) + activeState := int32(admin.Project_ACTIVE) err := projectRepo.Create(context.Background(), models.Project{ Identifier: "proj", Name: "proj", Description: "projDescription", + State: &activeState, }) assert.NoError(t, err) assert.True(t, query.Triggered) @@ -38,6 +40,7 @@ func TestGetProject(t *testing.T) { response["identifier"] = "project_id" response["name"] = "project_name" response["description"] = "project_description" + response["state"] = admin.Project_ACTIVE query := GlobalMock.NewMock() query.WithQuery(`SELECT * FROM "projects" WHERE "projects"."deleted_at" IS NULL AND ` + @@ -51,6 +54,7 @@ func TestGetProject(t *testing.T) { assert.Equal(t, "project_id", output.Identifier) assert.Equal(t, "project_name", output.Name) assert.Equal(t, "project_description", output.Description) + assert.Equal(t, int32(admin.Project_ACTIVE), *output.State) } func TestListProjects(t *testing.T) { @@ -60,17 +64,19 @@ func TestListProjects(t *testing.T) { fooProject["identifier"] = "foo" fooProject["name"] = "foo =)" fooProject["description"] = "foo description" + fooProject["state"] = admin.Project_ACTIVE projects[0] = fooProject barProject := make(map[string]interface{}) barProject["identifier"] = "bar" barProject["name"] = "Bar" barProject["description"] = "Bar description" + barProject["state"] = admin.Project_ACTIVE projects[1] = barProject GlobalMock := mocket.Catcher.Reset() GlobalMock.Logging = true - GlobalMock.NewMock().WithQuery(`SELECT * FROM "projects" WHERE "projects"."deleted_at" IS NULL ORDER BY identifier asc`). + GlobalMock.NewMock().WithQuery(`SELECT * FROM "projects" WHERE "projects"."deleted_at" IS NULL AND ((state != 1)) ORDER BY identifier asc`). WithReply(projects) var alphabeticalSortParam, _ = common.NewSortParameter(admin.Sort{ @@ -83,7 +89,27 @@ func TestListProjects(t *testing.T) { assert.Equal(t, "foo", output[0].Identifier) assert.Equal(t, "foo =)", output[0].Name) assert.Equal(t, "foo description", output[0].Description) + assert.Equal(t, int32(admin.Project_ACTIVE), *output[0].State) assert.Equal(t, "bar", output[1].Identifier) assert.Equal(t, "Bar", output[1].Name) assert.Equal(t, "Bar description", output[1].Description) + assert.Equal(t, int32(admin.Project_ACTIVE), *output[1].State) +} + +func TestUpdateProject(t *testing.T) { + projectRepo := NewProjectRepo(GetDbForTest(t), errors.NewTestErrorTransformer(), mockScope.NewTestScope()) + GlobalMock := mocket.Catcher.Reset() + + query := GlobalMock.NewMock() + query.WithQuery(`UPDATE "projects" SET "description" = ?, "identifier" = ?, "name" = ?, "state" = ?, "updated_at" = ? WHERE "projects"."deleted_at" IS NULL AND "projects"."identifier" = ?`) + + activeState := int32(admin.Project_ACTIVE) + err := projectRepo.UpdateProject(context.Background(), models.Project{ + Identifier: "project_id", + Name: "project_name", + Description: "project_description", + State: &activeState, + }) + assert.Nil(t, err) + assert.True(t, query.Triggered) } diff --git a/pkg/repositories/mocks/project_repo.go b/pkg/repositories/mocks/project_repo.go index 1bde48bd35..1bc892d20a 100644 --- a/pkg/repositories/mocks/project_repo.go +++ b/pkg/repositories/mocks/project_repo.go @@ -6,6 +6,7 @@ import ( "github.com/lyft/flyteadmin/pkg/common" "github.com/lyft/flyteadmin/pkg/repositories/interfaces" "github.com/lyft/flyteadmin/pkg/repositories/models" + "github.com/lyft/flyteidl/gen/pb-go/flyteidl/admin" ) type CreateProjectFunction func(ctx context.Context, project models.Project) error @@ -31,7 +32,11 @@ func (r *MockProjectRepo) Get(ctx context.Context, projectID string) (models.Pro if r.GetFunction != nil { return r.GetFunction(ctx, projectID) } - return models.Project{}, nil + activeState := int32(admin.Project_ACTIVE) + return models.Project{ + Identifier: projectID, + State: &activeState, + }, nil } func (r *MockProjectRepo) ListAll(ctx context.Context, sortParameter common.SortParameter) ([]models.Project, error) { diff --git a/pkg/repositories/models/project.go b/pkg/repositories/models/project.go index d43646a771..adcae65a7a 100644 --- a/pkg/repositories/models/project.go +++ b/pkg/repositories/models/project.go @@ -6,4 +6,6 @@ type Project struct { Name string // Human-readable name, not a unique identifier. Description string `gorm:"type:varchar(300)"` Labels []byte + // GORM doesn't save the zero value for ints, so we use a pointer for the State field + State *int32 `gorm:"default:0;index"` } diff --git a/pkg/repositories/transformers/project.go b/pkg/repositories/transformers/project.go index 2dd9ae88aa..b34cf1d565 100644 --- a/pkg/repositories/transformers/project.go +++ b/pkg/repositories/transformers/project.go @@ -13,6 +13,7 @@ type CreateProjectModelInput struct { } func CreateProjectModel(project *admin.Project) models.Project { + stateInt := int32(project.State) projectBytes, err := proto.Marshal(project) if err != nil { return models.Project{} @@ -22,6 +23,7 @@ func CreateProjectModel(project *admin.Project) models.Project { Name: project.Name, Description: project.Description, Labels: projectBytes, + State: &stateInt, } } @@ -36,6 +38,7 @@ func FromProjectModel(projectModel models.Project, domains []*admin.Domain) admi Name: projectModel.Name, Description: projectModel.Description, Labels: projectDeserialized.Labels, + State: admin.Project_ProjectState(*projectModel.State), } project.Domains = domains return project diff --git a/pkg/repositories/transformers/project_test.go b/pkg/repositories/transformers/project_test.go index aae46fa4b4..d7dc77ff7c 100644 --- a/pkg/repositories/transformers/project_test.go +++ b/pkg/repositories/transformers/project_test.go @@ -16,8 +16,10 @@ func TestCreateProjectModel(t *testing.T) { Id: "project_id", Name: "project_name", Description: "project_description", + State: admin.Project_ACTIVE, }) + activeState := int32(admin.Project_ACTIVE) assert.Equal(t, models.Project{ Identifier: "project_id", Name: "project_name", @@ -27,14 +29,17 @@ func TestCreateProjectModel(t *testing.T) { 0x6a, 0x65, 0x63, 0x74, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x22, 0x13, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x5f, 0x64, 0x65, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, }, + State: &activeState, }, projectModel) } func TestFromProjectModel(t *testing.T) { + activeState := int32(admin.Project_ACTIVE) projectModel := models.Project{ Identifier: "proj_id", Name: "proj_name", Description: "proj_description", + State: &activeState, } domains := []*admin.Domain{ { @@ -52,20 +57,24 @@ func TestFromProjectModel(t *testing.T) { Name: "proj_name", Description: "proj_description", Domains: domains, + State: admin.Project_ACTIVE, }, &project)) } func TestFromProjectModels(t *testing.T) { + activeState := int32(admin.Project_ACTIVE) projectModels := []models.Project{ { Identifier: "proj1_id", Name: "proj1_name", Description: "proj1_description", + State: &activeState, }, { Identifier: "proj2_id", Name: "proj2_name", Description: "proj2_description", + State: &activeState, }, } domains := []*admin.Domain{ @@ -84,6 +93,7 @@ func TestFromProjectModels(t *testing.T) { assert.Equal(t, fmt.Sprintf("proj%v_id", index+1), project.Id) assert.Equal(t, fmt.Sprintf("proj%v_name", index+1), project.Name) assert.Equal(t, fmt.Sprintf("proj%v_description", index+1), project.Description) + assert.Equal(t, admin.Project_ACTIVE, project.State) assert.EqualValues(t, domains, project.Domains) } }