fix(graphql): Validate origin_branch on BranchCreate(#5674) #6703
Conversation
|
|
|
Thanks @SalarAmir for your contribution! One of the CI pipeline checks failed, would you be able to make the quick change to lint the python file please and push to your fork? 🙏🏻 |
There was a problem hiding this comment.
thanks for the PR @SalarAmir, I left a few comments to go with Pete's suggestion on fixing the linting issue
| @@ -9,6 +9,7 @@ | |||
|
|
|||
| from infrahub.core.branch import Branch | |||
| from infrahub.database import retry_db_transaction | |||
| from infrahub.exceptions import InfrahubError | |||
There was a problem hiding this comment.
this is not a valid import. I'd suggest replacing InfrahubError with ProcessingError
| if model.origin_branch: | ||
| if model.origin_branch == model.name: | ||
| raise InfrahubError(f"A branch cannot originate from itself: {model.name}") | ||
| origin_branch_obj = await Branch.get_by_name(db=graphql_context.db, name=model.origin_branch) |
There was a problem hiding this comment.
this call will raise a BranchNotFoundError if the branch does not exist, so the following conditional won't ever be reached
| raise InfrahubError(f"A branch cannot originate from itself: {model.name}") | ||
| origin_branch_obj = await Branch.get_by_name(db=graphql_context.db, name=model.origin_branch) | ||
| if not origin_branch_obj: | ||
| raise InfrahubError(f"Origin branch '{model.origin_branch}' does not exist.") |
There was a problem hiding this comment.
I know the bug report does not make this clear, but we actually only allow branching from the default branch. It would be good to check for that in this conditional, something like
if not origin_branch_obj.is_default:
raise ProcessingError(...)
This pull request addresses and resolves issue #5674.
The BranchCreate mutation previously allowed the creation of a branch with a non-existent origin_branch or with an origin_branch that was the same as the branch being created. This commit introduces validation to prevent these invalid states.
Changes:
Validation Logic: Added checks in the BranchCreate.mutate method to ensure:
The origin_branch exists before creating a new branch from it.
A branch cannot be its own origin_branch.
Error Handling: The mutation now raises an InfrahubError with a descriptive message if the validation fails.
Unit Tests: Added a new test class, TestBranchCreateWithOrigin, with three test cases to verify the correctness of the new validation logic.
These changes ensure the integrity of branch creation and provide clear feedback to the user in case of an error.