[autorules] Credentials deletion enhancements #516
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewazores
force-pushed
the
auto-rules-credentials-delete
branch
2 times, most recently
from
f013fa0 to
2337f5e
Compare
|
@vic-ma this is ready |
If Credentials are deleted afer a rule is triggered/a PeriodicArchiver starts, the PeriodicArchiver should fail to archive from that point forward since its Credentials access has been revoked
andrewazores
force-pushed
the
auto-rules-credentials-delete
branch
from
5fb9bd2 to
0fc4f3d
Compare
vic-ma
approved these changes
Jun 16, 2021
andrewazores
added a commit
that referenced
this pull request
Jun 17, 2021
* Respond 404 when deleting nonexistent credentials * Retrieve Credentials from Manager on each PeriodicArchiver run If Credentials are deleted afer a rule is triggered/a PeriodicArchiver starts, the PeriodicArchiver should fail to archive from that point forward since its Credentials access has been revoked * Test credentials revocation during archiver run
andrewazores
added a commit
that referenced
this pull request
Jun 22, 2021
* Rebase fixes * Rough out (very rough) automatic rules * Add very rudimentary configuration file credentials management * Cleanup * Add FIXMEs * Fix mistake, retrieve credentials by alias (for now) * Store credentials under subdirectory of conf dir * Move subdirectory creation into module * Add FIXME * Persist rules to disk Credentials persist optionally (default: false) * Add file open options, some FIXMEs, fix imports * Expect and serialize single credential per file * Expect and serialize single rule per file * Split rule processing logic out of RuleRegistry * Ensure RuleProcessor is created, injected, and enabled by DI root * Extract helper method * Implement periodic archival in rules * Don't retry pruning already-pruned recordings * Normalize URLs (URIs) * Log at lower level * Implement variable number of preserved archives * Add maxAge/maxSize to Rule * Encapsulate Rule fields, add Builder * Rules sanitize own names * Add PeriodicArchiverFactory PeriodicArchiver provided through DI for testability/decoupling * Cleaning * Cleaning * Cleaning * Avoid hardcoded request paths * Cleanup * Add TODO * Add handlers for POSTing Credentials * Add handler for DELETEing Credentials * Implement handlers for POSTing rules * Fixup * Automate add/remove demo app * Implement GET handler for Rules * Change credentials POST path * Include Location header on rules creation response * Handle Rules POST as multipart form or JSON * Rename RulesGetHandler to RuleGetHandler * Implement GET for all rules * Remove unused method * Implement handler to DELETE rules by name * Update pom.xml for configuration directories * Fix Spotbugs bugs * Rename credentials POST handler to reflect targetId path param * Add FIXME * Fix content-type bug, apply formatting * Fix periodic archiver recording name * Make test script host URL configurable Allows for testing while using -web start:dev + CORS setup, for example * Update for multiple demo apps * Remove unnecessary(?) handling for extra form type * Remove 'persist' form attribute * Apply spotless formatting * Revert "Remove unnecessary(?) handling for extra form type" This reverts commit 8e20fad. * Remove TODO * Replace rulestest.sh with AutoRulesIT.java * Add RuleRegistryTest * Add RuleProcessorTest * Apply auto_ prefix to recordings in one place * Remove unnecessary client-side timeout * Minor refactor * Add PeriodicArchiverTest * Extract shared client credentials header logic * Restore rulestest.sh for manual testing * Ensure rule definitions do not partially collide * Implement deleteRules(ServiceRef serviceRef) * Cancel periodic archivers if associated target disappears * Fix logger message formatting * Enable JMX auth on test container * Update FIXME * Correct core version * [autorules] Rules POST handler enhancements (#423) * Use URL-safe Base64 encoder/decoder (#419) * Use URL-safe Base64 encoder/decoder Use encoder which produces, and decoder which expects, the URL-safe Base64 alphabet variant. The decoder is also tolerant of missing padding characters at the end, which the client will not send due to their illegality in the WebSocket Subprotocol header value * Update web-client to include base64url client-side changes * Update web-client to fix build (#424) * Add Quarkus JVM sample app (#418) * Throw exception if added rule collides by name * Throw exception if integer attrs negative Add unit tests for rules post handler * Refactor to extract string constants to enum Add tests for non-integer params * Correct spotbugs findings * Add some Rule attribute assertions and tests * Do all attribute validation in Rule * Correct exception message formatting * Catch IllegalArgumentException and rethrow as ApiException 400 * Minor refactor * Remove no-op CHANGED event handling * Include exception reason in API response message * Rebase fixes * Bugfix: double async target discovery notifications * [autorules] Rules deletion enhancements (#506) * Update rulestest.sh to match current smoketest.sh * Add rule deletion test step * Refactor to not swallow IOExceptions * Track archiver tasks by sref/rule tuple * fixup! Update rulestest.sh to match current smoketest.sh * Deleting Rule definition stops archival tasks using that rule * Use direct API request paths, not injected handler method calls * Respond 404 when deleting non-existent rule * Add unit tests * [autorules] Mention all missing form fields on credentials POST (#511) * Mention all missing form fields on credentials POST * Quote missing parameter names in error message * Handle any blank form value, not only null * Apply spotless formatting * fixup! Handle any blank form value, not only null * [autorules] Cancel periodic archivers on failure (#513) * PeriodicArchivers notify RuleProcessor on archive failure RuleProcessor uses this to cancel the archiver's future runs * Add test that PeriodicArchiver notifies on failure * Apply spotless formatting * Minor refactor * Add test that RuleProcessor cancels failed archivers * Remove unnecessary assertions * [autorules] rulestest.sh touchup (#519) * Set USE_AUTH * Don't delete rule definition * [autorules] Credentials deletion enhancements (#516) * Respond 404 when deleting nonexistent credentials * Retrieve Credentials from Manager on each PeriodicArchiver run If Credentials are deleted afer a rule is triggered/a PeriodicArchiver starts, the PeriodicArchiver should fail to archive from that point forward since its Credentials access has been revoked * Test credentials revocation during archiver run * JMXServiceURL/ServiceRef decoupling rebase fixes * Correct log messages * Correct exception message * Remove rulestest.sh
andrewazores
added a commit
to andrewazores/cryostat
that referenced
this pull request
Jun 24, 2021
* Rebase fixes * Rough out (very rough) automatic rules * Add very rudimentary configuration file credentials management * Cleanup * Add FIXMEs * Fix mistake, retrieve credentials by alias (for now) * Store credentials under subdirectory of conf dir * Move subdirectory creation into module * Add FIXME * Persist rules to disk Credentials persist optionally (default: false) * Add file open options, some FIXMEs, fix imports * Expect and serialize single credential per file * Expect and serialize single rule per file * Split rule processing logic out of RuleRegistry * Ensure RuleProcessor is created, injected, and enabled by DI root * Extract helper method * Implement periodic archival in rules * Don't retry pruning already-pruned recordings * Normalize URLs (URIs) * Log at lower level * Implement variable number of preserved archives * Add maxAge/maxSize to Rule * Encapsulate Rule fields, add Builder * Rules sanitize own names * Add PeriodicArchiverFactory PeriodicArchiver provided through DI for testability/decoupling * Cleaning * Cleaning * Cleaning * Avoid hardcoded request paths * Cleanup * Add TODO * Add handlers for POSTing Credentials * Add handler for DELETEing Credentials * Implement handlers for POSTing rules * Fixup * Automate add/remove demo app * Implement GET handler for Rules * Change credentials POST path * Include Location header on rules creation response * Handle Rules POST as multipart form or JSON * Rename RulesGetHandler to RuleGetHandler * Implement GET for all rules * Remove unused method * Implement handler to DELETE rules by name * Update pom.xml for configuration directories * Fix Spotbugs bugs * Rename credentials POST handler to reflect targetId path param * Add FIXME * Fix content-type bug, apply formatting * Fix periodic archiver recording name * Make test script host URL configurable Allows for testing while using -web start:dev + CORS setup, for example * Update for multiple demo apps * Remove unnecessary(?) handling for extra form type * Remove 'persist' form attribute * Apply spotless formatting * Revert "Remove unnecessary(?) handling for extra form type" This reverts commit 8e20fad. * Remove TODO * Replace rulestest.sh with AutoRulesIT.java * Add RuleRegistryTest * Add RuleProcessorTest * Apply auto_ prefix to recordings in one place * Remove unnecessary client-side timeout * Minor refactor * Add PeriodicArchiverTest * Extract shared client credentials header logic * Restore rulestest.sh for manual testing * Ensure rule definitions do not partially collide * Implement deleteRules(ServiceRef serviceRef) * Cancel periodic archivers if associated target disappears * Fix logger message formatting * Enable JMX auth on test container * Update FIXME * Correct core version * [autorules] Rules POST handler enhancements (cryostatio#423) * Use URL-safe Base64 encoder/decoder (cryostatio#419) * Use URL-safe Base64 encoder/decoder Use encoder which produces, and decoder which expects, the URL-safe Base64 alphabet variant. The decoder is also tolerant of missing padding characters at the end, which the client will not send due to their illegality in the WebSocket Subprotocol header value * Update web-client to include base64url client-side changes * Update web-client to fix build (cryostatio#424) * Add Quarkus JVM sample app (cryostatio#418) * Throw exception if added rule collides by name * Throw exception if integer attrs negative Add unit tests for rules post handler * Refactor to extract string constants to enum Add tests for non-integer params * Correct spotbugs findings * Add some Rule attribute assertions and tests * Do all attribute validation in Rule * Correct exception message formatting * Catch IllegalArgumentException and rethrow as ApiException 400 * Minor refactor * Remove no-op CHANGED event handling * Include exception reason in API response message * Rebase fixes * Bugfix: double async target discovery notifications * [autorules] Rules deletion enhancements (cryostatio#506) * Update rulestest.sh to match current smoketest.sh * Add rule deletion test step * Refactor to not swallow IOExceptions * Track archiver tasks by sref/rule tuple * fixup! Update rulestest.sh to match current smoketest.sh * Deleting Rule definition stops archival tasks using that rule * Use direct API request paths, not injected handler method calls * Respond 404 when deleting non-existent rule * Add unit tests * [autorules] Mention all missing form fields on credentials POST (cryostatio#511) * Mention all missing form fields on credentials POST * Quote missing parameter names in error message * Handle any blank form value, not only null * Apply spotless formatting * fixup! Handle any blank form value, not only null * [autorules] Cancel periodic archivers on failure (cryostatio#513) * PeriodicArchivers notify RuleProcessor on archive failure RuleProcessor uses this to cancel the archiver's future runs * Add test that PeriodicArchiver notifies on failure * Apply spotless formatting * Minor refactor * Add test that RuleProcessor cancels failed archivers * Remove unnecessary assertions * [autorules] rulestest.sh touchup (cryostatio#519) * Set USE_AUTH * Don't delete rule definition * [autorules] Credentials deletion enhancements (cryostatio#516) * Respond 404 when deleting nonexistent credentials * Retrieve Credentials from Manager on each PeriodicArchiver run If Credentials are deleted afer a rule is triggered/a PeriodicArchiver starts, the PeriodicArchiver should fail to archive from that point forward since its Credentials access has been revoked * Test credentials revocation during archiver run * JMXServiceURL/ServiceRef decoupling rebase fixes * Correct log messages * Correct exception message * Remove rulestest.sh
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses the following review deficiencies from #416 :
DELETE /api/v2/targets/:targetId/credentials(TODO):