AF-30 Clustering implementation #54
Open
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
10 tasks
sparshev
added a commit
that referenced
this pull request
Feb 23, 2024
Fixed multiple issues with Native driver on macos - mostly: * Proper passing of metadata as env variables to the workload - sudo & su are quite prohibitive there and to not make the node config harder I used just storing the metadata to env export variable and source it in the shell command. The file is stored in tmp and available via ACL to the newly created user only. * Proper log output for the workload right into the node log - it prepends stdout/stderr lines with the resource identifier and looks nice. * Using current fish node group as dynamic user group by default - otherwise the user is incomplete. * Configuration for the binaries location - by default it tries to find it in PATH, but now it's easy to override. * Modern MacOS on M1 doesn't allow to remove the created user, so have to give aquarium-fish binary "Full Disk Access" permission (added to wiki). Also a couple of small improvements: * Moved improved tests helpers from #54 * Moved a couple of fixes for build system from #54 * Found & fixed issue with single-process archiving during the build on macos
* Added clustered nodes support for integration tests & simple test for cluster
* Switched to fasthttp from deprecated gorilla websocket
* Prepared the initial sync functions
* Reorganized the cluster initiator-receiver to be in the same place
* Completed the basic synchronization of the created/updated types
* Added some cluster docs in the readme, added crc32 for skipping repeated msg
* Removed the node description due to useless in general for the cluster
* Added proper messages sync in the cluster & fixed a couple of bugs
* Now messages will be processed/broadcaster only if sum is unique
* Fixed a couple of cluster init issues
* Moved cluster hooks to use messages instead of maps
* Moved integration test logging to use node name instead of node
address - it's harder to figure out what's happening with randomly
generated port then with the node name.
* Added long chain (5 locations) cluster sync & simple cluster tests
* Added proper tests fish node stop procedure
* Moved sync process closer to cluster and reworked cluster info update
* Added startup maintenance mode for first or debug cluster connect
* Clarified interface and added handy stop/start functions to control fish
* Trying better constants to run parallel in github CI
* Fixed issue with check on modern golang
* Added node pubkey verification and a way to request node cluster connections
* Added Resource to the list of cluster objects to sync
* Added precaution to not accidentally join the wrong cluster
* Added node name receiving and storing in the cluster client connection
* Found that implementing the whole cluster right away is quite hard...
* Removed necessity of providing vote to the executeApplication
* This allows to use Resource directly when the Application is already running
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // We do not fail here because we should not give attacker more information | ||
| return as, nil | ||
| } | ||
| db = db.Where(securedFilter) |
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 4 months ago
To fix the problem, we should avoid directly embedding user-controlled data into SQL queries. Instead, we should use parameterized queries provided by the database library. In this case, we can modify the ApplicationStateFind function to use parameterized queries with the db.Where method.
- Modify the
ApplicationStateFindfunction inlib/fish/application_state.goto use parameterized queries. - Ensure that the
ExpressionSQLFilterfunction inlib/util/expression_sql_filter.goreturns a safe, parameterized query.
Suggested changeset
2
lib/fish/application_state.go
| @@ -28,3 +28,3 @@ | ||
| if filter != nil { | ||
| securedFilter, err := util.ExpressionSQLFilter(*filter) | ||
| securedFilter, args, err := util.ExpressionSQLFilter(*filter) | ||
| if err != nil { | ||
| @@ -34,3 +34,3 @@ | ||
| } | ||
| db = db.Where(securedFilter) | ||
| db = db.Where(securedFilter, args...) | ||
| } |
lib/util/expression_sql_filter.go
Outside changed files
| @@ -26,3 +26,3 @@ | ||
| // * `id = 1 OR lol in (SELECT * FROM users)` - will fail | ||
| func ExpressionSQLFilter(filter string) (string, error) { | ||
| func ExpressionSQLFilter(filter string) (string, []interface{}, error) { | ||
| reader := strings.NewReader(filter) | ||
| @@ -30,5 +30,6 @@ | ||
| if err != nil { | ||
| return "", err | ||
| return "", nil, err | ||
| } | ||
| return exp.String(), nil | ||
| query, args := exp.ToSQL() | ||
| return query, args, nil | ||
| } |
Copilot is powered by AI and may make mistakes. Always verify output.
| // We do not fail here because we should not give attacker more information | ||
| return at, nil | ||
| } | ||
| db = db.Where(secured_filter) |
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 4 months ago
To fix the problem, we should avoid directly embedding user-provided input into SQL queries. Instead, we should use parameterized queries to safely include user input. This involves modifying the ExpressionSQLFilter function to return a parameterized query and the corresponding parameters, and then using these parameters in the db.Where method.
Suggested changeset
2
lib/fish/application_task.go
| @@ -28,3 +28,3 @@ | ||
| if filter != nil { | ||
| secured_filter, err := util.ExpressionSQLFilter(*filter) | ||
| secured_filter, args, err := util.ExpressionSQLFilter(*filter) | ||
| if err != nil { | ||
| @@ -34,3 +34,3 @@ | ||
| } | ||
| db = db.Where(secured_filter) | ||
| db = db.Where(secured_filter, args...) | ||
| } | ||
| @@ -44,3 +44,3 @@ | ||
| if filter != nil { | ||
| securedFilter, err := util.ExpressionSQLFilter(*filter) | ||
| securedFilter, args, err := util.ExpressionSQLFilter(*filter) | ||
| if err != nil { | ||
| @@ -51,3 +51,3 @@ | ||
| // Adding parentheses to be sure we're have `application_uid AND (filter)` | ||
| db = db.Where("(" + securedFilter + ")") | ||
| db = db.Where("(" + securedFilter + ")", args...) | ||
| } |
lib/util/expression_sql_filter.go
Outside changed files
| @@ -26,3 +26,3 @@ | ||
| // * `id = 1 OR lol in (SELECT * FROM users)` - will fail | ||
| func ExpressionSQLFilter(filter string) (string, error) { | ||
| func ExpressionSQLFilter(filter string) (string, []interface{}, error) { | ||
| reader := strings.NewReader(filter) | ||
| @@ -30,5 +30,5 @@ | ||
| if err != nil { | ||
| return "", err | ||
| return "", nil, err | ||
| } | ||
| return exp.String(), nil | ||
| return exp.String(), exp.Args(), nil | ||
| } |
Copilot is powered by AI and may make mistakes. Always verify output.
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 change should add a proper clustering support with all the great functionality we had and even more with resistance to brain-split.
The change is BREAKING because removes the node description: not actually needed since all the decisions are made on the node itself.
TODO: Add more tests and proper documentation of each aspect of the cluster.
Related Issue
#30
Motivation and Context
Without a p2p cluster - what a life?
How Has This Been Tested?
Automatically
Screenshots (if appropriate):
Types of changes
Checklist: