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
1 change: 0 additions & 1 deletion deploy/k8s-dev/policybinding-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ metadata:
spec:
group: sts.rustfs.com
names:
categories: []
kind: PolicyBinding
plural: policybindings
shortNames:
Expand Down
137 changes: 130 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,56 @@ mod controller_watch_tests {
use crate::types::v1alpha1::tenant::RpcSecretRef;
use futures::{TryStreamExt, stream};
use k8s_openapi::apimachinery::pkg::apis::meta::v1 as metav1;
use serde::Deserialize;
use std::collections::{BTreeMap, BTreeSet};
use std::fs;
use std::path::{Path, PathBuf};
use std::time::{SystemTime, UNIX_EPOCH};

fn parse_crds(yaml: &str) -> Vec<CustomResourceDefinition> {
serde_yaml_ng::Deserializer::from_str(yaml)
.map(|document| {
CustomResourceDefinition::deserialize(document)
.expect("CRD document should deserialize")
})
.collect()
}

fn is_helm_manifest_path(path: &Path) -> bool {
path.extension()
.and_then(|extension| extension.to_str())
.is_some_and(|extension| {
["yaml", "yml", "json"]
.iter()
.any(|expected| extension.eq_ignore_ascii_case(expected))
Comment on lines +1055 to +1057

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match Helm's case-sensitive manifest extensions

When a file under crds/ uses an uppercase suffix—as the new .YML and .JSON fixtures do—this helper selects it, but Helm's hasManifestExtension accepts only the exact lowercase extensions .yaml, .yml, and .json. The validation therefore does not actually mirror Helm: it can parse and count an artifact that Helm will skip during CRD installation, or fail on an uppercase auxiliary file Helm ignores. Use the same case-sensitive extension comparison as Helm.

Useful? React with 👍 / 👎.

})
}

fn chart_crd_manifest_paths(crd_dir: &Path) -> Vec<PathBuf> {
let mut directories = vec![crd_dir.to_path_buf()];
let mut manifests = Vec::new();

while let Some(directory) = directories.pop() {
for entry in fs::read_dir(&directory)
.expect("chart CRD directory should be readable")
.collect::<Result<Vec<_>, _>>()
.expect("chart CRD directory entries should be readable")
{
let path = entry.path();
let file_type = entry
.file_type()
.expect("chart CRD entry type should be readable");
if file_type.is_dir() {
directories.push(path);
} else if file_type.is_file() && is_helm_manifest_path(&path) {
manifests.push(path);
}
}
}

manifests.sort();
manifests
}

#[test]
fn cert_manager_certificate_api_resource_is_stable() {
Expand All @@ -1050,12 +1099,13 @@ mod controller_watch_tests {
fn rendered_crds_omit_empty_categories() {
let yaml = render_crds_yaml().expect("CRDs render");

// The API server prunes empty arrays, so emitting `categories: []` leaves
// GitOps tooling diffing a desired [] against an absent field forever.
assert!(
!yaml.contains("categories:"),
"rendered CRDs must not carry an empty categories list:\n{yaml}"
);
for crd in parse_crds(&yaml) {
let name = crd.metadata.name.as_deref().unwrap_or("<unnamed>");
assert!(
crd.spec.names.categories.is_none(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify that categories is absent from the serialized YAML

If rendering regresses to emitting spec.names.categories: null, deserializing into the optional categories field produces None, so this assertion passes even though the key was not omitted. That allows regenerated packaged artifacts to violate the exact omission this regression test is intended to enforce; inspect the YAML mapping for key presence rather than relying only on the deserialized Option.

Useful? React with 👍 / 👎.

"rendered CRD {name} must omit an empty spec.names.categories list"
);
}
}

#[test]
Expand Down Expand Up @@ -1673,7 +1723,7 @@ mod controller_watch_tests {
}

#[test]
fn tracked_tenant_crds_match_generated_schema() {
fn tracked_crds_match_generated_schema() {
let yaml = render_crds_yaml().expect("CRDs render to YAML");
let (tenant, policy_binding) = yaml
.split_once("---\n")
Expand All @@ -1687,6 +1737,79 @@ mod controller_watch_tests {
policy_binding,
include_str!("../deploy/rustfs-operator/crds/policybinding-crd.yaml")
);
assert_eq!(
policy_binding,
include_str!("../deploy/k8s-dev/policybinding-crd.yaml")
);
}

#[test]
fn chart_crd_names_match_generated_crds_without_duplicates() {
let generated_names = parse_crds(&render_crds_yaml().expect("CRDs render to YAML"))
.into_iter()
.map(|crd| crd.metadata.name.expect("generated CRD should have a name"))
.collect::<BTreeSet<_>>();
let crd_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("deploy/rustfs-operator/crds");

let mut tracked_names = BTreeMap::new();
for path in chart_crd_manifest_paths(&crd_dir) {
let yaml = fs::read_to_string(&path).expect("tracked CRD should be readable");
for crd in parse_crds(&yaml) {
let name = crd.metadata.name.expect("tracked CRD should have a name");
assert!(
tracked_names.insert(name.clone(), path.clone()).is_none(),
"chart contains duplicate CRD {name}"
);
}
}

assert_eq!(
tracked_names.into_keys().collect::<BTreeSet<_>>(),
generated_names,
"chart CRD names must match the generated CRDs"
);
}

#[test]
fn chart_crd_manifest_paths_match_helm_file_selection() {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("system time should follow the Unix epoch")
.as_nanos();
let crd_dir = std::env::temp_dir().join(format!(
"rustfs-operator-crd-manifests-{}-{unique}",
std::process::id()
));
fs::create_dir_all(crd_dir.join("nested"))
.expect("nested test CRD directory should be created");
for relative_path in [
"tenant.yaml",
"policybinding.YML",
"nested/duplicate.JSON",
"README.md",
] {
fs::write(crd_dir.join(relative_path), [])
.expect("test CRD manifest should be created");
}

let paths = chart_crd_manifest_paths(&crd_dir)
.into_iter()
.map(|path| {
path.strip_prefix(&crd_dir)
.expect("test manifest should be under the CRD directory")
.to_path_buf()
})
.collect::<Vec<_>>();

assert_eq!(
paths,
vec![
PathBuf::from("nested/duplicate.JSON"),
PathBuf::from("policybinding.YML"),
PathBuf::from("tenant.yaml"),
]
);
fs::remove_dir_all(&crd_dir).expect("test CRD directory should be removed");
}

fn tenant_owner_ref(name: &str) -> metav1::OwnerReference {
Expand Down
Loading