Skip to content

Commit 6b1cde7

Browse files
committed
feat(oabctl): wire k8s secret refs into K8sDriver (slice 3d)
spec.secrets values for a k8s-runtime manifest must now be k8s-secret://<secret-name>#<key> — parsed in secrets.rs (pure, no API call: unlike aws-sm://, a k8s Secret is referenced by name+key directly, kubelet resolves it at pod-start time) and wired into build_deployment as env[].valueFrom.secretKeyRef. Any other scheme (an ECS aws-sm:// ref or raw ARN left over from copy-pasting an ECS manifest) fails loudly at apply time instead of silently mis-deploying — same "declare the contract, fail closed on mismatch" shape the rest of this driver already uses. The Secret object itself must already exist in the target namespace — creating it is a separate concern, deliberately out of scope here (mirrors aws-sm://, which likewise only *references* a secret Secrets Manager already holds, never creates one). reject_unsupported() now only guards spec.bundleFrom (3c, still open) — secrets are no longer in that list. 6 new tests (2 in secrets.rs for the parser, 4 in k8s_driver.rs), 98/98 total green, clippy -D warnings clean. Ref: studio#97 (K8s driver — ADR #63 slice 3, sub-slice tracking)
1 parent eb8e3cd commit 6b1cde7

2 files changed

Lines changed: 132 additions & 23 deletions

File tree

crates/oabctl/src/k8s_driver.rs

Lines changed: 80 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
//! K8s implementation of `ProvisionDriver` (ADR #63 slice 3b).
22
//!
33
//! Deliberately narrow for this slice: `apply`/`scale`/`delete` against a k8s
4-
//! `Deployment`, mirroring the shape `EcsDriver` already has. Two things a
5-
//! manifest can carry are explicitly **not yet supported** and fail loudly
6-
//! rather than silently mis-deploying:
4+
//! `Deployment`, mirroring the shape `EcsDriver` already has.
75
//!
8-
//! - `spec.bundleFrom` (the composed persona/skills bundle) — ECS gets this
9-
//! for free via its S3 file carrier; k8s needs a ConfigMap/volume carrier,
10-
//! tracked as sub-slice 3c.
11-
//! - `spec.secrets` — ECS resolves these into `Secret.valueFrom` ARNs; a k8s
12-
//! target needs a different output shape (a Secret key selector), tracked
13-
//! as sub-slice 3d.
6+
//! `spec.secrets` is wired (sub-slice 3d): each value must be
7+
//! `k8s-secret://<secret-name>#<key>` (see `secrets::parse_k8s_secret_uri`)
8+
//! and becomes an `env[].valueFrom.secretKeyRef` — the Secret object itself
9+
//! must already exist in the target namespace; creating it is a separate
10+
//! concern (same non-creating shape `aws-sm://` already has for ECS). ECS's
11+
//! `aws-sm://`/raw-ARN values in a k8s-runtime manifest fail loudly at apply
12+
//! time — a manifest error, not a silent no-op.
13+
//!
14+
//! `spec.bundleFrom` (the composed persona/skills bundle) is explicitly
15+
//! **not yet supported** and fails loudly rather than silently
16+
//! mis-deploying — ECS gets this for free via its S3 file carrier, k8s needs
17+
//! a ConfigMap/volume carrier, tracked as sub-slice 3c.
1418
//!
1519
//! Observing k8s state into the canonical 6-state (the `apply`/`scale`
1620
//! counterpart to `status.rs`'s ECS `service_status`/`instance_status`) is
@@ -26,7 +30,8 @@ use anyhow::{Context, Result};
2630
use async_trait::async_trait;
2731
use k8s_openapi::api::apps::v1::{Deployment, DeploymentSpec};
2832
use k8s_openapi::api::core::v1::{
29-
Container, EnvVar, PodSpec, PodTemplateSpec, ResourceRequirements, Toleration,
33+
Container, EnvVar, EnvVarSource, PodSpec, PodTemplateSpec, ResourceRequirements,
34+
SecretKeySelector, Toleration,
3035
};
3136
use k8s_openapi::apimachinery::pkg::api::resource::Quantity;
3237
use k8s_openapi::apimachinery::pkg::apis::meta::v1::{LabelSelector, ObjectMeta};
@@ -83,8 +88,8 @@ fn require_kubernetes_runtime(m: &OABServiceManifest) -> Result<&crate::manifest
8388
}
8489
}
8590

