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

Commit

Permalink
change to user_identifier
Browse files Browse the repository at this point in the history
Signed-off-by: byhsu <[email protected]>
  • Loading branch information
ByronHsu committed May 2, 2023
1 parent 6565f1e commit d04c400
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 11 deletions.
17 changes: 7 additions & 10 deletions pkg/manager/impl/execution_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,10 @@ func (m *ExecutionManager) getExecutionConfig(ctx context.Context, request *admi
}

// In the case of reference_launch_plan subworkflow, the context comes from flytepropeller instead of the user side, so user auth is missing.
// We skip getUserIDFromContext but can still get ExecUserId because flytepropeller passes it in the execution request.
// We skip getUserIdentityFromContext but can still get ExecUserId because flytepropeller passes it in the execution request.
// https://github.com/flyteorg/flytepropeller/blob/03a6672960ed04e7687ba4f790fee9a02a4057fb/pkg/controller/nodes/subworkflow/launchplan/admin.go#L114
if workflowExecConfig.GetSecurityContext().GetRunAs().GetExecUserId() == "" {
workflowExecConfig.SecurityContext.RunAs.ExecUserId, err = getUserIDFromContext(ctx)
if workflowExecConfig.GetSecurityContext().GetRunAs().GetUserIdentifier() == "" {
workflowExecConfig.SecurityContext.RunAs.UserIdentifier, err = getUserIdentityFromContext(ctx)

if err != nil {
return nil, err
Expand All @@ -424,13 +424,10 @@ func (m *ExecutionManager) getExecutionConfig(ctx context.Context, request *admi
return &workflowExecConfig, nil
}

// TODO: move this out of the core logic
func getUserIDFromContext(ctx context.Context) (string, error) {
idx := auth.IdentityContextFromContext(ctx)
userInfo := idx.UserInfo()
email := userInfo.Email
func getUserIdentityFromContext(ctx context.Context) (string, error) {
idCtx := auth.IdentityContextFromContext(ctx)

return email, nil
return idCtx.UserID(), nil
}

func (m *ExecutionManager) getClusterAssignment(ctx context.Context, request *admin.ExecutionCreateRequest) (
Expand Down Expand Up @@ -702,7 +699,7 @@ func resolveSecurityCtx(ctx context.Context, executionConfigSecurityCtx *core.Se
if executionConfigSecurityCtx != nil && executionConfigSecurityCtx.RunAs != nil &&
(len(executionConfigSecurityCtx.RunAs.K8SServiceAccount) > 0 ||
len(executionConfigSecurityCtx.RunAs.IamRole) > 0 ||
len(executionConfigSecurityCtx.RunAs.ExecUserId) > 0) {
len(executionConfigSecurityCtx.RunAs.UserIdentifier) > 0) {
return executionConfigSecurityCtx
}
logger.Warn(ctx, "Setting security context from auth Role")
Expand Down
11 changes: 11 additions & 0 deletions pkg/manager/impl/execution_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/flyteorg/flyteadmin/pkg/runtime"
"github.com/flyteorg/flyteidl/clients/go/coreutils"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event"
"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/service"
"github.com/gogo/protobuf/jsonpb"
"github.com/golang/protobuf/ptypes"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -5330,3 +5331,13 @@ func TestAddStateFilter(t *testing.T) {
})

}

func TestGetUserIdentityFromContext(t *testing.T) {

idCtx, err := auth.NewIdentityContext("", "byhsu", "", time.Now(), sets.String{}, &service.UserInfoResponse{}, map[string]interface{}{})
assert.NoError(t, err)
ctx := context.WithValue(context.Background(), auth.ContextKeyIdentityContext, idCtx)
uid, err := getUserIdentityFromContext(ctx)
assert.NoError(t, err)
assert.Equal(t, "byhsu", uid)
}
2 changes: 1 addition & 1 deletion pkg/manager/impl/util/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func MergeIntoExecConfig(workflowExecConfig admin.WorkflowExecutionConfig, spec
if spec.GetSecurityContext().GetRunAs() != nil &&
(len(spec.GetSecurityContext().GetRunAs().GetK8SServiceAccount()) > 0 ||
len(spec.GetSecurityContext().GetRunAs().GetIamRole()) > 0 ||
len(spec.GetSecurityContext().GetRunAs().GetExecUserId()) > 0) {
len(spec.GetSecurityContext().GetRunAs().GetUserIdentifier()) > 0) {
workflowExecConfig.SecurityContext = spec.GetSecurityContext()
}
}
Expand Down

0 comments on commit d04c400

Please sign in to comment.