Skip to content

Commit 52af95a

Browse files
authored
refactor(driver_store): Add checks to ensure validity of existing + d… (#283)
1 parent 0fbb8c7 commit 52af95a

16 files changed

Lines changed: 71 additions & 34 deletions

‎docker-compose.yml‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ services:
5050
# Environment
5151
GITHUB_ACTION: ${GITHUB_ACTION:-}
5252
# Service Hosts
53-
<<: [*redis-client-env, *postgresdb-client-env,*deployment-env, *build-api-env]
53+
<<:
54+
[
55+
*redis-client-env,
56+
*postgresdb-client-env,
57+
*deployment-env,
58+
*build-api-env,
59+
]
5460

5561
redis:
5662
image: eqalpha/keydb
@@ -99,7 +105,7 @@ services:
99105
- 9000:9000
100106
- 9090:9090
101107
environment:
102-
<< : *s3-client-env
108+
<<: *s3-client-env
103109
MINIO_ROOT_USER: $AWS_KEY
104110
MINIO_ROOT_PASSWORD: $AWS_SECRET
105111
command: server /data --console-address ":9090"
@@ -109,11 +115,11 @@ services:
109115
depends_on:
110116
- minio
111117
environment:
112-
<< : *s3-client-env
118+
<<: *s3-client-env
113119
entrypoint: >
114120
sh -c '
115121
sleep 3 &&
116-
mc config host add s3 $AWS_S3_ENDPOINT $AWS_KEY $AWS_SECRET &&
122+
mc alias set s3 $AWS_S3_ENDPOINT $AWS_KEY $AWS_SECRET &&
117123
mc mb -p s3/$AWS_S3_BUCKET &&
118124
exit 0
119125
'

‎spec/helper.cr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ macro around_suite(block)
5555
end
5656
end
5757

58-
around_suite ->{
58+
around_suite -> {
5959
clear_tables
6060
}
6161

‎src/api/chaos.cr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ module PlaceOS::Core::Api
1414
@[AC::Param::Info(name: path, description: "the driver executable name", example: "drivers_place_meet_c54390a")]
1515
driver_key : String,
1616
@[AC::Param::Info(description: "optionally provide the edge id the driver is running on", example: "edge-12345")]
17-
edge_id : String? = nil
17+
edge_id : String? = nil,
1818
) : Nil
1919
raise Error::NotFound.new("no process manager found for #{driver_key}") unless manager = module_manager.process_manager(driver_key, edge_id)
2020
manager.kill(driver_key)

‎src/api/command.cr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module PlaceOS::Core::Api
1212
@[AC::Route::POST("/:module_id/load")]
1313
def load(
1414
@[AC::Param::Info(description: "the module id we want to load", example: "mod-1234")]
15-
module_id : String
15+
module_id : String,
1616
) : Nil
1717
mod = Model::Module.find(module_id)
1818
raise Error::NotFound.new("module #{module_id} not found in database") unless mod
@@ -25,7 +25,7 @@ module PlaceOS::Core::Api
2525
@[AC::Param::Info(description: "the module id we want to send an execute request to", example: "mod-1234")]
2626
module_id : String,
2727
@[AC::Param::Info(description: "the user context for the execution", example: "user-1234")]
28-
user_id : String? = nil
28+
user_id : String? = nil,
2929
) : Nil
3030
unless module_manager.process_manager(module_id, &.module_loaded?(module_id))
3131
Log.info { {module_id: module_id, message: "module not loaded"} }
@@ -62,7 +62,7 @@ module PlaceOS::Core::Api
6262
def module_debugger(
6363
socket,
6464
@[AC::Param::Info(description: "the module we want to debug", example: "mod-1234")]
65-
module_id : String
65+
module_id : String,
6666
) : Nil
6767
# Forward debug messages to the websocket
6868
module_manager.process_manager(module_id, &.attach_debugger(module_id, socket))

