Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 66 additions & 10 deletions src/commands/local/doctor.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::run_cmd_output;
use super::{run_cmd_output, MANAGED_BY_LABEL, PROVIDER_INSTALL_MANAGED_BY};
use std::error::Error;

/// `hops local doctor` — verify what `hops local start` set up on the current
Expand Down Expand Up @@ -124,7 +124,35 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) {
cond_detail("Healthy", &healthy),
);

// Drift detector: the Provider must point at its OWN per-provider DRC.
// A provider installed via `hops provider install` (e.g. a fork built from
// source) is labeled and uses the `<provider>-runtime` convention instead of
// the bootstrap `local-dev-<provider>` DRC. Recognize that so it reads as an
// intentional custom install, not drift — while still verifying cluster-admin.
let custom_install = provider_label(e.provider_name, MANAGED_BY_LABEL).as_deref()
== Some(PROVIDER_INSTALL_MANAGED_BY);
let (exp_drc, exp_binding, exp_sa) = if custom_install {
(
format!("{}-runtime", e.provider_name),
format!("{}-cluster-admin", e.provider_name),
e.provider_name.to_string(),
)
} else {
(e.drc.to_string(), e.binding.to_string(), e.sa.to_string())
};
d.check(
"install mode",
true,
if custom_install {
format!(
"custom install via `hops provider install` (package: {})",
provider_package(e.provider_name)
)
} else {
"bootstrap (hops local start)".to_string()
},
);

// The Provider must point at its expected per-provider DRC.
let rc = jsonpath(&[
"get",
"provider.pkg.crossplane.io",
Expand All @@ -133,7 +161,7 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) {
"jsonpath={.spec.runtimeConfigRef.name}",
])
.unwrap_or_default();
let rc_ok = rc == e.drc;
let rc_ok = rc == exp_drc;
d.check(
"runtimeConfigRef pinned to its own DRC",
rc_ok,
Expand All @@ -143,41 +171,41 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) {
format!(
"runtimeConfigRef is \"{}\" (expected \"{}\") — drifted; provider lacks its cluster-admin ServiceAccount and cannot observe XRs",
if rc.is_empty() { "<unset>" } else { &rc },
e.drc
exp_drc
)
},
);

let drc_ok = exists(&["get", "deploymentruntimeconfig", e.drc]);
let drc_ok = exists(&["get", "deploymentruntimeconfig", exp_drc.as_str()]);
d.check(
"DeploymentRuntimeConfig present",
drc_ok,
if drc_ok {
String::new()
} else {
format!("DeploymentRuntimeConfig \"{}\" missing", e.drc)
format!("DeploymentRuntimeConfig \"{}\" missing", exp_drc)
},
);

// cluster-admin ClusterRoleBinding bound to the pinned SA.
let role = jsonpath(&[
"get",
"clusterrolebinding",
e.binding,
exp_binding.as_str(),
"-o",
"jsonpath={.roleRef.name}",
]);
let subjects = jsonpath(&[
"get",
"clusterrolebinding",
e.binding,
exp_binding.as_str(),
"-o",
"jsonpath={.subjects[?(@.kind==\"ServiceAccount\")].name}",
]);
let binding_ok = role.as_deref() == Some("cluster-admin")
&& subjects
.as_deref()
.map(|s| s.split_whitespace().any(|n| n == e.sa))
.map(|s| s.split_whitespace().any(|n| n == exp_sa.as_str()))
.unwrap_or(false);
d.check(
"cluster-admin binding -> pinned SA",
Expand All @@ -187,7 +215,7 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) {
} else {
format!(
"ClusterRoleBinding \"{}\" missing or not binding cluster-admin to ServiceAccount \"{}\"",
e.binding, e.sa
exp_binding, exp_sa
)
},
);
Expand Down Expand Up @@ -229,6 +257,34 @@ fn cond_detail(cond: &str, status: &str) -> String {
}
}

/// Read a metadata label off a Provider via `-o json` (robust against the dotted
/// label key that jsonpath escaping mishandles).
fn provider_label(provider: &str, key: &str) -> Option<String> {
let json = run_cmd_output(
"kubectl",
&["get", "provider.pkg.crossplane.io", provider, "-o", "json"],
)
.ok()?;
let v: serde_json::Value = serde_json::from_str(&json).ok()?;
v.get("metadata")?
.get("labels")?
.get(key)?
.as_str()
.map(|s| s.to_string())
}

