Skip to content

Commit c27a1fe

Browse files
gememmagememma
committed
Fix check for existing ip table chain names
1 parent 46ebca6 commit c27a1fe

File tree

6 files changed

+43
-52
lines changed

6 files changed

+43
-52
lines changed

‎Cargo.lock

-25
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎mirrord/agent/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ hickory-resolver.workspace = true
4949
bollard = "0.18"
5050
tokio-util.workspace = true
5151
streammap-ext.workspace = true
52-
sysinfo = "0.29"
5352
libc.workspace = true
5453
faccess = "0.2"
5554
bytes.workspace = true

‎mirrord/agent/iptables/src/error.rs

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ impl From<io::Error> for IPTablesError {
1515
}
1616
}
1717

18+
impl From<String> for IPTablesError {
19+
fn from(value: String) -> Self {
20+
Self(value.into())
21+
}
22+
}
23+
1824
impl From<IPTablesError> for Arc<dyn Error + Send + Sync + 'static> {
1925
fn from(value: IPTablesError) -> Self {
2026
value.0.into()

‎mirrord/agent/iptables/src/lib.rs

+20-9
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,26 @@ where
186186
let ipt = Arc::new(ipt);
187187

188188
tracing::trace!("checking for leftover or existing iptable chains...");
189-
// FIXME: change from .ok().len(), need a map over inner, could probably collect into a vec
190-
// then .len() if ipt.list_rules(IPTABLE_PREROUTING).ok().len() > 0 ||
191-
// ipt.list_rules(IPTABLE_MESH).ok().len() > 0 ||
192-
// ipt.list_rules(IPTABLE_STANDARD).ok().len() > 0 {
193-
// // either another agent is running or it failed to properly clean up
194-
// return Err("Dirty IP table detected in target. Either another mirrord agent is already running, or a previous agent failed to properly clean up after itself.".into())
195-
// }
196-
// panic here? inspect the stacktrace
197-
panic!();
189+
// FIXME: this is so ugly
190+
for name in [IPTABLE_PREROUTING, IPTABLE_MESH, IPTABLE_STANDARD] {
191+
if ipt
192+
.list_rules(name)
193+
.ok()
194+
.iter()
195+
.flatten()
196+
.collect::<Vec<_>>()
197+
.len()
198+
> 0
199+
{
200+
// either another agent is running or it failed to properly clean up
201+
return Err(
202+
"Dirty IP table detected in target. Either another mirrord agent is already \
203+
running, or a previous agent failed to properly clean up after itself."
204+
.to_string()
205+
.into(),
206+
);
207+
}
208+
}
198209

199210
let mut redirect = if let Some(vendor) = MeshVendor::detect(ipt.as_ref())? {
200211
match &vendor {

‎mirrord/agent/iptables/src/mesh.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -238,27 +238,27 @@ mod tests {
238238
create_mesh_list_values(&mut mock);
239239

240240
mock.expect_create_chain()
241-
.with(eq(IPTABLE_PREROUTING.as_str()))
241+
.with(eq(IPTABLE_PREROUTING))
242242
.times(1)
243243
.returning(|_| Ok(()));
244244

245245
mock.expect_insert_rule()
246246
.with(
247-
eq(IPTABLE_PREROUTING.as_str()),
247+
eq(IPTABLE_PREROUTING),
248248
eq("-m tcp -p tcp --dport 69 -j REDIRECT --to-ports 420"),
249249
eq(1),
250250
)
251251
.times(1)
252252
.returning(|_, _, _| Ok(()));
253253

254254
mock.expect_create_chain()
255-
.with(eq(IPTABLE_MESH.as_str()))
255+
.with(eq(IPTABLE_MESH))
256256
.times(1)
257257
.returning(|_| Ok(()));
258258

259259
mock.expect_insert_rule()
260260
.with(
261-
eq(IPTABLE_MESH.as_str()),
261+
eq(IPTABLE_MESH),
262262
eq(format!("-m owner --gid-owner {gid} -p tcp -j RETURN")),
263263
eq(1),
264264
)
@@ -267,20 +267,20 @@ mod tests {
267267

268268
mock.expect_insert_rule()
269269
.with(
270-
eq(IPTABLE_MESH.as_str()),
270+
eq(IPTABLE_MESH),
271271
eq("-o lo -m tcp -p tcp --dport 69 -j REDIRECT --to-ports 420"),
272272
eq(2),
273273
)
274274
.times(1)
275275
.returning(|_, _, _| Ok(()));
276276

277277
mock.expect_remove_chain()
278-
.with(eq(IPTABLE_PREROUTING.as_str()))
278+
.with(eq(IPTABLE_PREROUTING))
279279
.times(1)
280280
.returning(|_| Ok(()));
281281

282282
mock.expect_remove_chain()
283-
.with(eq(IPTABLE_MESH.as_str()))
283+
.with(eq(IPTABLE_MESH))
284284
.times(1)
285285
.returning(|_| Ok(()));
286286

‎mirrord/agent/iptables/src/prerouting.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -95,21 +95,21 @@ mod tests {
9595
let mut mock = MockIPTables::new();
9696

9797
mock.expect_create_chain()
98-
.with(eq(IPTABLE_PREROUTING.as_str()))
98+
.with(eq(IPTABLE_PREROUTING))
9999
.times(1)
100100
.returning(|_| Ok(()));
101101

102102
mock.expect_insert_rule()
103103
.with(
104-
eq(IPTABLE_PREROUTING.as_str()),
104+
eq(IPTABLE_PREROUTING),
105105
eq("-m tcp -p tcp --dport 69 -j REDIRECT --to-ports 420"),
106106
eq(1),
107107
)
108108
.times(1)
109109
.returning(|_, _, _| Ok(()));
110110

111111
mock.expect_remove_chain()
112-
.with(eq(IPTABLE_PREROUTING.as_str()))
112+
.with(eq(IPTABLE_PREROUTING))
113113
.times(1)
114114
.returning(|_| Ok(()));
115115

@@ -123,13 +123,13 @@ mod tests {
123123
let mut mock = MockIPTables::new();
124124

125125
mock.expect_create_chain()
126-
.with(eq(IPTABLE_PREROUTING.as_str()))
126+
.with(eq(IPTABLE_PREROUTING))
127127
.times(1)
128128
.returning(|_| Ok(()));
129129

130130
mock.expect_insert_rule()
131131
.with(
132-
eq(IPTABLE_PREROUTING.as_str()),
132+
eq(IPTABLE_PREROUTING),
133133
eq("-m tcp -p tcp --dport 69 -j REDIRECT --to-ports 420"),
134134
eq(1),
135135
)
@@ -138,15 +138,15 @@ mod tests {
138138

139139
mock.expect_insert_rule()
140140
.with(
141-
eq(IPTABLE_PREROUTING.as_str()),
141+
eq(IPTABLE_PREROUTING),
142142
eq("-m tcp -p tcp --dport 169 -j REDIRECT --to-ports 1420"),
143143
eq(2),
144144
)
145145
.times(1)
146146
.returning(|_, _, _| Ok(()));
147147

148148
mock.expect_remove_chain()
149-
.with(eq(IPTABLE_PREROUTING.as_str()))
149+
.with(eq(IPTABLE_PREROUTING))
150150
.times(1)
151151
.returning(|_| Ok(()));
152152

@@ -161,20 +161,20 @@ mod tests {
161161
let mut mock = MockIPTables::new();
162162

163163
mock.expect_create_chain()
164-
.with(eq(IPTABLE_PREROUTING.as_str()))
164+
.with(eq(IPTABLE_PREROUTING))
165165
.times(1)
166166
.returning(|_| Ok(()));
167167

168168
mock.expect_remove_rule()
169169
.with(
170-
eq(IPTABLE_PREROUTING.as_str()),
170+
eq(IPTABLE_PREROUTING),
171171
eq("-m tcp -p tcp --dport 69 -j REDIRECT --to-ports 420"),
172172
)
173173
.times(1)
174174
.returning(|_, _| Ok(()));
175175

176176
mock.expect_remove_chain()
177-
.with(eq(IPTABLE_PREROUTING.as_str()))
177+
.with(eq(IPTABLE_PREROUTING))
178178
.times(1)
179179
.returning(|_| Ok(()));
180180

0 commit comments

Comments
 (0)