Skip to content

Commit

Permalink
Add org-aware error messaging for project validation (#49)
Browse files Browse the repository at this point in the history
  • Loading branch information
katrogan authored Jan 29, 2024
1 parent 4742c0b commit aaade8d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
22 changes: 16 additions & 6 deletions flyteadmin/pkg/manager/impl/validation/project_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validation

import (
"context"
"fmt"

"google.golang.org/grpc/codes"
"k8s.io/apimachinery/pkg/util/validation"
Expand All @@ -20,6 +21,15 @@ const maxNameLength = 64
const maxDescriptionLength = 300
const maxLabelArrayLength = 16

const orgErrorMsg = " and org '%s'"

func getOrgForErrorMsg(org string) string {
if len(org) == 0 {
return ""
}
return fmt.Sprintf(orgErrorMsg, org)
}

func ValidateProjectRegisterRequest(request admin.ProjectRegisterRequest) error {
if request.Project == nil {
return shared.GetMissingArgumentError(shared.Project)
Expand Down Expand Up @@ -60,8 +70,8 @@ func ValidateProjectAndDomain(
project, err := db.ProjectRepo().Get(ctx, projectID, org)
if err != nil {
return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
"failed to validate that project [%s] and domain [%s] are registered, err: [%+v]",
projectID, domainID, err)
"failed to validate that project [%s] and domain [%s]%s are registered, err: [%+v]",
projectID, domainID, getOrgForErrorMsg(org), err)
}
if *project.State != int32(admin.Project_ACTIVE) {
return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
Expand All @@ -87,8 +97,8 @@ func ValidateProjectForUpdate(
project, err := db.ProjectRepo().Get(ctx, projectID, org)
if err != nil {
return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
"failed to validate that project [%s] is registered, err: [%+v]",
projectID, err)
"failed to validate that project [%s]%s is registered, err: [%+v]",
projectID, getOrgForErrorMsg(org), err)
}
if *project.State != int32(admin.Project_ACTIVE) {
return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
Expand All @@ -105,8 +115,8 @@ func ValidateProjectExists(
_, err := db.ProjectRepo().Get(ctx, projectID, org)
if err != nil {
return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
"failed to validate that project [%s] exists, err: [%+v]",
projectID, err)
"failed to validate that project [%s]%s exists, err: [%+v]",
projectID, getOrgForErrorMsg(org), err)
}
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func TestValidateProjectAndDomainError(t *testing.T) {
err := ValidateProjectAndDomain(context.Background(), mockRepo, testutils.GetApplicationConfigWithDefaultDomains(),
"flyte-project-id", "domain", "org")
assert.EqualError(t, err,
"failed to validate that project [flyte-project-id] and domain [domain] are registered, err: [foo]")
"failed to validate that project [flyte-project-id] and domain [domain] and org 'org' are registered, err: [foo]")
}

func TestValidateProjectAndDomainNotFound(t *testing.T) {
Expand All @@ -302,7 +302,7 @@ func TestValidateProjectAndDomainNotFound(t *testing.T) {
}
err := ValidateProjectAndDomain(context.Background(), mockRepo, testutils.GetApplicationConfigWithDefaultDomains(),
"flyte-project", "domain", "org")
assert.EqualError(t, err, "failed to validate that project [flyte-project] and domain [domain] are registered, err: [project [flyte-project] not found]")
assert.EqualError(t, err, "failed to validate that project [flyte-project] and domain [domain] and org 'org' are registered, err: [project [flyte-project] not found]")
}

func TestValidateProjectDb(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestValidatWorkflowInvalidProjectAndDomain(t *testing.T) {
request := testutils.GetWorkflowRequest()
err := ValidateWorkflow(context.Background(), request, testutils.GetRepoWithDefaultProjectAndErr(errors.New("foo")),
workflowConfig)
assert.EqualError(t, err, "failed to validate that project [project] and domain [domain] are registered, err: [foo]")
assert.EqualError(t, err, "failed to validate that project [project] and domain [domain] and org 'org' are registered, err: [foo]")
}

func TestValidateWorkflowEmptyDomain(t *testing.T) {
Expand Down

0 comments on commit aaade8d

Please sign in to comment.