/// The Provider's spec.package (image ref) — shown for custom installs.
fn provider_package(provider: &str) -> String {
jsonpath(&[
"get",
"provider.pkg.crossplane.io",
provider,
"-o",
"jsonpath={.spec.package}",
])
.unwrap_or_default()
}

/// True when `kubectl get <args>` finds the resource. Uses `--ignore-not-found`
/// so a missing resource is `false` rather than an error.
fn exists(get_args: &[&str]) -> bool {
Expand Down
8 changes: 8 additions & 0 deletions src/commands/local/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ use std::process::{Command, Stdio};
const LOCAL_STATE_DIR: &str = ".hops/local";
const REPO_CACHE_DIR: &str = "repo-cache";

/// metadata.labels key recording who manages a resource (k8s recommended label).
pub const MANAGED_BY_LABEL: &str = "app.kubernetes.io/managed-by";
/// `MANAGED_BY_LABEL` value that `hops provider install` stamps on the Provider,
/// DeploymentRuntimeConfig, and ClusterRoleBinding it manages — so `hops local
/// doctor` can tell an intentional custom install (e.g. a forked provider built
/// from source) apart from accidental drift.
pub const PROVIDER_INSTALL_MANAGED_BY: &str = "hops-provider-install";

/// Env var checked by kubectl helpers to inject `--context <name>`.
pub const HOPS_KUBE_CONTEXT_ENV: &str = "HOPS_KUBE_CONTEXT";

Expand Down
11 changes: 10 additions & 1 deletion src/commands/provider/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::commands::local::package_install::{
};
use crate::commands::local::{
kubectl_apply_stdin, run_cmd, run_cmd_output, sync_registry_hosts_entry,
HOPS_KUBE_CONTEXT_ENV,
HOPS_KUBE_CONTEXT_ENV, MANAGED_BY_LABEL, PROVIDER_INSTALL_MANAGED_BY,
};
use clap::Args;
use serde::Deserialize;
Expand Down Expand Up @@ -1002,6 +1002,8 @@ fn build_provider_yaml(
kind: Provider
metadata:
name: {name}
labels:
{MANAGED_BY_LABEL}: {PROVIDER_INSTALL_MANAGED_BY}
spec:
package: {package_ref}
packagePullPolicy: Always
Expand All @@ -1028,6 +1030,8 @@ fn build_runtime_config_yaml(
kind: DeploymentRuntimeConfig
metadata:
name: {name}
labels:
{MANAGED_BY_LABEL}: {PROVIDER_INSTALL_MANAGED_BY}
spec:
serviceAccountTemplate:
metadata:
Expand Down Expand Up @@ -1056,6 +1060,8 @@ fn build_cluster_role_binding_yaml(name: &str, service_account: &str) -> String
kind: ClusterRoleBinding
metadata:
name: {name}
labels:
{MANAGED_BY_LABEL}: {PROVIDER_INSTALL_MANAGED_BY}
subjects:
- kind: ServiceAccount
name: {service_account}
Expand Down Expand Up @@ -1084,6 +1090,7 @@ mod tests {
assert!(yaml.contains("name: provider-helm"));
assert!(yaml.contains("package: registry.example/provider-helm:dev-abc"));
assert!(yaml.contains("name: provider-helm-runtime"));
assert!(yaml.contains("app.kubernetes.io/managed-by: hops-provider-install"));
assert!(!yaml.contains("skipDependencyResolution"));
}

Expand All @@ -1107,6 +1114,7 @@ mod tests {
assert!(!without.contains("package-runtime"));
assert!(!without.contains("deploymentTemplate"));
assert!(without.contains("serviceAccountTemplate"));
assert!(without.contains("app.kubernetes.io/managed-by: hops-provider-install"));
}

#[test]
Expand All @@ -1116,6 +1124,7 @@ mod tests {
assert!(yaml.contains("name: p-cluster-admin"));
assert!(yaml.contains("name: p\n namespace: crossplane-system"));
assert!(yaml.contains("name: cluster-admin"));
assert!(yaml.contains("app.kubernetes.io/managed-by: hops-provider-install"));
}

const TEST_LOCAL_REGISTRY: &str = "registry.crossplane-system.svc.cluster.local:5000";
Expand Down