-
Notifications
You must be signed in to change notification settings - Fork 590
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
fix(authorization): canCreate should not return void #502
Conversation
fiat-web/src/main/java/com/netflix/spinnaker/fiat/controllers/AuthorizeController.java
Outdated
Show resolved
Hide resolved
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.
Argument /:userId/:resourceType/create
makes sense to me.
fiat-web/src/main/java/com/netflix/spinnaker/fiat/controllers/AuthorizeController.java
Outdated
Show resolved
Hide resolved
fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatService.java
Outdated
Show resolved
Hide resolved
fiat-web/src/main/java/com/netflix/spinnaker/fiat/controllers/AuthorizeController.java
Outdated
Show resolved
Hide resolved
Co-Authored-By: Cameron Fieber <cameron@fieber.ca>
I was wrong about not changing the URI - this is breaking since we always do always call fiat on CREATE (and the enable/disable flag is on the fiat side). We should leave the URI as is, or if we want to migrate it to the new form we should maintain a request mapping in fiat for the old form for some period of time - otherwise we are stuck in the situation of breaking front50 until we deploy new fiat (and then breaking old front50 until we deploy new). |
The thing is, it is already broken in its current state, so if we sneak that change in, we won't break something that works |
Ah you are right - good point. Gonna |
@spinnakerbot cherry-pick 1.17 |
Fixes Retrofit client for canCreate. Modifies create check endpoint for consistency.
Cherry pick successful: #504 |
Retrofit methods cannot have a
void
return type. We either need to provide a return type, or a callback. This fixes the following exception:I also added
canCreate
to the path, because it makes more sense.