86-
/// Reject the two not-yet-supported manifest features explicitly (see module
87-
/// docs) instead of silently dropping them.
91+
/// Reject the still-not-yet-supported manifest feature explicitly (see
92+
/// module docs) instead of silently dropping it.
8893
fn reject_unsupported(m: &OABServiceManifest) -> Result<()> {
8994
if m.spec.bundle_from.is_some() {
9095
anyhow::bail!(
@@ -93,16 +98,44 @@ fn reject_unsupported(m: &OABServiceManifest) -> Result<()> {
9398
m.metadata.name
9499
);
95100
}
96-
if !m.spec.secrets.is_empty() {
97-
anyhow::bail!(
98-
"k8s secret refs not implemented yet (studio#97 sub-slice 3d) — '{}/{}' has spec.secrets set",
99-
m.metadata.namespace,
100-
m.metadata.name
101-
);
102-
}
103101
Ok(())
104102
}
105103

104+
/// Build the `env[]` entries for `spec.secrets`: each value must be a
105+
/// `k8s-secret://<secret-name>#<key>` ref, which becomes a `secretKeyRef` —
106+
/// kubelet resolves it at pod-start time, no API call needed here (unlike
107+
/// ECS's `aws-sm://`, which resolves to an ARN up front). Any other scheme
108+
/// (an ECS `aws-sm://` ref left over from copy-pasting an ECS manifest, a
109+
/// raw ARN, ...) is a manifest error, not silently dropped.
110+
fn secret_env_vars(m: &OABServiceManifest) -> Result<Vec<EnvVar>> {
111+
m.spec
112+
.secrets
113+
.iter()
114+
.map(|(env_name, value)| {
115+
let (secret_name, key) = crate::secrets::parse_k8s_secret_uri(value)
116+
.with_context(|| {
117+
format!(
118+
"spec.secrets['{env_name}'] for k8s runtime must use \
119+
k8s-secret://<secret-name>#<key> (got '{value}') — '{}/{}'",
120+
m.metadata.namespace, m.metadata.name
121+
)
122+
})??;
123+
Ok(EnvVar {
124+
name: env_name.clone(),
125+
value_from: Some(EnvVarSource {
126+
secret_key_ref: Some(SecretKeySelector {
127+
name: secret_name.to_string(),
128+
key: key.to_string(),
129+
optional: None,
130+
}),
131+
..Default::default()
132+
}),
133+
..Default::default()
134+
})
135+
})
136+
.collect()
137+
}
138+
106139
fn resource_requirements(resources: &crate::manifest::Resources) -> ResourceRequirements {
107140
let mut quantities = BTreeMap::new();
108141
quantities.insert("cpu".to_string(), Quantity(resources.cpu.clone()));
@@ -141,6 +174,7 @@ fn build_deployment(m: &OABServiceManifest) -> Result<Deployment> {
141174
..Default::default()
142175
});
143176
}
177+
env.extend(secret_env_vars(m)?);
144178

145179
// Same convention as EcsDriver (apply.rs): the image's default CMD points
146180
// at a config.toml nothing populates, so override it to load configFrom
@@ -338,10 +372,34 @@ mod tests {
338372
}
339373

340374
#[test]
341-
fn reject_unsupported_bails_on_secrets() {
375+
fn reject_unsupported_passes_manifests_with_k8s_secrets() {
376+
let m = k8s_manifest(None, &[("DISCORD_BOT_TOKEN", "k8s-secret://oab-orca#DISCORD_BOT_TOKEN")]);
377+
reject_unsupported(&m).unwrap();
378+
}
379+
380+
#[test]
381+
fn build_deployment_wires_secret_key_ref() {
382+
let m = k8s_manifest(None, &[("DISCORD_BOT_TOKEN", "k8s-secret://oab-orca#DISCORD_BOT_TOKEN")]);
383+
let dep = build_deployment(&m).unwrap();
384+
let pod = dep.spec.unwrap().template.spec.unwrap();
385+
let env = pod.containers[0].env.as_ref().unwrap();
386+
let secret_env = env.iter().find(|e| e.name == "DISCORD_BOT_TOKEN").unwrap();
387+
let secret_ref = secret_env
388+
.value_from
389+
.as_ref()
390+
.unwrap()
391+
.secret_key_ref
392+
.as_ref()
393+
.unwrap();
394+
assert_eq!(secret_ref.name, "oab-orca");
395+
assert_eq!(secret_ref.key, "DISCORD_BOT_TOKEN");
396+
}
397+
398+
#[test]
399+
fn build_deployment_rejects_non_k8s_secret_scheme() {
342400
let m = k8s_manifest(None, &[("DISCORD_BOT_TOKEN", "aws-sm://oab/prod/orca#DISCORD_BOT_TOKEN")]);
343-
let err = reject_unsupported(&m).unwrap_err();
344-
assert!(err.to_string().contains("3d"));
401+
let err = build_deployment(&m).unwrap_err();
402+
assert!(err.to_string().contains("k8s-secret://"));
345403
}
346404

347405
#[test]

crates/oabctl/src/secrets.rs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Shared resolution for `spec.secrets` values.
22
//!
3-
//! Values can be either a Secrets Manager reference in ECS-native
3+
//! For ECS: values can be either a Secrets Manager reference in ECS-native
44
//! `valueFrom` format directly (a full ARN, optionally suffixed with
55
//! `:<jsonKey>::` to extract one field of a JSON secret — an ECS-only
66
//! convention; the Secrets Manager API itself has no knowledge of it), or
@@ -9,6 +9,12 @@
99
//! identical here so a manifest author can write one convention across both
1010
//! `spec.secrets` (consumed by ECS at container launch) and `config.toml`
1111
//! (consumed by openab itself at runtime).
12+
//!
13+
//! For k8s: `k8s-secret://<secret-name>#<key>` references a k8s Secret
14+
//! object's key directly (see [`parse_k8s_secret_uri`]) — no ARN-resolution
15+
//! equivalent needed, since kubelet resolves name+key at pod-start time.
16+
//! `k8s_driver::build_deployment` is the consumer; `aws-sm://` values in a
17+
//! k8s-runtime manifest are a manifest error, not silently ignored.
1218
1319
use anyhow::{Context, Result};
1420

@@ -26,6 +32,25 @@ fn parse_aws_sm_uri(value: &str) -> Option<Result<(&str, &str)>> {
2632
})
2733
}
2834

