Bug | Name prefix cleanup fails when Kubernetes 63-char truncation cuts into the prefixed name

[TBeijen]

Preface

• If repo-server-api is the direction the project will be heading anyway, this is mostly an FYI and a further argument to head in that direction. I don't yet have the full picture of advantages/disadvantages of using either the traditional rendering method, or repo-server-api. For us, switching to repo-server-api avoided this problem altogether.
• Imo the main challenge will be for rename fix to be robust, handling anything charts could be using to come up with resource names. If that turns out brittle, it might be best to just not fix.

Description

When using render methods that rely on name prefixing (i.e. not repo-server-api), the tool prefixes Application names as {5-char-uuid}-{b|t}-{originalName} to disambiguate base and target branches in the shared cluster. After rendering, strings.ReplaceAll(manifests, app.Id, app.Name) in extract.go strips the prefix from the rendered manifests.

This cleanup fails when a Helm chart's resource naming truncates names at Kubernetes' 63-character limit, cutting into the prefixed application name. The full app.Id string no longer appears in the rendered YAML, so ReplaceAll is a no-op for those resources. The diff then shows eafb2-b-... vs eafb2-t-... as a false positive change.

Reproduction

(We ran into this using chart oci://ghcr.io/clickhouse/clickhouse-operator-helm, version 0.0.4.)

Given an ArgoCD Application named non-prod-clickhouse-operator (28 chars), and a chart that constructs resource names as trunc(releaseName, 62 - len(suffix)) + - + suffix (capped at 63 chars total):

Layer	Name
Original Application name	non-prod-clickhouse-operator (28 chars)
Prefixed name (base)	eafb2-b-non-prod-clickhouse-operator (36 chars)
Prefixed name (target)	eafb2-t-non-prod-clickhouse-operator (36 chars)

The prefix adds 8 characters (eafb2-b- / eafb2-t-), pushing the Helm release name from 28 to 36 chars.

Resource name examples

The chart's resourceName helper uses the formula: trunc(fullname, 62 - len(suffix)) + - + suffix, total max 63 chars.

Resource	Suffix (len)	Fullname truncated?	Final resource name	Cleanup works?
Deployment	controller-manager (18)	No (36 <= 44)	eafb2-b-non-prod-clickhouse-operator-controller-manager (55)	Yes
MutatingWebhookConfiguration	mutating-webhook-configuration (30)	Yes (36 > 32)	eafb2-b-non-prod-clickhouse-oper-mutating-webhook-configuration (63)	No

In the second case, the fullname is truncated to eafb2-b-non-prod-clickhouse-oper (32 chars). The ReplaceAll looks for eafb2-b-non-prod-clickhouse-operator (36 chars) which no longer exists in the manifest, so the prefix leaks into the diff output.

Threshold

The breakpoint is: suffix length > 62 - len(prefixed_name). For a 36-char prefixed name, any suffix longer than 26 characters triggers this. The 8-char prefix overhead makes this much more likely than it would be for the original (unprefixed) name.

Impact

• False positive diffs: resources appear changed (showing eafb2-b- vs eafb2-t- prefix fragments) when they are actually identical
• Affects any chart that truncates resource names at 63 chars and has suffixes long enough to trigger truncation with the added prefix overhead
• Only affects render methods that use name prefixing (not repo-server-api)

Relevant code

• pkg/extract/prefix.go — addApplicationPrefix() creates prefixed names
• pkg/extract/extract.go — strings.ReplaceAll(manifests, app.Id, app.Name) cleanup

Possible fix directions

• Use a regex-based replacement that accounts for truncated variants of app.Id
• After rendering, match resource names by a prefix-aware pattern rather than exact string match
• Document this as a known limitation of the non-repo-server-api render methods

Workaround

Using RENDER_METHOD=repo-server-api (available since v0.1.26) avoids name prefixing entirely and eliminates this issue.

Disclaimer

• PR draft created with assistance of LLM. Human in control and reviewed.

[dag-andersen]

Hi @TBeijen!

Sorry for the slow answer - last week was very busy 😵‍💫

Thanks a lot for the detailed report and reproduction. You were exactly right about the root cause.

For non-repo-server-api render methods, we prefix Application names for base and target isolation. If helm.releaseName is not set, Helm uses that prefixed Application name as .Release.Name. When charts truncate names at 63 chars, the prefixed part can get cut, and our cleanup check misses it - which creates false diffs (...-b-... vs ...-t-...).

I pushed a fix here:

• Fix | Avoid false diffs by setting Helm releaseName when unset #411

What changed:

• Set helm.releaseName only when it is unset
• Use the original Application name (before runtime prefixing)

Notes:

• Existing user-defined helm.releaseName is preserved
• repo-server-api remains unaffected

I also validated this against your ClickHouse-style repro case, and the false-positive diff is gone.

Thanks again for raising this! 🚀

[TBeijen]

@dag-andersen Nice! Looks like an elegant solution. So iiuc fix boils down to:

If releaseName is not already actively set (for whatever reason), use it to prevent the prefix from appearing in the rendered output.

---

Some questions not strictly related to this issue (feel free to just close this one, or ask me to start discussion elsewhere 😉 )

Now in a way I like the approach of repo-server. From what I understand it boils down to constructing the right payload, and then using it as a 'dumb' rendering engine. Less (or no) cluster, project or syncPolicy patching. No prefixing.

I started trying that for the workaround of this issue.

Some things I noticed:

• Transient failures where repo-server seemed unable to download charts. Not deeply investigated yet, hard to pinpoint.
• Looks like those error (might be hitting CDN rate limits, not sure) are exposed as a PermissionDenied error, which might not be accurate (I'll create an issue for that, but need to investigate some more)

In a way I hope trying out repo-server with dedicated permanent argo setup will mitigate the occasional chart download error. Which would also give the speed advantage.

Regardless of using repo-server I was wondering: Does permanent Argo for diffs work in the traditional render method with concurrent PR runs? I suppose that's where the unique application name prefixes come in?

[dag-andersen]

@TBeijen Thanks! - yep, your summary is exactly right 🚀

Does permanent Argo for diffs work in the traditional render method with concurrent PR runs? I suppose that's where the unique application name prefixes come in?

Yes, exactly. The unique prefixes are there to avoid collisions across concurrent runs (and base vs target).

In a way I hope trying out repo-server with dedicated permanent argo setup will mitigate the occasional chart download error.

I would love to see bug reports related to Transient failures where repo-server and PermissionDenied errors. I can only fix problems I can actually see 👀 I am hoping repo-server-api can become the default in the future, so I want to squash as many bugs there as possible.

[TBeijen]

See #416
I'll try to fix the incorrect permission error, and see what chart pull error surfaces (If I manage to reproduce)