Skip to content

Commit 54361c4

Browse files
committed
Put service chaining hostreq on component instead of app
Signed-off-by: itowlson <[email protected]>
1 parent 4223521 commit 54361c4

File tree

7 files changed

+66
-31
lines changed

7 files changed

+66
-31
lines changed

Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/app/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,12 @@ impl App {
169169
/// Checks that the application does not have any host requirements
170170
/// outside the supported set. The error case returns a comma-separated
171171
/// list of unmet requirements.
172-
pub fn ensure_needs_only(&self, supported: &[&str]) -> std::result::Result<(), String> {
173-
self.locked.ensure_needs_only(supported)
172+
pub fn ensure_needs_only(
173+
&self,
174+
trigger_type: &str,
175+
supported: &[&str],
176+
) -> std::result::Result<(), String> {
177+
self.locked.ensure_needs_only(trigger_type, supported)
174178
}
175179

176180
/// Scrubs the locked app to only contain the given list of components

crates/loader/src/local.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,6 @@ impl LocalLoader {
8484

8585
let metadata = locked_metadata(application, triggers.keys().cloned())?;
8686

87-
let app_requires_service_chaining = components.values().any(requires_service_chaining);
88-
8987
let variables = variables
9088
.into_iter()
9189
.map(|(name, v)| Ok((name.to_string(), locked_variable(v)?)))
@@ -111,19 +109,16 @@ impl LocalLoader {
111109
}))
112110
.await?;
113111

114-
let mut host_requirements = ValuesMapBuilder::new();
115-
if app_requires_service_chaining {
116-
host_requirements.string(
117-
spin_locked_app::locked::SERVICE_CHAINING_KEY,
118-
spin_locked_app::locked::HOST_REQ_REQUIRED,
119-
);
120-
}
121-
let host_requirements = host_requirements.build();
112+
let host_requirements = spin_locked_app::values::ValuesMap::new();
122113

123114
let mut must_understand = vec![];
124115
if !host_requirements.is_empty() {
125116
must_understand.push(spin_locked_app::locked::MustUnderstand::HostRequirements);
126117
}
118+
if components.iter().any(|c| !c.host_requirements.is_empty()) {
119+
must_understand
120+
.push(spin_locked_app::locked::MustUnderstand::ComponentHostRequirements);
121+
}
127122

128123
drop(sloth_guard);
129124

@@ -150,6 +145,8 @@ impl LocalLoader {
150145
spin_factor_outbound_networking::AllowedHostsConfig::validate(&allowed_outbound_hosts)
151146
.context("`allowed_outbound_hosts` is malformed")?;
152147

148+
let component_requires_service_chaining = requires_service_chaining(&component);
149+
153150
let metadata = ValuesMapBuilder::new()
154151
.string("description", component.description)
155152
.string_array("allowed_outbound_hosts", allowed_outbound_hosts)
@@ -213,6 +210,15 @@ impl LocalLoader {
213210
.map(|(k, v)| (k.into(), v))
214211
.collect();
215212

213+
let mut host_requirements = ValuesMapBuilder::new();
214+
if component_requires_service_chaining {
215+
host_requirements.string(
216+
spin_locked_app::locked::SERVICE_CHAINING_KEY,
217+
spin_locked_app::locked::HOST_REQ_REQUIRED,
218+
);
219+
}
220+
let host_requirements = host_requirements.build();
221+
216222
Ok(LockedComponent {
217223
id: id.as_ref().into(),
218224
metadata,
@@ -221,6 +227,7 @@ impl LocalLoader {
221227
files,
222228
config,
223229
dependencies,
230+
host_requirements,
224231
})
225232
}
226233

crates/loader/tests/ui/service-chaining.lock

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"spin_lock_version": 1,
33
"must_understand": [
4-
"host_requirements"
4+
"component_host_requirements"
55
],
66
"metadata": {
77
"authors": [
@@ -18,9 +18,6 @@
1818
"triggers": {},
1919
"version": "6.11.2"
2020
},
21-
"host_requirements": {
22-
"local_service_chaining": "required"
23-
},
2421
"triggers": [
2522
{
2623
"id": "four-lights-http-trigger",
@@ -65,6 +62,9 @@
6562
"env": {
6663
"env1": "first",
6764
"env2": "second"
65+
},
66+
"host_requirements": {
67+
"local_service_chaining": "required"
6868
}
6969
},
7070
{

crates/locked-app/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ edition = { workspace = true }
77
[dependencies]
88
anyhow = { workspace = true }
99
async-trait = { workspace = true }
10+
itertools = { workspace = true }
1011
serde = { workspace = true }
1112
serde_json = { workspace = true }
1213
spin-serde = { path = "../serde" }

crates/locked-app/src/locked.rs

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
//! Spin lock file (spin.lock) serialization models.
22
3-
use std::path::PathBuf;
3+
use std::{collections::HashSet, path::PathBuf};
44

5+
use itertools::Itertools;
56
use serde::{Deserialize, Serialize};
67
use serde_json::Value;
78
use spin_serde::{DependencyName, FixedVersionBackwardCompatible};
@@ -25,21 +26,19 @@ pub const HOST_REQ_OPTIONAL: &str = "optional";
2526
/// Indicates that a host feature is required.
2627
pub const HOST_REQ_REQUIRED: &str = "required";
2728

29+
// TODO: it turns out that using an enum for this results in bad
30+
// errors by non-understanders (unknown variant rather than "I'm sorry
31+
// Dave I can't do that")
2832
/// Identifies fields in the LockedApp that the host must process if present.
2933
#[derive(Clone, Debug, Serialize, Deserialize)]
3034
#[serde(rename_all = "snake_case")]
3135
pub enum MustUnderstand {
3236
/// If present in `must_understand`, the host must support all features
3337
/// in the app's `host_requirements` section.
3438
HostRequirements,
35-
}
36-
37-
/// Features or capabilities the application requires the host to support.
38-
#[derive(Clone, Debug, Serialize, Deserialize)]
39-
#[serde(rename_all = "snake_case")]
40-
pub enum HostRequirement {
41-
/// The application requires local service chaining.
42-
LocalServiceChaining,
39+
/// If present in `must_understand`, the host must support all features
40+
/// in components' `host_requirements` section.
41+
ComponentHostRequirements,
4342
}
4443

4544
/// A LockedApp represents a "fully resolved" Spin application.
@@ -187,10 +186,26 @@ impl LockedApp {
187186
/// Checks that the application does not have any host requirements
188187
/// outside the supported set. The error case returns a comma-separated
189188
/// list of unmet requirements.
190-
pub fn ensure_needs_only(&self, supported: &[&str]) -> Result<(), String> {
191-
let unmet_requirements = self
192-
.host_requirements
193-
.keys()
189+
pub fn ensure_needs_only(&self, trigger_type: &str, supported: &[&str]) -> Result<(), String> {
190+
let app_host_requirements = self.host_requirements.keys();
191+
192+
let component_ids = self
193+
.triggers
194+
.iter()
195+
.filter(|t| t.trigger_type == trigger_type)
196+
.flat_map(|t| t.trigger_config.get("component"))
197+
.filter_map(|v| v.as_str())
198+
.collect::<HashSet<_>>();
199+
let components = self
200+
.components
201+
.iter()
202+
.filter(|c| component_ids.contains(c.id.as_str()));
203+
let component_host_requirements = components.flat_map(|c| c.host_requirements.keys());
204+
205+
let all_host_requirements = app_host_requirements.chain(component_host_requirements);
206+
207+
let unmet_requirements = all_host_requirements
208+
.unique()
194209
.filter(|hr| !supported.contains(&hr.as_str()))
195210
.map(|s| s.to_string())
196211
.collect::<Vec<_>>();
@@ -225,6 +240,13 @@ pub struct LockedComponent {
225240
/// Component dependencies
226241
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
227242
pub dependencies: BTreeMap<DependencyName, LockedComponentDependency>,
243+
/// Host requirements
244+
#[serde(
245+
default,
246+
skip_serializing_if = "ValuesMap::is_empty",
247+
deserialize_with = "deserialize_host_requirements"
248+
)]
249+
pub host_requirements: ValuesMap,
228250
}
229251

230252
/// A LockedDependency represents a "fully resolved" Spin component dependency.

crates/trigger/src/cli.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ impl<T: Trigger<B::Factors>, B: RuntimeFactorsBuilder> FactorsTriggerCommand<T,
182182
};
183183

184184
// Validate required host features
185-
if let Err(unmet) = app.ensure_needs_only(&T::supported_host_requirements()) {
185+
if let Err(unmet) = app.ensure_needs_only(T::TYPE, &T::supported_host_requirements()) {
186186
anyhow::bail!("This application requires the following features that are not available in this version of the '{}' trigger: {unmet}", T::TYPE);
187187
}
188188

0 commit comments

Comments
 (0)