Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Commit

Permalink
Merge pull request from GHSA-r847-6w6h-r8g4
Browse files Browse the repository at this point in the history 
* Fix list filters

* Fix sort keys

* split into 2 maps

* move columns to models

* move columns to models
  • Loading branch information
@iaroslav-ciupin
iaroslav-ciupin authored Aug 31, 2023
1 parent 6c97bb9 commit b3177ef
Show file tree
Hide file tree
Showing 51 changed files with 500 additions and 379 deletions.
12 changes: 11 additions & 1 deletion pkg/clusterresource/impl/admin_service_data_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import (
"context"
"fmt"

"github.com/flyteorg/flyteadmin/pkg/clusterresource/interfaces"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/service"

"github.com/flyteorg/flyteadmin/pkg/clusterresource/interfaces"
"github.com/flyteorg/flyteadmin/pkg/common"
"github.com/flyteorg/flyteadmin/pkg/repositories/models"
)

// Implementation of an interfaces.FlyteAdminDataProvider which fetches data using a flyteadmin service client
Expand All @@ -32,6 +35,13 @@ func (p serviceAdminProvider) GetClusterResourceAttributes(ctx context.Context,

var activeProjectsFilter = fmt.Sprintf("ne(state,%d)", admin.Project_ARCHIVED)

var descCreatedAtSortParam = admin.Sort{
Direction: admin.Sort_DESCENDING,
Key: "created_at",
}

var descCreatedAtSortDBParam, _ = common.NewSortParameter(&descCreatedAtSortParam, models.ProjectColumns)

func (p serviceAdminProvider) GetProjects(ctx context.Context) (*admin.Projects, error) {
projects := make([]*admin.Project, 0)
listReq := &admin.ProjectListRequest{
Expand Down
9 changes: 0 additions & 9 deletions pkg/clusterresource/impl/shared.go
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
package impl

import (
"github.com/flyteorg/flyteadmin/pkg/common"
"github.com/flyteorg/flyteadmin/pkg/errors"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"google.golang.org/grpc/codes"
)

func NewMissingEntityError(entity string) error {
return errors.NewFlyteAdminErrorf(codes.NotFound, "Failed to find [%s]", entity)
}

var descCreatedAtSortParam = admin.Sort{
Direction: admin.Sort_DESCENDING,
Key: "created_at",
}

var descCreatedAtSortDBParam, _ = common.NewSortParameter(descCreatedAtSortParam)
16 changes: 13 additions & 3 deletions pkg/common/sorting.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package common

import (
"fmt"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/flyteorg/flyteadmin/pkg/errors"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
Expand All @@ -23,13 +24,22 @@ func (s *sortParamImpl) GetGormOrderExpr() string {
return s.gormOrderExpression
}

func NewSortParameter(sort admin.Sort) (SortParameter, error) {
func NewSortParameter(sort *admin.Sort, allowed sets.String) (SortParameter, error) {
if sort == nil {
return nil, nil
}

key := sort.Key
if !allowed.Has(key) {
return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid sort key '%s'", key)
}

var gormOrderExpression string
switch sort.Direction {
case admin.Sort_DESCENDING:
gormOrderExpression = fmt.Sprintf(gormDescending, sort.Key)
gormOrderExpression = fmt.Sprintf(gormDescending, key)
case admin.Sort_ASCENDING:
gormOrderExpression = fmt.Sprintf(gormAscending, sort.Key)
gormOrderExpression = fmt.Sprintf(gormAscending, key)
default:
return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid sort order specified: %v", sort)
}
Expand Down
41 changes: 35 additions & 6 deletions pkg/common/sorting_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,53 @@ package common
import (
"testing"

"k8s.io/apimachinery/pkg/util/sets"

"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/stretchr/testify/assert"
)

func TestSortParameter_Nil(t *testing.T) {
sortParameter, err := NewSortParameter(nil, nil)

assert.NoError(t, err)
assert.Nil(t, sortParameter)
}

func TestSortParameter_InvalidSortKey(t *testing.T) {
_, err := NewSortParameter(&admin.Sort{
Direction: admin.Sort_ASCENDING,
Key: "wrong",
}, sets.NewString("name"))

assert.EqualError(t, err, "invalid sort key 'wrong'")
}

func TestSortParameter_InvalidSortDirection(t *testing.T) {
_, err := NewSortParameter(&admin.Sort{
Direction: 2,
Key: "name",
}, sets.NewString("name"))

assert.EqualError(t, err, `invalid sort order specified: key:"name" direction:2 `)
}

func TestSortParameter_Ascending(t *testing.T) {
sortParameter, err := NewSortParameter(admin.Sort{
sortParameter, err := NewSortParameter(&admin.Sort{
Direction: admin.Sort_ASCENDING,
Key: "name",
})
assert.Nil(t, err)
}, sets.NewString("name"))

assert.NoError(t, err)
assert.Equal(t, "name asc", sortParameter.GetGormOrderExpr())
}

func TestSortParameter_Descending(t *testing.T) {
sortParameter, err := NewSortParameter(admin.Sort{
sortParameter, err := NewSortParameter(&admin.Sort{
Direction: admin.Sort_DESCENDING,
Key: "project",
})
assert.Nil(t, err)
}, sets.NewString("project"))

assert.NoError(t, err)
assert.Equal(t, "project desc", sortParameter.GetGormOrderExpr())
}
23 changes: 11 additions & 12 deletions pkg/manager/impl/description_entity_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@ import (
"context"
"strconv"

"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flytestdlib/contextutils"
"github.com/flyteorg/flytestdlib/logger"
"github.com/flyteorg/flytestdlib/promutils"
"google.golang.org/grpc/codes"

"github.com/flyteorg/flyteadmin/pkg/common"

"github.com/flyteorg/flyteadmin/pkg/errors"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/util"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/validation"
"github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces"
"github.com/flyteorg/flyteadmin/pkg/repositories/models"
"github.com/flyteorg/flyteadmin/pkg/repositories/transformers"
runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flytestdlib/contextutils"
"github.com/flyteorg/flytestdlib/logger"
"github.com/flyteorg/flytestdlib/promutils"
"google.golang.org/grpc/codes"
)

type DescriptionEntityMetrics struct {
Expand Down Expand Up @@ -65,13 +65,12 @@ func (d *DescriptionEntityManager) ListDescriptionEntity(ctx context.Context, re
logger.Error(ctx, "failed to get database filter")
return nil, err
}
var sortParameter common.SortParameter
if request.SortBy != nil {
sortParameter, err = common.NewSortParameter(*request.SortBy)
if err != nil {
return nil, err
}

sortParameter, err := common.NewSortParameter(request.SortBy, models.DescriptionEntityColumns)
if err != nil {
return nil, err
}

offset, err := validation.ValidateToken(request.Token)
if err != nil {
return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument,
Expand Down
46 changes: 18 additions & 28 deletions pkg/manager/impl/execution_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,32 @@ import (
"strconv"
"time"

"github.com/flyteorg/flytestdlib/promutils/labeled"

"github.com/flyteorg/flyteadmin/plugins"

"github.com/benbjohnson/clock"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/flytek8s"

"github.com/flyteorg/flyteadmin/auth"

"github.com/flyteorg/flyteadmin/pkg/manager/impl/resources"

dataInterfaces "github.com/flyteorg/flyteadmin/pkg/data/interfaces"
"github.com/flyteorg/flytestdlib/contextutils"
"github.com/flyteorg/flytestdlib/logger"
"github.com/flyteorg/flytestdlib/promutils"
"github.com/flyteorg/flytestdlib/promutils/labeled"
"github.com/flyteorg/flytestdlib/storage"
"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/timestamp"
"github.com/prometheus/client_golang/prometheus"
"google.golang.org/grpc/codes"

"github.com/flyteorg/flyteadmin/pkg/common"

"github.com/flyteorg/flytestdlib/logger"
"github.com/flyteorg/flytestdlib/storage"

"github.com/flyteorg/flyteadmin/auth"
cloudeventInterfaces "github.com/flyteorg/flyteadmin/pkg/async/cloudevent/interfaces"
eventWriter "github.com/flyteorg/flyteadmin/pkg/async/events/interfaces"
"github.com/flyteorg/flyteadmin/pkg/async/notifications"
notificationInterfaces "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces"
"github.com/flyteorg/flyteadmin/pkg/common"
dataInterfaces "github.com/flyteorg/flyteadmin/pkg/data/interfaces"
"github.com/flyteorg/flyteadmin/pkg/errors"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/executions"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/resources"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/shared"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/util"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/validation"
"github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
Expand All @@ -42,13 +40,7 @@ import (
"github.com/flyteorg/flyteadmin/pkg/repositories/transformers"
runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces"
workflowengineInterfaces "github.com/flyteorg/flyteadmin/pkg/workflowengine/interfaces"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
"google.golang.org/grpc/codes"

"github.com/benbjohnson/clock"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/shared"
"github.com/golang/protobuf/proto"
"github.com/flyteorg/flyteadmin/plugins"
)

const childContainerQueueKey = "child_queue"
Expand Down Expand Up @@ -1434,12 +1426,10 @@ func (m *ExecutionManager) ListExecutions(
if err != nil {
return nil, err
}
var sortParameter common.SortParameter
if request.SortBy != nil {
sortParameter, err = common.NewSortParameter(*request.SortBy)
if err != nil {
return nil, err
}

sortParameter, err := common.NewSortParameter(request.SortBy, models.ExecutionColumns)
if err != nil {
return nil, err
}

offset, err := validation.ValidateToken(request.Token)
Expand Down
62 changes: 26 additions & 36 deletions pkg/manager/impl/execution_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,64 +3,54 @@ package impl
import (
"context"
"errors"
"fmt"
"strings"
"testing"

"github.com/flyteorg/flyteadmin/plugins"

"google.golang.org/grpc/status"

"google.golang.org/protobuf/types/known/timestamppb"
"time"

"github.com/benbjohnson/clock"
"github.com/flyteorg/flyteadmin/pkg/common"
commonTestUtils "github.com/flyteorg/flyteadmin/pkg/common/testutils"
flyteAdminErrors "github.com/flyteorg/flyteadmin/pkg/errors"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/executions"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/shared"
managerInterfaces "github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
managerMocks "github.com/flyteorg/flyteadmin/pkg/manager/mocks"
"github.com/flyteorg/flyteadmin/pkg/runtime"
"github.com/flyteorg/flyteidl/clients/go/coreutils"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event"
mockScope "github.com/flyteorg/flytestdlib/promutils"
"github.com/flyteorg/flytestdlib/storage"
"github.com/gogo/protobuf/jsonpb"
"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/wrappers"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc/codes"

"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/timestamppb"
"k8s.io/apimachinery/pkg/api/resource"

eventWriterMocks "github.com/flyteorg/flyteadmin/pkg/async/events/mocks"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/flyteorg/flyteadmin/auth"

commonMocks "github.com/flyteorg/flyteadmin/pkg/common/mocks"

"github.com/flyteorg/flytestdlib/storage"

"time"

"fmt"

eventWriterMocks "github.com/flyteorg/flyteadmin/pkg/async/events/mocks"
notificationMocks "github.com/flyteorg/flyteadmin/pkg/async/notifications/mocks"
"github.com/flyteorg/flyteadmin/pkg/common"
commonMocks "github.com/flyteorg/flyteadmin/pkg/common/mocks"
commonTestUtils "github.com/flyteorg/flyteadmin/pkg/common/testutils"
dataMocks "github.com/flyteorg/flyteadmin/pkg/data/mocks"
flyteAdminErrors "github.com/flyteorg/flyteadmin/pkg/errors"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/executions"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/shared"
"github.com/flyteorg/flyteadmin/pkg/manager/impl/testutils"
managerInterfaces "github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
managerMocks "github.com/flyteorg/flyteadmin/pkg/manager/mocks"
"github.com/flyteorg/flyteadmin/pkg/repositories/interfaces"
repositoryMocks "github.com/flyteorg/flyteadmin/pkg/repositories/mocks"
"github.com/flyteorg/flyteadmin/pkg/repositories/models"
"github.com/flyteorg/flyteadmin/pkg/repositories/transformers"
"github.com/flyteorg/flyteadmin/pkg/runtime"
runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces"
runtimeIFaceMocks "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces/mocks"
runtimeMocks "github.com/flyteorg/flyteadmin/pkg/runtime/mocks"
workflowengineInterfaces "github.com/flyteorg/flyteadmin/pkg/workflowengine/interfaces"
workflowengineMocks "github.com/flyteorg/flyteadmin/pkg/workflowengine/mocks"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
mockScope "github.com/flyteorg/flytestdlib/promutils"
"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes/wrappers"
"github.com/stretchr/testify/assert"
"github.com/flyteorg/flyteadmin/plugins"
)

var spec = testutils.GetExecutionRequest().Spec
Expand Down Expand Up @@ -2979,7 +2969,7 @@ func TestListExecutions(t *testing.T) {
assert.True(t, domainFilter, "Missing domain equality filter")
assert.False(t, nameFilter, "Included name equality filter")
assert.Equal(t, limit, input.Limit)
assert.Equal(t, "domain asc", input.SortParameter.GetGormOrderExpr())
assert.Equal(t, "execution_domain asc", input.SortParameter.GetGormOrderExpr())
assert.Equal(t, 2, input.Offset)
assert.EqualValues(t, map[common.Entity]bool{
common.Execution: true,
Expand Down Expand Up @@ -3027,7 +3017,7 @@ func TestListExecutions(t *testing.T) {
Limit: limit,
SortBy: &admin.Sort{
Direction: admin.Sort_ASCENDING,
Key: "domain",
Key: "execution_domain",
},
Token: "2",
})
Expand Down Expand Up @@ -3965,7 +3955,7 @@ func TestListExecutions_LegacyModel(t *testing.T) {
assert.True(t, domainFilter, "Missing domain equality filter")
assert.False(t, nameFilter, "Included name equality filter")
assert.Equal(t, limit, input.Limit)
assert.Equal(t, "domain asc", input.SortParameter.GetGormOrderExpr())
assert.Equal(t, "execution_domain asc", input.SortParameter.GetGormOrderExpr())
assert.Equal(t, 2, input.Offset)
return interfaces.ExecutionCollectionOutput{
Executions: []models.Execution{
Expand Down Expand Up @@ -4010,7 +4000,7 @@ func TestListExecutions_LegacyModel(t *testing.T) {
Limit: limit,
SortBy: &admin.Sort{
Direction: admin.Sort_ASCENDING,
Key: "domain",
Key: "execution_domain",
},
Token: "2",
})
Expand Down
Loading

0 comments on commit b3177ef

Please sign in to comment.