-
Notifications
You must be signed in to change notification settings - Fork 2
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
Invoke CreateProject from Tenant #161
Conversation
This pull request has been linked to Shortcut Story #13122: Tenant should create project in Quarterdeck when a Project is created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
If the Quarterdeck project create fails do we want to delete the project in trtl database, or do we want to retry Quarterdeck project create later?
// TODO: Distinguish between trtl errors and quarterdeck errors. | ||
if err = s.createProject(ctx, tproject); err != nil { | ||
log.Error().Err(err).Msg("could not create project") | ||
c.JSON(qd.ErrorStatus(err), api.ErrorResponse("could not create project")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a trtl
error what will qd.ErrorStatus(err)
do - does it return a 500?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will return a 500. I was thinking we might want to have a struct similar to StatusError
either in the tenant db package or in trtl so that we could switch on on the type here and return a more useful status code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me - would you mind creating a follow on story for that?
Good question - my current thinking is that we should just try to delete the project. |
Ok - deleting makes sense; do we have a story for that? |
Scope of changes
This completes the project security checks on the Tenant side by telling Quarterdeck to create the organization-project link when the user creates a project. This ensures that users will be able to edit api keys only in their organization since Quarterdeck can check to make sure that the organization-project mapping already exists.
Fixes SC-13122
Type of change
Acceptance criteria
Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible.
Author checklist
Reviewer(s) checklist