From 5cdbe9b0b664de1ba0cdb004e696b2d7bf153061 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 19 Oct 2026 23:42:36 +0200 Subject: [PATCH] fix(deploy): taint Helm changes of and/or, later conditions, sortAlpha, parse errors Review round 4 of #2677 (user decisions: R4-1..R4-4, R4-6, R4-7, R4-9; R4-5 measured and left out). Every case ends in an UNKNOWN image, never a wrong one. Changes not run exactly (helm_template.c): - A deep merge into a values map is not exact when a change was made at or below its path (a set storing a map there): every values map a set or a merge changes keeps its path, and such a merge leaves the images after it UNKNOWN. - and / or at an operand whose truth is not known -- UNKNOWN, or a map a change made unknown -- leave the images after them UNKNOWN when an operand after it may change maps. - In an if / with an UNKNOWN condition skips, the else-if / else-with conditions after it are looked at too: a change there taints, an assignment there makes its variable UNKNOWN. - sortAlpha sorts a []string in place: it counts as a change (a list is never poisoned, so it taints). No other Sprig list function changes its argument in place. - A template, a define body or a scanned template file the grammar does not parse may hide a change: its images are UNKNOWN. Work (helm_template.c): - A capture's keys leave the key set one by one; the set is no longer cleared by walking its whole capacity after each capture. - Template files are found by a hash of their path, and an include of a name that is no define is answered once per render. Known limit, documented (README): each template file renders on its own, while Helm renders a chart's files with one shared .Values. Running them in Helm's order is the planned follow-up; the coarse rule across files cost 83 more correct images on the 20 infra corpora and was left out. On the 20 infra corpora: Image edges identical to round 4 (0 lost, 0 added), no budget trip. Tests: R4-1..R4-4 and R4-9 fail on 3401def5 by their assertions (each sub case too); R4-6 and R4-7 are correctness controls, their cost measured outside the suite. Signed-off-by: Martin Vogel --- README.md | 1 + src/pipeline/helm_template.c | 172 +++++++++++++++++++++++++--------- tests/test_grammar_probe_f.c | 9 +- tests/test_pipeline.c | 177 +++++++++++++++++++++++++++++++++++ 4 files changed, 312 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 590c9eef9..ee1db98bb 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,7 @@ The install script placed beside the binary is **reported, not deleted**: uninst - `INFRA_MAPS`: a Kubernetes Service to each workload whose pod labels carry every `key: value` of the Service's selector, within one namespace - `CONNECTS_TO`: a Kubernetes workload or compose service to the Service its containers' environment addresses (`PRODUCT_CATALOG_SERVICE_ADDR: productcatalogservice:3550`, a URL's host, `..svc...`, a bare name on an `*_ADDR` / `*_HOST` / `*_URL` / ... key; compose: the project's `.env` interpolated): a Kubernetes Service of the same bundle and namespace, or a compose service of the same file. The edge keeps `via: env`, `key` and `container`, never the value - `USAGE` with `ref: image`: a Kubernetes workload (any pod template: containers, initContainers, CronJobs, custom resources) or a docker compose service to the `Image` it runs +- Helm templates are rendered for their images: with the chart's values (the `Chart` `DEPLOYS` them, `default_of`: the values keys read) and with each release's (a Flux HelmRelease, a kustomize `helmCharts` entry, a Terraform `helm_release`: `override_of`). An image the render cannot know is left out, never guessed: after a change of values it does not run exactly, a `tpl` or include it cannot look into, a release value it cannot read, or a work budget it runs past. Known limit: each template file renders on its own, while Helm renders a chart's files with one shared `.Values` (a subchart's is its parent's `.Values.`), so a chart that changes `.Values` in one file and reads it in another can still show a wrong image - `DEPLOYS`: what puts manifests, kustomizations or charts on a cluster, to what it puts there: a Flux Kustomization or Argo Application to the kustomizations / manifests / chart of its path, a Flux HelmRelease to its chart (`spec.chart` or `chartRef` → OCIRepository), a shell command or Make recipe (`kubectl apply -f/-k`, `kustomize build | kubectl apply`, `helm install/upgrade/template`), a Go installer loading a chart with the Helm SDK, a Terraform `helm_release`, a Terraform `kubernetes_*` resource the object it declares (a `Resource` like a manifest's, `ecosystem: terraform`, when its name is a literal), a kustomization inflating a chart (`helmCharts`; its values file an `IMPORTS`). A chart outside the repository is an external `Chart`, keyed by its publisher when its repository URL is stated (`__chart__/`, the same node in every project, the URL in `repository`), else one per project and name; the edge says `via` (`flux`, `argo`, `helmrelease`, `kubectl`, `kustomize`, `helm`, `helm-sdk`, `terraform`, `helmcharts`, ...) - `DEPENDS_ON`: a Helm chart to the charts it declares (the local chart for a resolvable `file://` repository, otherwise an external `Chart`; the edge keeps `repository`), and a `go.mod` / `requirements.txt` file to an external `Package`; a Dockerfile to the `Image`s it builds on (`via: from`, `copy-from`); an `Image` to the Dockerfile it is built from (`via: build`; `tool`: `skaffold`, `compose`, `docker-build` for `docker` / `podman` / `nerdctl` / `buildah` build commands in scripts, Make and CI); a compose service to the services its `depends_on` / `links` name - `CONFIGURES`: code reading an environment variable to the shared `EnvVar` of that name (`strategy: env_access`); a Kubernetes container, compose service or Dockerfile (`ENV` of the image) setting one to that container's own `EnvVar` (`strategy: env_set`, with `source`: `value`, `configMapKeyRef`, `secretKeyRef`, `fieldRef`, ..., `compose`, `dockerfile`; values are never stored); and that container's `EnvVar` to the code reading the name inside the files its image is built from (`strategy: env_read`, with `dockerfile`): the image's files are the sources its Dockerfile's `COPY` / `ADD` put into it, less the context's `.dockerignore` diff --git a/src/pipeline/helm_template.c b/src/pipeline/helm_template.c index 5004c2e7d..7c9a5d894 100644 --- a/src/pipeline/helm_template.c +++ b/src/pipeline/helm_template.c @@ -916,15 +916,17 @@ struct cbm_helm_defines { hd_file_t **files; /* each allocated once: defines point into them */ int nfiles; int cap; - CBMHashTable *names; /* name -> hd_def_t* */ + CBMHashTable *names; /* name -> hd_def_t* */ + CBMHashTable *by_rel; /* path -> hd_file_t* */ }; cbm_helm_defines_t *cbm_helm_defines_new(void) { cbm_helm_defines_t *d = cbm_calloc(CBM_MEM_CLASS_OTHER, sizeof(*d)); if (d) { d->names = cbm_ht_create(0); - if (!d->names) { - cbm_free(CBM_MEM_CLASS_OTHER, d); + d->by_rel = cbm_ht_create(0); + if (!d->names || !d->by_rel) { + cbm_helm_defines_free(d); return NULL; } } @@ -1018,6 +1020,7 @@ void cbm_helm_defines_add(cbm_helm_defines_t *d, const char *rel, const char *sr ts_parser_delete(parser); } d->files[d->nfiles++] = f; + cbm_ht_set(d->by_rel, f->rel, f); if (f->tree) { hd_scan(d, f, ts_tree_root_node(f->tree), hd_depth(rel)); } @@ -1037,6 +1040,9 @@ void cbm_helm_defines_free(cbm_helm_defines_t *d) { cbm_ht_foreach(d->names, hd_free_def, NULL); cbm_ht_free(d->names); } + if (d->by_rel) { + cbm_ht_free(d->by_rel); /* its keys are the files' own */ + } for (int i = 0; i < d->nfiles; i++) { if (d->files[i]->tree) { ts_tree_delete(d->files[i]->tree); @@ -1051,12 +1057,7 @@ void cbm_helm_defines_free(cbm_helm_defines_t *d) { /* Template file `rel` of the set; NULL: none. */ static const hd_file_t *hd_file_find(const cbm_helm_defines_t *d, const char *rel) { - for (int i = 0; d && i < d->nfiles; i++) { - if (strcmp(d->files[i]->rel, rel) == 0) { - return d->files[i]; - } - } - return NULL; + return d && d->by_rel ? cbm_ht_get(d->by_rel, rel) : NULL; } static const hd_def_t *hd_find(const cbm_helm_defines_t *d, const char *name) { @@ -1394,6 +1395,11 @@ typedef struct { * and for a merge of values also by values path (everything below it) */ CBMHashTable *poison_maps; CBMHashTable *poison_paths; + /* the values path of each values map changed (a set or a merge): a merge + * above one of them reaches what it put there */ + const char **changes; + int nchanges; + int chcap; /* a change of maps not run exactly: every capture ending later is UNKNOWN */ bool tainted; CBMHashTable *mutates; /* define name -> it may change a map (he_define_may_mutate) */ @@ -1655,6 +1661,37 @@ static bool hst_poison(hst_t *st, const hv_t *v, bool deep) { return hst_poison_at(st, v, deep, 0); } +/* Has a values map at path p or below it been changed? */ +static bool hst_changed_below(hst_t *st, const char *p) { + size_t lp = strlen(p); + for (int i = 0; i < st->nchanges; i++) { + const char *c = st->changes[i]; + if (!hst_step(st) || + (strncmp(c, p, lp) == 0 && (c[lp] == '\0' || c[lp] == '.' || c[lp] == '['))) { + return true; + } + } + return false; +} + +/* Values map at path p changed in place; false with no room to say so. */ +static bool hst_record_change(hst_t *st, const char *p) { + if (st->nchanges == st->chcap) { + int cap = st->chcap ? st->chcap * PAIR_LEN : CBM_SZ_16; + const char **g = cbm_arena_alloc(st->a, (size_t)cap * sizeof(*g)); + if (!g) { + return false; + } + if (st->nchanges) { + memcpy(g, st->changes, (size_t)st->nchanges * sizeof(*g)); + } + st->changes = g; + st->chcap = cap; + } + st->changes[st->nchanges++] = p; + return true; +} + static bool hst_poison_at(hst_t *st, const hv_t *v, bool deep, int depth) { if (depth > HELM_MAX_NESTING || !hst_step(st)) { return false; @@ -1666,9 +1703,12 @@ static bool hst_poison_at(hst_t *st, const hv_t *v, bool deep, int depth) { return false; } /* a deep change reaches every map within: one not known, one a change - * already made unknown, the values root (every path below it), or a map - * not of the values (whose maps no path covers) cannot be poisoned whole */ - if (deep && (hv_poisoned(st, v) || (v->path && !v->path[0]) || (v->k == HV_JSON && !v->path))) { + * already made unknown, the values root (every path below it), a map not + * of the values (whose maps no path covers), or a values map below which + * a change was made (it may hold another's map now) cannot be poisoned + * whole */ + if (deep && (hv_poisoned(st, v) || (v->path && !v->path[0]) || (v->k == HV_JSON && !v->path) || + (v->path && hst_changed_below(st, v->path)))) { return false; } if (deep && v->k == HV_MAP) { @@ -1681,7 +1721,7 @@ static bool hst_poison_at(hst_t *st, const hv_t *v, bool deep, int depth) { } char key[CBM_SZ_32]; snprintf(key, sizeof(key), "%p", map); - if (!hst_set_add(st, &st->poison_maps, key)) { + if (!hst_set_add(st, &st->poison_maps, key) || (v->path && !hst_record_change(st, v->path))) { return false; } return !(deep && v->path) || hst_set_add(st, &st->poison_paths, v->path); @@ -1935,12 +1975,15 @@ static void hst_capture_end(hst_t *st) { p->keys[p->n] = whole ? keys : NULL; p->n++; } + /* the keys this capture recorded, one by one: a clear walks the set's + * whole capacity, which one huge capture leaves behind for every later + * one */ + for (int i = 0; st->ckset && i < st->nck; i++) { + (void)cbm_ht_delete(st->ckset, st->ckeys[i]); + } st->capture = -SKIP_ONE; st->cap = (hsink_t){0}; st->nck = 0; - if (st->ckset) { - cbm_ht_clear(st->ckset); - } } /* Output of the top-level template: into the capture until its newline. */ @@ -2415,10 +2458,16 @@ static hv_t he_strfn(hst_t *st, const char *fn, const hv_t *a, int n) { return HV_UNKNOWN; } -/* Sprig functions that change a map in place. */ +/* Sprig functions that change their argument in place: set / unset / merge + * a map, sortAlpha a []string (splitList, keys, regexSplit give one; a list + * is never poisoned, so it always counts as a change not run). Every other + * list function copies its argument (push, append, prepend, reverse, uniq, + * without, compact, rest, initial, concat, chunk) or reads it (first, last, + * has, slice -- which shares it but changes nothing). */ static bool he_mutator(const char *s, size_t n) { static const char *const MUTATORS[] = { - "set", "unset", "merge", "mergeOverwrite", "mustMerge", "mustMergeOverwrite", NULL}; + "set", "unset", "merge", "mergeOverwrite", "mustMerge", "mustMergeOverwrite", + "sortAlpha", NULL}; return hy_in(s, n, MUTATORS); } @@ -2478,8 +2527,8 @@ static bool he_is_fn(const char *s, size_t n, const char *name) { * looked into), or an include / template of a name not known or of one that * may (he_define_may_mutate). A variable's name ($set) is no call. */ static bool he_may_mutate(hst_t *st, const char *src, TSNode n, int depth) { - if (depth > HELM_MAX_NESTING) { - return true; + if (depth > HELM_MAX_NESTING || ts_node_has_error(n)) { + return true; /* a form the grammar rejects (Go may not) may hide a change */ } TSTreeCursor cur = ts_tree_cursor_new(n); bool found = false; @@ -2514,6 +2563,9 @@ static bool he_may_mutate(hst_t *st, const char *src, TSNode n, int depth) { return found || st->stop; } +/* Memo states of he_define_may_mutate / he_file_may_mutate. */ +enum { HM_NO = 1, HM_YES = 2, HM_BUSY = 3, HM_STALE = 4 }; + /* The chart's template file an include names -- Helm names each one * `<.Template.BasePath>/` -- or NULL: none of this chart's. */ static const hd_file_t *he_template_file(hst_t *st, const char *name) { @@ -2534,7 +2586,6 @@ static const hd_file_t *he_template_file(hst_t *st, const char *name) { * while it is being scanned -- one including itself -- may. A no that leaned * on a define still being scanned is not kept. */ static bool he_file_may_mutate(hst_t *st, const hd_file_t *f, int depth) { - enum { HM_NO = 1, HM_YES = 2, HM_BUSY = 3 }; if (!f->tree) { return true; } @@ -2564,14 +2615,6 @@ static bool he_file_may_mutate(hst_t *st, const hd_file_t *f, int depth) { * scanned again when next asked), so defines including each other never fix * a wrong no -- a yes is final. */ static bool he_define_may_mutate(hst_t *st, const char *name, int depth) { - enum { HM_NO = 1, HM_YES = 2, HM_BUSY = 3, HM_STALE = 4 }; - const hd_def_t *d = hd_find(st->defs, name); - if (!d) { - /* a template file of the chart by its name (the checksum idiom) is - * looked into; a name not the chart's (a library's) may */ - const hd_file_t *f = he_template_file(st, name); - return f ? he_file_may_mutate(st, f, depth) : true; - } if (!st->mutates) { st->mutates = cbm_ht_create(0); } @@ -2579,6 +2622,21 @@ static bool he_define_may_mutate(hst_t *st, const char *name, int depth) { if (memo == HM_YES || memo == HM_NO) { return memo == HM_YES; } + const hd_def_t *d = hd_find(st->defs, name); + if (!d) { + /* a template file of the chart by its name (the checksum idiom) is + * looked into; a name not the chart's (a library's) may. Kept by + * name once final, so an include in a loop asks once */ + const hd_file_t *f = he_template_file(st, name); + bool m = f ? he_file_may_mutate(st, f, depth) : true; + bool final = + m || (st->file_mutates && (uintptr_t)cbm_ht_get(st->file_mutates, f->rel) == HM_NO); + const char *key = final && st->mutates ? cbm_arena_strdup(st->a, name) : NULL; + if (key) { + cbm_ht_set(st->mutates, key, (void *)(uintptr_t)(m ? HM_YES : HM_NO)); + } + return m; + } if (memo == HM_BUSY) { st->busy_hits++; return false; @@ -2988,7 +3046,15 @@ static hv_t he_call(hst_t *st, hframe_t *fr, TSNode call, const hv_t *piped) { uint32_t total = na + (piped ? SKIP_ONE : 0); for (uint32_t i = 0; i < total; i++) { v = i < na ? he_eval(st, fr, ts_node_named_child(args, i)) : *piped; - if (v.k == HV_UNK) { + if (v.k == HV_UNK || hv_poisoned(st, &v)) { + /* its truth is not known (a poisoned map's either): whether + * the operands after it run is not, nor a change they make */ + for (uint32_t j = i + SKIP_ONE; j < na; j++) { + if (he_may_mutate(st, fr->src, ts_node_named_child(args, j), st->nesting)) { + hst_taint(st, fn); + break; + } + } return HV_UNKNOWN; } bool t = hv_truth(&v); @@ -3212,20 +3278,25 @@ static int he_scan_skipped(hst_t *st, hframe_t *fr, TSNode n, bool in_range) { return leave; } -/* The children of n but its condition / collection (what was run), each: +/* The children of n but what was run -- its collection, its conditions up + * to `ran` (the one run; an else if / else with after it would be) -- each: * the variables it would assign UNKNOWN, and a change of maps it may make * (he_may_mutate) leaves every image after it UNKNOWN; returns how it would * leave the loop. */ -static int he_spoil_parts(hst_t *st, hframe_t *fr, TSNode n, const char *only, bool in_range) { +static int he_spoil_parts(hst_t *st, hframe_t *fr, TSNode n, TSNode ran, const char *only, + bool in_range) { int leave = HL_NONE; bool mutates = false; + bool after = false; /* past `ran` */ uint32_t cc = ts_node_child_count(n); for (uint32_t i = 0; i < cc; i++) { TSNode c = ts_node_child(n, i); const char *f = ts_node_field_name_for_child(n, i); - if (!ts_node_is_named(c) || hn_is(c, "range_variable_definition") || - (f && (strcmp(f, "condition") == 0 || strcmp(f, "range") == 0)) || - (only && !(f && strcmp(f, only) == 0))) { + bool cond = f && strcmp(f, "condition") == 0; + bool was_run = cond && !after; + after = after || (cond && ts_node_eq(c, ran)); + if (!ts_node_is_named(c) || hn_is(c, "range_variable_definition") || was_run || + (f && strcmp(f, "range") == 0) || (only && !(f && strcmp(f, only) == 0))) { continue; } leave |= he_scan_skipped(st, fr, c, in_range); @@ -3237,16 +3308,17 @@ static int he_spoil_parts(hst_t *st, hframe_t *fr, TSNode n, const char *only, b return leave; } -/* The bodies of if / with / range n, not run because its condition or - * collection is UNKNOWN (that one was run): their output, the variables they - * would assign and a change they would make are UNKNOWN. A break / continue - * in them may or may not leave the loop around: what the rest of that loop's - * body would assign or change is UNKNOWN too, and a break leaves it now (its - * later iterations are not known), a continue this iteration. */ -static void he_skipped(hst_t *st, hframe_t *fr, TSNode n) { +/* The bodies of if / with / range n, not run because its condition `ran` or + * its collection (a null `ran`) is UNKNOWN (that one was run): their output, + * the variables they -- and the conditions after `ran` -- would assign and a + * change they would make are UNKNOWN. A break / continue in them may or may + * not leave the loop around: what the rest of that loop's body would assign + * or change is UNKNOWN too, and a break leaves it now (its later iterations + * are not known), a continue this iteration. */ +static void he_skipped(hst_t *st, hframe_t *fr, TSNode n, TSNode ran) { hst_out_unknown(st, fr); bool range = hn_is(n, "range_action"); - int leave = he_spoil_parts(st, fr, n, NULL, range); + int leave = he_spoil_parts(st, fr, n, ran, NULL, range); if (leave == HL_NONE) { return; } @@ -3257,7 +3329,7 @@ static void he_skipped(hst_t *st, hframe_t *fr, TSNode n) { loop = ts_node_parent(loop); } if (hn_is(loop, "range_action")) { - (void)he_spoil_parts(st, fr, loop, "body", true); + (void)he_spoil_parts(st, fr, loop, (TSNode){0}, "body", true); } if (leave & HL_BREAK) { st->brk = true; @@ -3293,7 +3365,7 @@ static void he_branches(hst_t *st, hframe_t *fr, TSNode n, bool with) { if (v.k == HV_UNK || hv_poisoned(st, &v)) { stop = true; active = false; - he_skipped(st, fr, n); /* which branch renders is unknown */ + he_skipped(st, fr, n, c); /* which branch renders is unknown */ continue; } active = hv_truth(&v); @@ -3331,7 +3403,7 @@ static void he_range(hst_t *st, hframe_t *fr, TSNode n) { } hv_t coll = he_eval(st, fr, expr); if (coll.k == HV_UNK || hv_poisoned(st, &coll)) { - he_skipped(st, fr, n); + he_skipped(st, fr, n, (TSNode){0}); return; } hv_t *items = NULL; @@ -3472,6 +3544,9 @@ static hv_t he_render_define(hst_t *st, const char *name, hv_t arg, hsink_t *int hst_taint(st, "include"); /* nested too deep: not run */ return HV_UNKNOWN; } + if (ts_node_has_error(d->node)) { + hst_taint(st, "parse"); /* a form the grammar rejects may hide a change */ + } int mark = st->nvars; hframe_t sub = {.src = d->file->src, .dot = arg, .var_base = st->nvars, .out = into}; hst_var_push(st, "", 0, arg); @@ -3584,6 +3659,11 @@ bool cbm_helm_render(CBMArena *a, const char *rel, const char *src, size_t len, uint32_t *lines = cbm_arena_alloc(&work, (size_t)nl * sizeof(*lines)); TSTree *tree = lines ? ts_parser_parse_string(st.parser, NULL, src, (uint32_t)len) : NULL; if (tree) { + /* a template that does not parse may hide a change (Go may take a + * form the grammar rejects): no image of it is known */ + if (ts_node_has_error(ts_tree_root_node(tree))) { + hst_taint(&st, "parse"); + } int k = 0; lines[k++] = 0; for (size_t i = 0; i < len; i++) { diff --git a/tests/test_grammar_probe_f.c b/tests/test_grammar_probe_f.c index 0c8cf008e..b51ad995e 100644 --- a/tests/test_grammar_probe_f.c +++ b/tests/test_grammar_probe_f.c @@ -4947,7 +4947,14 @@ TEST(probe_helm_release_terraform_postrender_forms) { * app/charts/cache/templates, as Helm sets it (an image here shows it). Its * checksum of cm.yaml names its own file, which changes no map -- not app's * cm.yaml, which sets a value: its image stays known, as does app's (which - * checksums its Secret). */ + * checksums its Secret). + * + * NOT a cross-file guarantee: app's cm.yaml sets .Values.image.tag, and Helm + * renders a chart's files with one shared .Values (a subchart's is the + * parent's .Values.). Each file renders alone here, so app's image is + * known only because nothing models that set reaching deploy.yaml -- the + * known limit (review round 4, R4-5), whose exact fix is to run the family's + * files in Helm's order with one .Values (the run-included-files follow-up). */ TEST(probe_helm_checksum_include_by_base_path) { static const GpfFile files[] = { {"charts/app/Chart.yaml", "apiVersion: v2\nname: app\nversion: 1.0.0\ndependencies:\n" diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index aef677834..f004cca3e 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5050,6 +5050,176 @@ TEST(helm_template_file_including_itself) { PASS(); } +static const char HELM_IMAGE_VALUES[] = + "image: {repository: ghcr.io/acme/app, tag: \"1.0\"}\ncfg: {a: {}}\nempty: {}\n"; +#define HELM_IMAGE_LINE "image: \"{{ .Values.image.repository }}:{{ .Values.image.tag }}\"\n" + +/* A set below a values map puts .Values.image at cfg.a.d; a deep merge into + * the map above it (.Values.cfg) reaches .Values.image there: the image after + * it is UNKNOWN. */ +TEST(helm_deep_merge_over_a_change_below) { + static const char tpl[] = + "{{- $_ := set .Values.cfg.a \"d\" .Values.image }}\n" + "{{- $_ := mergeOverwrite .Values.cfg (dict \"a\" (dict \"d\" (dict \"tag\" \"2.0\"))) }}\n" + HELM_IMAGE_LINE; + char *got = helm_capture(HELM_IMAGE_VALUES, tpl, NULL, 3); + ASSERT_STR_EQ(got, ""); + free(got); + PASS(); +} + +/* and / or stop at an operand whose truth is not known (.Release.Namespace; + * a map a set changed -- .Values.empty is empty no more): whether the + * operands after it run is not known, nor a change they make -- the image + * after it is UNKNOWN. A known false one runs none: the image stays known. */ +TEST(helm_and_or_with_an_unknown_operand) { + static const char *const tpls[] = { + "{{- $_ := and .Release.Namespace (set .Values.image \"tag\" \"2.0\") }}\n" HELM_IMAGE_LINE, + "{{- $_ := or .Release.Namespace (set .Values.image \"tag\" \"2.0\") }}\n" HELM_IMAGE_LINE, + "{{- $_ := set .Values.empty \"x\" \"y\" }}{{- $_ := and .Values.empty " + "(set .Values.image \"tag\" \"2.0\") }}\n" HELM_IMAGE_LINE}; + for (size_t i = 0; i < sizeof(tpls) / sizeof(tpls[0]); i++) { + char *got = helm_capture(HELM_IMAGE_VALUES, tpls[i], NULL, 2); + bool unknown = strcmp(got, "") == 0; + free(got); + ASSERT_TRUE(unknown); + } + static const char known[] = + "{{- $_ := and false (set .Values.image \"tag\" \"2.0\") }}\n" HELM_IMAGE_LINE; + char *got = helm_capture(HELM_IMAGE_VALUES, known, NULL, 2); + ASSERT_STR_EQ(got, "ghcr.io/acme/app:1.0"); + free(got); + PASS(); +} + +/* An if whose first condition is UNKNOWN: Helm evaluates the else-if + * conditions after it -- a set there, or an assignment ($tag = ...), may run: + * the image after it is UNKNOWN. */ +TEST(helm_later_condition_of_a_skipped_if) { + static const char set[] = + "{{- if regexMatch \"^v\" .Values.image.tag }}{{- else if set .Values.image \"tag\" \"2.0\" }}" + "{{- end }}\n" HELM_IMAGE_LINE; + char *got = helm_capture(HELM_IMAGE_VALUES, set, NULL, 2); + ASSERT_STR_EQ(got, ""); + free(got); + static const char assign[] = + "{{- $tag := \"1.0\" }}{{- if regexMatch \"^v\" .Values.image.tag }}" + "{{- else if $tag = \"2.0\" }}{{- end }}\n" + "image: \"ghcr.io/acme/app:{{ $tag }}\"\n"; + got = helm_capture(HELM_IMAGE_VALUES, assign, NULL, 2); + ASSERT_STR_EQ(got, ""); + free(got); + PASS(); +} + +/* sortAlpha sorts a []string in place (splitList gives one): the list $tags + * holds changes -- not run here, so the image after it is UNKNOWN. */ +TEST(helm_sort_alpha_changes_its_list) { + static const char tpl[] = "{{- $tags := splitList \",\" .Values.tags }}" + "{{- $_ := sortAlpha $tags }}\n" + "image: \"ghcr.io/acme/app:{{ first $tags }}\"\n"; + char *got = helm_capture("tags: \"2.0,1.0\"\n", tpl, NULL, 2); + ASSERT_STR_EQ(got, ""); + free(got); + PASS(); +} + +/* A form the grammar rejects may hide a change (Go may take it): a template, + * a define it runs, or a template file it includes that does not parse + * leaves its images UNKNOWN. */ +TEST(helm_parse_error_spoils) { + static const char bad[] = "{{- $x := }}\n" HELM_IMAGE_LINE; + char *got = helm_capture(HELM_IMAGE_VALUES, bad, NULL, 2); + ASSERT_STR_EQ(got, ""); + free(got); + static const char defs_src[] = + "{{- define \"bad\" -}}{{- $x := }}{{- end -}}\n" + "{{- define \"good\" -}}ok{{- end -}}\n"; + cbm_helm_defines_t *d = cbm_helm_defines_new(); + ASSERT_NOT_NULL(d); + cbm_helm_defines_add(d, "charts/app/templates/_bad.tpl", defs_src, strlen(defs_src)); + static const char bad_file[] = "{{- $x := }}\nkind: ConfigMap\n"; + cbm_helm_defines_add(d, "charts/app/templates/bad.yaml", bad_file, strlen(bad_file)); + static const char good[] = "x: {{ include \"good\" . }}\n" HELM_IMAGE_LINE; + static const char via_define[] = "x: {{ include \"bad\" . }}\n" HELM_IMAGE_LINE; + static const char via_file[] = + "x: {{ include (print $.Template.BasePath \"/bad.yaml\") . | sha256sum }}\n" HELM_IMAGE_LINE; + char *ok = helm_capture_chart(HELM_IMAGE_VALUES, good, d, &HELM_APP, 2); + char *by_define = helm_capture_chart(HELM_IMAGE_VALUES, via_define, d, &HELM_APP, 2); + char *by_file = helm_capture_chart(HELM_IMAGE_VALUES, via_file, d, &HELM_APP, 2); + cbm_helm_defines_free(d); + ASSERT_STR_EQ(ok, "ghcr.io/acme/app:1.0"); + ASSERT_STR_EQ(by_define, ""); + ASSERT_STR_EQ(by_file, ""); + free(ok); + free(by_define); + free(by_file); + PASS(); +} + +/* One capture reading 20,000 keys, then 2,000 captures of two keys each: + * every image and mark after the big one stays right (its keys leave the + * set one by one; the set is never cleared by walking its capacity). */ +TEST(helm_captures_after_a_huge_capture) { + enum { ITEMS = 20000, LATER = 2000 }; + size_t cap = (size_t)ITEMS * 8 + CBM_SZ_256; + char *vals = malloc(cap); + ASSERT_NOT_NULL(vals); + size_t o = (size_t)snprintf(vals, cap, "image: {repository: ghcr.io/acme/app, tag: \"1.0\"}\n" + "big: ["); + for (int i = 0; i < ITEMS; i++) { + o += (size_t)snprintf(vals + o, cap - o, "%s{x: a}", i ? "," : ""); + } + snprintf(vals + o, cap - o, "]\n"); + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + yyjson_mut_val *v = doc ? cbm_helm_values_parse(doc, vals, strlen(vals)) : NULL; + free(vals); + static const char tpl[] = + "image: \"r/{{ range .Values.big }}{{ .x }}{{ end }}\"\n" + "{{- range 2000 }}\n" + "image: \"{{ $.Values.image.repository }}:{{ $.Values.image.tag }}\"\n" + "{{- end }}\n"; + CBMArena a; + cbm_arena_init_lazy(&a, CBM_SZ_4K); + cbm_helm_probe_t probes[2] = {{.line = 1}, {.line = 3}}; + bool ok = cbm_helm_render(&a, "t.yaml", tpl, strlen(tpl), v, NULL, NULL, probes, 2); + int n = probes[1].n; + bool last = n == LATER && probes[1].image[n - 1] && + strcmp(probes[1].image[n - 1], "ghcr.io/acme/app:1.0") == 0 && + probes[1].keys[n - 1] && + strcmp(probes[1].keys[n - 1], "[\"image.repository\",\"image.tag\"]") == 0; + cbm_arena_destroy(&a); + if (doc) { + yyjson_mut_doc_free(doc); + } + ASSERT_TRUE(ok); + ASSERT_EQ(n, LATER); + ASSERT_TRUE(last); + PASS(); +} + +/* 5,000 includes of one name in a loop -- a template file of the chart with + * no change, or a name that is none -- get one answer each time (asked + * once): the first spoils nothing, the second every image after it. */ +TEST(helm_includes_of_one_name_in_a_loop) { + cbm_helm_defines_t *d = helm_chart_files(); + ASSERT_NOT_NULL(d); + static const char clean[] = + "{{- range 5000 }}{{ $_ := include (print $.Template.BasePath \"/secret.yaml\") $ }}" + "{{ end }}\n" HELM_IMAGE_LINE; + static const char missing[] = + "{{- range 5000 }}{{ $_ := include (print $.Template.BasePath \"/missing.yaml\") $ }}" + "{{ end }}\n" HELM_IMAGE_LINE; + char *kept = helm_capture_chart(HELM_IMAGE_VALUES, clean, d, &HELM_APP, 2); + char *spoiled = helm_capture_chart(HELM_IMAGE_VALUES, missing, d, &HELM_APP, 2); + cbm_helm_defines_free(d); + ASSERT_STR_EQ(kept, "ghcr.io/acme/app:1.0"); + ASSERT_STR_EQ(spoiled, ""); + free(kept); + free(spoiled); + PASS(); +} + /* A compose project's .env feeds its compose file (variables in image, * build): changing it is no closure repair -- the unchanged compose file's * services would keep the old image. It rebuilds; no stale image remains. */ @@ -24811,6 +24981,13 @@ SUITE(pipeline_semantic_manifest_repro) { RUN_TEST(helm_subchart_base_path_names_its_own_file); RUN_TEST(helm_include_of_a_missing_template_file); RUN_TEST(helm_template_file_including_itself); + RUN_TEST(helm_deep_merge_over_a_change_below); + RUN_TEST(helm_and_or_with_an_unknown_operand); + RUN_TEST(helm_later_condition_of_a_skipped_if); + RUN_TEST(helm_sort_alpha_changes_its_list); + RUN_TEST(helm_parse_error_spoils); + RUN_TEST(helm_captures_after_a_huge_capture); + RUN_TEST(helm_includes_of_one_name_in_a_loop); RUN_TEST(incremental_compose_dotenv_change_rebuilds); RUN_TEST(incremental_dockerfile_edit_keeps_docker_build_edges); RUN_TEST(incremental_second_build_declarer_keeps_the_edge);