‎src/api/drivers.cr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ module PlaceOS::Core::Api
1616
@[AC::Param::Info(description: "the commit hash of the driver to check is compiled", example: "e901494")]
1717
commit : String,
1818
@[AC::Param::Info(description: "the driver database id", example: "driver-GFEaAlJB5")]
19-
tag : String
19+
tag : String,
2020
) : Bool
2121
driver = Model::Driver.find!(tag)
2222
repository = driver.repository!
@@ -31,7 +31,7 @@ module PlaceOS::Core::Api
3131
@[AC::Param::Info(description: "the commit hash of the driver to check is compiled", example: "e901494")]
3232
commit : String,
3333
@[AC::Param::Info(description: "the driver database id", example: "driver-GFEaAlJB5")]
34-
tag : String
34+
tag : String,
3535
) : String
3636
driver = Model::Driver.find!(tag)
3737
repository = driver.repository!
@@ -47,7 +47,7 @@ module PlaceOS::Core::Api
4747
@[AC::Route::POST("/:driver_id/reload")]
4848
def reload(
4949
@[AC::Param::Info(description: "the driver database id", example: "driver-GFEaAlJB5")]
50-
driver_id : String
50+
driver_id : String,
5151
) : String
5252
result = store.reload_driver(driver_id)
5353
render status: result[:status], text: result[:message]
@@ -63,7 +63,7 @@ module PlaceOS::Core::Api
6363
@[AC::Param::Info(description: "the commit hash of the driver to be built", example: "e901494")]
6464
commit : String,
6565
@[AC::Param::Info(description: "the branch of the repository", example: "main")]
66-
branch : String = "master"
66+
branch : String = "master",
6767
) : Nil
6868
Log.context.set(driver: driver_file, repository: repository, commit: commit, branch: branch)
6969
repo = Model::Repository.find!(repository)

‎src/api/edge.cr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ module PlaceOS::Core::Api
1313
def edge_control(
1414
socket,
1515
@[AC::Param::Info(description: "the edge this device is handling", example: "edge-1234")]
16-
edge_id : String
16+
edge_id : String,
1717
) : Nil
1818
module_manager.manage_edge(edge_id, socket)
1919
end

