Screwdriver: Fails to delete a Pipeline belonging to a non-existent repository

[FenrirUnbound]

Expected

Given a deleted repository, when you delete the corresponding Pipeline, it will remove the Pipeline and all data associated with it.

Actual

Delete a repository. When you delete the corresponding Pipeline, the server returns a 500. The Pipeline also continues to persist.

Initial Investigation

There is a permissions check to validate that the User can modify the repository. This, in turn, queries the Github API on that repository. Since it's deleted, the scm.getPermissions() call throws an error.

[FenrirUnbound]

Given how Github treats a protected repository as a missing repository, I'm unsure of a clean way to handle this scenario. At least serving a proper 404 would be a nice start, but that doesn't resolve the specific workflow of deleting things out of order.

[d2lam]

Yeah this is a known problem.
It's good you created an issue, so we can track it here.

[stjohnjohnson]

I think there should be two failure cases:

• Branch doesn't exist
• Repo doesn't exist

In the event of Repo doesn't exist, the project is technically "orphaned" and we should either auto-remove it or inform the admins to remove it.

[Filbird]

Indeed, if a user attempts to delete a private repository it does not have access to, a 404 is thrown. For the time being I think we can enable deletion of pipelines with missing repositories if the privateRepo flag is set to false.

It seems that in the case of a branch not existing, the API does NOT return a 404. I wonder if it would be worth making use of web hooks to automatically remove a pipeline when a branch is deleted?

[Filbird]

@tkyi Brought up an alternative solution in https://github.com/screwdriver-cd/screwdriver/pull/1187.

It's possible we could get a 404 code for a different reason, and I'm concerned we're not covering all cases right now. What if we don't allow users to delete a pipeline when there's a 404, but we allow Screwdriver cluster admins. That way, users can file a support request if they really need the pipeline to be deleted and we can manually do it.
We can reuse the check used here: https://github.com/screwdriver-cd/screwdriver/blob/master/plugins/banners/index.js#L24

I think it makes sense to err on the side of caution and go with this approach.