From 2d37657391bf751fdc05e7c7d0313673fbc0c5ff Mon Sep 17 00:00:00 2001 From: Tamal Saha Date: Mon, 13 Jul 2026 12:48:25 +0600 Subject: [PATCH] Make CRD dedup winner deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit crd-only deduplicates CRDs by GroupKind, keeping the first copy seen. The winner was decided by the order Helm's CRDObjects() enumerates charts, which is not stable — it varies with how subcharts were loaded (directory vs .tgz, freshly pulled OCI deps, Helm version). #4 only sorted the output file order, not which duplicate wins, so the generated crds-only chart still flapped between conflicting copies of the same CRD. Collect every candidate with its source chart path and a content digest, then pick the winner by a total order: group, kind, chart depth (the parent chart wins over its subcharts), chart path, filename, and finally content digest. The digest tiebreak covers the case where one subchart is present as both a directory and a .tgz (or reached via a diamond dependency) with differing content and therefore an otherwise identical (group, kind, path, filename) key. Warn only when a dropped copy actually differs from the winner. Signed-off-by: Tamal Saha --- pkg/cmds/crdonly.go | 126 +++++++++++++++++++++++++++++--------------- 1 file changed, 84 insertions(+), 42 deletions(-) diff --git a/pkg/cmds/crdonly.go b/pkg/cmds/crdonly.go index ac9c1c7..4ff4247 100644 --- a/pkg/cmds/crdonly.go +++ b/pkg/cmds/crdonly.go @@ -17,6 +17,8 @@ limitations under the License. package cmds import ( + "crypto/sha256" + "encoding/hex" "fmt" "os" "sort" @@ -52,35 +54,76 @@ func NewCmdGenerateCRDOnlyChart() *cobra.Command { } newChartName := ch.Metadata.Name + "-certified-crds" - // Map to store unique CRDs: key is (group, kind, plural), value is the chart.File and source chart name - crdMap := make(map[schema.GroupKind]*chart.File) - sourceMap := make(map[schema.GroupKind]string) // for warning messages - - // First: collect CRDs from the main (parent) chart — these take precedence - collectCRDs(ch, ch.Name(), crdMap, sourceMap) - - // Then: collect from all dependencies (subcharts) - for _, dep := range ch.Dependencies() { - if dep != nil { - collectCRDs(dep, dep.Name(), crdMap, sourceMap) + // Collect every CRD in the chart tree. The parent chart's CRDObjects() + // already recurses into all subcharts, so a single call captures them + // all. Each candidate remembers its source chart so that, when the same + // GroupKind is provided by more than one chart, the winner can be picked + // deterministically below. + var candidates []crdCandidate + for _, c := range ch.CRDObjects() { + key, err := extractCRDKey(c.File.Data) + if err != nil { + fmt.Printf("Warning: Failed to parse CRD %s: %v\n", c.Filename, err) + continue } + sum := sha256.Sum256(c.File.Data) + candidates = append(candidates, crdCandidate{ + key: *key, + chartPath: strings.TrimSuffix(c.Filename, "/"+c.Name), + filename: c.Filename, + digest: hex.EncodeToString(sum[:]), + file: c.File, + }) } - // Convert to slice in a deterministic order (sorted by group then kind) - // so repeated runs over the same input always produce identical output. - crdKeys := make([]schema.GroupKind, 0, len(crdMap)) - for key := range crdMap { - crdKeys = append(crdKeys, key) - } - sort.Slice(crdKeys, func(i, j int) bool { - if crdKeys[i].Group != crdKeys[j].Group { - return crdKeys[i].Group < crdKeys[j].Group + // Order candidates deterministically so the same input always yields the + // same output, independent of Helm's dependency iteration order (which is + // not guaranteed stable and varies with how subcharts were loaded): + // 1. group, then kind -> stable output file order + // 2. chart depth -> the parent chart wins over its subcharts + // 3. chart path, filename -> stable tiebreak among subcharts + // 4. content digest -> final tiebreak when the same GroupKind is + // provided by two sources that share a path + // (e.g. a directory and a .tgz of one subchart, + // or a diamond dependency) yet differ in content + sort.Slice(candidates, func(i, j int) bool { + a, b := candidates[i], candidates[j] + if a.key.Group != b.key.Group { + return a.key.Group < b.key.Group + } + if a.key.Kind != b.key.Kind { + return a.key.Kind < b.key.Kind + } + if da, db := chartDepth(a.chartPath), chartDepth(b.chartPath); da != db { + return da < db + } + if a.chartPath != b.chartPath { + return a.chartPath < b.chartPath + } + if a.filename != b.filename { + return a.filename < b.filename } - return crdKeys[i].Kind < crdKeys[j].Kind + return a.digest < b.digest }) - crdFiles := make([]*chart.File, 0, len(crdMap)) - for _, key := range crdKeys { - crdFiles = append(crdFiles, crdMap[key]) + + // The first candidate for each GroupKind wins; the rest are dropped. Since + // candidates are already sorted by (group, kind), crdFiles comes out in a + // stable order too — no further sorting needed. A warning is emitted only + // when a dropped copy actually differs in content from the winner, so that + // identical duplicates (the common case) stay quiet while genuine version + // conflicts remain visible. + winners := make(map[schema.GroupKind]crdCandidate) + crdFiles := make([]*chart.File, 0, len(candidates)) + for _, cand := range candidates { + if winner, exists := winners[cand.key]; exists { + if cand.digest != winner.digest { + fmt.Printf("Warning: CRD %s/%s conflicts between %s and %s — keeping version from %s\n", + cand.key.Kind, cand.key.Group, winner.chartPath, cand.chartPath, winner.chartPath) + } + continue + } + winners[cand.key] = cand + crdFiles = append(crdFiles, cand.file) } var extraFiles []*chart.File @@ -168,24 +211,23 @@ func NewCmdGenerateCRDOnlyChart() *cobra.Command { return cmd } -func collectCRDs(ch *chart.Chart, sourceName string, crdMap map[schema.GroupKind]*chart.File, sourceMap map[schema.GroupKind]string) { - for _, f := range ch.CRDObjects() { - key, err := extractCRDKey(f.File.Data) - if err != nil { - fmt.Printf("Warning: Failed to parse CRD %s from %s: %v\n", f.Name, sourceName, err) - continue - } - - if existingSource, exists := sourceMap[*key]; exists { - fmt.Printf("Warning: CRD %s/%s duplicated in %s — keeping version from %s\n", - key.Kind, key.Group, sourceName, existingSource) - continue - } - - // New unique CRD - crdMap[*key] = f.File - sourceMap[*key] = sourceName - } +// crdCandidate is a single CRD file discovered somewhere in the chart tree, +// tagged with enough source information to pick a deterministic winner when the +// same GroupKind is provided by multiple charts. +type crdCandidate struct { + key schema.GroupKind + chartPath string // dotted chart path of the source chart, e.g. "kubedb" or "kubedb/charts/petset" + filename string // full path of the CRD file within the chart tree, used as a tiebreak + digest string // sha256 of the file content, the final deterministic tiebreak + file *chart.File +} + +// chartDepth reports how deeply a chart is nested within the tree: 0 for the +// top-level chart, 1 for its direct subcharts, and so on. Helm builds a +// subchart's full path as "/charts/", so counting the "/charts/" +// separators yields the nesting depth. +func chartDepth(chartPath string) int { + return strings.Count(chartPath, "/charts/") } // extractCRDKey parses the YAML CRD and builds a unique key