‎src/api/status.cr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ module PlaceOS::Core::Api
4242
@[AC::Route::GET("/driver")]
4343
def driver(
4444
@[AC::Param::Info(name: "path", description: "the path of the compiled driver", example: "/path/to/compiled_driver")]
45-
driver_path : String
45+
driver_path : String,
4646
) : DriverStatus
4747
DriverStatus.new(
4848
local: module_manager.local_processes.driver_status(driver_path),

‎src/constants.cr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ module PlaceOS::Core
2323
# In k8s we can grab the Pod information from the environment
2424
# https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/#use-pod-fields-as-values-for-environment-variables
2525
CORE_HOST_RAW = ENV["CORE_HOST"]? || System.hostname
26-
CORE_HOST = !CORE_HOST_RAW.starts_with?('[') && CORE_HOST_RAW.includes?(':') ? "[#{CORE_HOST_RAW}]" : CORE_HOST_RAW
27-
CORE_PORT = (ENV["CORE_PORT"]? || "3000").to_i
26+
CORE_HOST = !CORE_HOST_RAW.starts_with?('[') && CORE_HOST_RAW.includes?(':') ? "[#{CORE_HOST_RAW}]" : CORE_HOST_RAW
27+
CORE_PORT = (ENV["CORE_PORT"]? || "3000").to_i
2828

2929
PROD = ENV["SG_ENV"]?.try(&.downcase) == "production"
3030

‎src/placeos-core/driver_manager/driver_store.cr‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,17 @@ module PlaceOS::Core
1818
def compiled?(file_name : String, commit : String, branch : String, uri : String) : Bool
1919
Log.debug { {message: "Checking whether driver is compiled or not?", driver: file_name, commit: commit, branch: branch, repo: uri} }
2020
path = Path[binary_path, executable_name(file_name, commit)]
21-
return true if File.exists?(path)
21+
22+
if File.exists?(path)
23+
# Validate that the local file is a valid executable by running it with -h
24+
if validate_binary(path)
25+
return true
26+
else
27+
Log.warn { {message: "Local binary exists but is corrupted, removing and re-downloading", driver_file: file_name, path: path.to_s} }
28+
File.delete(path) rescue nil
29+
end
30+
end
31+
2232
resp = BuildApi.compiled?(file_name, commit, branch, uri)
2333
return false unless resp.success?
2434
ret = fetch_binary(LinkData.from_json(resp.body)) rescue nil
@@ -90,6 +100,16 @@ module PlaceOS::Core
90100
{driver_source, commit, Core::ARCH}.join("_").downcase
91101
end
92102

103+
private def validate_binary(path : Path) : Bool
104+
# Try to execute the binary with -h flag to validate it's a working executable
105+
result = Process.run(path.to_s, ["-h"], output: Process::Redirect::Close, error: Process::Redirect::Close)
106+
# If the process runs without crashing, consider it valid
107+
result.exit_code == 0
108+
rescue ex : Exception
109+
Log.error(exception: ex) { {message: "Driver binary validation failed", path: path.to_s} }
110+
false
111+
end
112+
93113
def reload_driver(driver_id : String)
94114
if driver = Model::Driver.find?(driver_id)
95115
repo = driver.repository!
@@ -120,16 +140,27 @@ module PlaceOS::Core
120140
ConnectProxy::HTTPClient.new(url.host.not_nil!, 9000).get(uri.to_s)
121141
end
122142
if resp.success?
123-
unless link.size == resp.headers.fetch("Content-Length", "0").to_i
124-
Log.error { {message: "Expected content length #{link.size}, but received #{resp.headers.fetch("Content-Length", "0")}", driver_file: driver_file} }
143+
# Check Content-Length header first if available
144+
content_length = resp.headers.fetch("Content-Length", "0").to_i64
145+
if content_length > 0 && link.size != content_length
146+
Log.error { {message: "Expected content length #{link.size}, but received #{content_length}", driver_file: driver_file} }
125147
raise Error.new("Response size doesn't match with build service returned result")
126148
end
127149

128150
body_io = IO::Digest.new(resp.body_io? || IO::Memory.new(resp.body), Digest::MD5.new)
151+
bytes_written = 0_i64
129152
File.open(filename, "wb+") do |f|
130-
IO.copy(body_io, f)
153+
bytes_written = IO.copy(body_io, f)
131154
f.chmod(0o755)
132155
end
156+
157+
# Verify actual downloaded size matches expected size
158+
unless link.size == bytes_written
159+
Log.error { {message: "Expected download size #{link.size}, but actually downloaded #{bytes_written} bytes", driver_file: driver_file} }
160+
File.delete(filename) if File.exists?(filename)
161+
raise Error.new("Downloaded size doesn't match expected size from build service")
162+
end
163+
133164
filename.to_s
134165
else
135166
raise Error.new("Unable to fetch driver. Error : #{resp.body}")

‎src/placeos-core/mappings/control_system_modules.cr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module PlaceOS::Core
1212

1313
def initialize(
1414
@startup : Bool = true,
15-
@module_manager : ModuleManager = ModuleManager.instance
15+
@module_manager : ModuleManager = ModuleManager.instance,
1616
)
1717
super()
1818
end
@@ -28,7 +28,7 @@ module PlaceOS::Core
2828
def self.update_mapping(
2929
system : Model::ControlSystem,
3030
startup : Bool = false,
31-
module_manager : ModuleManager = ModuleManager.instance
31+
module_manager : ModuleManager = ModuleManager.instance,
3232
) : Resource::Result
3333
relevant_node = startup || module_manager.discovery.own_node?(system.id.as(String))
3434
unless relevant_node
@@ -57,7 +57,7 @@ module PlaceOS::Core
5757
#
5858
def self.update_logic_modules(
5959
system : Model::ControlSystem,
60-
module_manager : ModuleManager = ModuleManager.instance
60+
module_manager : ModuleManager = ModuleManager.instance,
6161
) : Int32
6262
return 0 if system.destroyed?
6363

@@ -90,7 +90,7 @@ module PlaceOS::Core
9090
# Pass module_id and updated_name to overrride a lookup
9191
def self.set_mappings(
9292
control_system : Model::ControlSystem,
93-
mod : Model::Module?
93+
mod : Model::Module?,
9494
) : Hash(String, String)
9595
system_id = control_system.id.as(String)
9696
storage = Driver::RedisStorage.new(system_id, "system")

0 commit comments

Comments
 (0)