brigadecore / brigade

Event-driven scripting for Kubernetes
https://brigade.sh/
Apache License 2.0
2.41k stars 246 forks source link

fix project update failure if project doesn't exist and not logged in as root #1958

Closed AnuragThePathak closed 2 years ago

AnuragThePathak commented 2 years ago

Signed-off-by: Anurag Pathak anuragpathak911@gmail.com Fixes #1947

What this PR does / why we need it: It solves the issue of brig update receiving authorization error when project doesn't exist and not logged in as root.

Special notes for your reviewer: I've removed checking of project not found error in the project update function after the update attempt using Project Store because I feel it is no longer relevant now as we already did that earlier part of that function. I have also modified the unit test just that all test cases pass. However they may be rearranged now to match the change in the update function (I can do that as well if required). Moreover when -c flag is used we create a project which is requested to be updated but in logs we inform the project to be updated instead of informing it to be created. Not really sure if that's how it's supposed to work.

If applicable:

netlify[bot] commented 2 years ago

Deploy Preview for brigade-docs ready!

Name Link
Latest commit 8531d2bdee106df6aca5300d0026be921fb5298f
Latest deploy log https://app.netlify.com/sites/brigade-docs/deploys/62835a3147492e0008d0c950
Deploy Preview https://deploy-preview-1958--brigade-docs.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

krancour commented 2 years ago

@AnuragThePathak this looks great!

I am noticing one issue -- but it's not new -- not introduced by your change. That is that we aren't verifying the user has permission to create projects before proceeding with project creation.

Especially given that this PR does not introduce that problem, let's treat it as a separate problem. I'll open a new issue and if you don't mind, I'll assign it to you.

/brig run