Skip to content

Fix common prefix calculation#170

Open
robertaistleitner wants to merge 2 commits intoarttor:mainfrom
robertaistleitner:fix-common-prefix-calc
Open

Fix common prefix calculation#170
robertaistleitner wants to merge 2 commits intoarttor:mainfrom
robertaistleitner:fix-common-prefix-calc

Conversation

@robertaistleitner
Copy link
Copy Markdown

This is done by making commonPrefix variable nullable to detect the case that no commonPrefix is set.

Otherwise this would trigger finding a common prefix once no common prefix is found (in the old implementation that means the commonPrefix is an empty string which is just the result of finding out there is no common prefix).

...by making commonPrefix variable nullable to detect the case that no commonPrefix is set (and not start finding a common prefix once no common prefix is found, which means the commonPrefix is an empty string)
@arttor
Copy link
Copy Markdown
Owner

arttor commented May 27, 2025

is it a bugfix? can you please describe bug or add a test case reproducing the issue?

@robertaistleitner
Copy link
Copy Markdown
Author

Hey @arttor, sorry for the (very) late answer on this, I'm still using my fork and I'm super happy with the project but somehow missed to push this further.

Yes, it's a bugfix. The commonPrefix field on the Service struct was a plain string, using "" (empty string) as the sentinel for "not yet calculated." However, an empty string is also a valid result — when resources share no common prefix, the calculation legitimately returns "". This created a conflict:

  1. Common prefix is correctly computed as "" (no shared prefix).
  2. On the next resource, detectCommonPrefix sees prevName == "" and misinterprets it as "first invocation — no prefix computed yet."
  3. It resets the prefix to the new resource's full name, producing an incorrect common prefix.

The fix: Changes commonPrefix from string to *string (pointer). Now:

  • nil means "never computed" (first invocation)
  • "" means "computed, and resources share no common prefix"

This lets detectCommonPrefix correctly distinguish between the two cases.

Impact: Ensures TrimName (which strips the common prefix from Kubernetes resource names when generating Helm chart templates) works correctly when resources share no common prefix, preventing mangled or incorrectly trimmed resource names in the generated chart.

@robertaistleitner
Copy link
Copy Markdown
Author

@arttor is the failing CI something that was introduced in my branch? I don't really have an idea what the errors mean.

@arttor
Copy link
Copy Markdown
Owner

arttor commented Apr 13, 2026

it looks like. can you please double check that you have all latest commits from the main?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants