Skip to content
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.

Sign up for GitHub

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(gate): Typos in Account Management API #1510

Merged
merged 2 commits into from
Jan 26, 2022
Merged

fix(gate): Typos in Account Management API #1510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9622c3
fix(gate/web): Fix typo in PreAuthorize annotation
jvz Jan 20, 2022
3e28cd6
fix(gate/core): Fix retrofit signature error
jvz Jan 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/ClouddriverService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ List<AccountDefinition> getAccountDefinitionsByType(
AccountDefinition updateAccountDefinition(@Body AccountDefinition accountDefinition);

@DELETE("/credentials/{account}")
void deleteAccountDefinition(@Path("account") String account);
Response deleteAccountDefinition(@Path("account") String account);

@GET("/task/{taskDetailsId}")
Map getTaskDetails(@Path("taskDetailsId") String taskDetailsId);
Expand Down
4 changes: 2 additions & 2 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/CredentialsController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class CredentialsController {

@PutMapping
@ApiOperation('Updates an existing account definition.')
@PreAuthorize("hasPermission(#definition.name, 'ACCOUNT', 'WRITE')")
@PreAuthorize("hasPermission(#accountDefinition.name, 'ACCOUNT', 'WRITE')")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gotta ask if it's possible/easy to write a test that exposes this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Java files, IntelliJ throws warnings at you when you try to reference things in SpEL annotations that don't exist. The Groovy version doesn't do that, however. A test could be possible, though it's mostly setting up test fixtures and verifying said fixture works as expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe one day this'll become java...

@Alpha
ClouddriverService.AccountDefinition updateAccount(
@ApiParam('Account definition body including a discriminator field named "@type" with the account type.')
Expand All @@ -132,7 +132,7 @@ class CredentialsController {
@DeleteMapping('/{accountName}')
@ApiOperation(value = 'Deletes an account definition by name.',
notes = 'Deleted accounts can be restored via the update API. Previously deleted accounts cannot be "created" again to avoid conflicts with existing pipelines.')
@PreAuthorize("hasPermission(#definition.name, 'ACCOUNT', 'WRITE')")
@PreAuthorize("hasPermission(#accountName, 'ACCOUNT', 'WRITE')")
@Alpha
void deleteAccount(
@ApiParam('Name of account definition to delete.')
Expand Down