35+
/// Parse `k8s-secret://<secret-name>#<key>` into `(secret_name, key)`. Unlike
36+
/// `aws-sm://`, there is no resolution step — a k8s Secret is referenced by
37+
/// name+key directly (kubelet resolves it at pod-start time), so this is pure
38+
/// parsing, no API call. The Secret object itself must already exist in the
39+
/// target namespace; creating it is a separate concern (mirrors `aws-sm://`,
40+
/// which likewise only *references* a secret Secrets Manager already holds).
41+
/// Returns `None` if `value` doesn't use the `k8s-secret://` scheme.
42+
pub(crate) fn parse_k8s_secret_uri(value: &str) -> Option<Result<(&str, &str)>> {
43+
let rest = value.strip_prefix("k8s-secret://")?;
44+
Some(match rest.rsplit_once('#') {
45+
Some((secret_name, key)) if !secret_name.is_empty() && !key.is_empty() => {
46+
Ok((secret_name, key))
47+
}
48+
_ => Err(anyhow::anyhow!(
49+
"invalid k8s-secret:// secret ref '{value}' — expected k8s-secret://<secret-name>#<key>"
50+
)),
51+
})
52+
}
53+
2954
/// Resolve a `spec.secrets` value into the ECS-native `valueFrom` format ECS
3055
/// actually requires. ECS's `valueFrom` requires the *full* ARN (not just a
3156
/// secret name) whenever a JSON-key suffix is present, so an `aws-sm://`
@@ -256,4 +281,30 @@ mod tests {
256281
assert!(parse_aws_sm_uri("arn:aws:secretsmanager:us-east-1:123:secret:oab/x-AbCdEf").is_none());
257282
assert!(parse_aws_sm_uri("plain-secret-name").is_none());
258283
}
284+
285+
#[test]
286+
fn parse_k8s_secret_uri_extracts_name_and_key() {
287+
let (name, key) = parse_k8s_secret_uri("k8s-secret://oab-orca#DISCORD_BOT_TOKEN")
288+
.unwrap()
289+
.unwrap();
290+
assert_eq!(name, "oab-orca");
291+
assert_eq!(key, "DISCORD_BOT_TOKEN");
292+
}
293+
294+
#[test]
295+
fn parse_k8s_secret_uri_rejects_missing_hash() {
296+
assert!(parse_k8s_secret_uri("k8s-secret://oab-orca").unwrap().is_err());
297+
}
298+
299+
#[test]
300+
fn parse_k8s_secret_uri_rejects_empty_parts() {
301+
assert!(parse_k8s_secret_uri("k8s-secret://#key").unwrap().is_err());
302+
assert!(parse_k8s_secret_uri("k8s-secret://oab-orca#").unwrap().is_err());
303+
}
304+
305+
#[test]
306+
fn parse_k8s_secret_uri_returns_none_for_other_schemes() {
307+
assert!(parse_k8s_secret_uri("aws-sm://oab/telegram/pahudxbot#TOKEN").is_none());
308+
assert!(parse_k8s_secret_uri("plain-secret-name").is_none());
309+
}
259310
}

0 commit comments

Comments
 (0)