🐛 fix(helm/v2-alpha): Fixed Helm chart generation to support custom container names by reading the kubectl.kubernetes.io/default-container annotation instead of hardcoding "manager"

[camilamacedo86]

The Helm plugin now reads the kubectl.kubernetes.io/default-container
annotation to determine which container to template, instead of hardcoding
checks for "name: manager". This allows users to customize their container
names while still getting proper Helm templating for image references,
resources, environment variables, security contexts, and arguments.

Falls back to "manager" for backward compatibility with projects that
don't have the annotation.

Fixes #5449

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[camilamacedo86]

/override pull-kubebuilder-e2e-k8s-1-34-0

[k8s-ci-robot]

@camilamacedo86: Overrode contexts on behalf of camilamacedo86: pull-kubebuilder-e2e-k8s-1-34-0

Details

In response to this:

/override pull-kubebuilder-e2e-k8s-1-34-0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[AlirezaPourchali]

i think using the same image name controller:latest would be better!
and also another name for the container instead of my project name, maybe controller-test ?
both inside pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater_test.go

[camilamacedo86]

Hi @AlirezaPourchali

The points raised are addressed now.
WDYT?

[AlirezaPourchali]

I noticed you mentioned pushing new changes but I don't see any commits beyond e5ea31cb4 from Feb 8.

While working on this feature myself (I originally opened issue #5449), I tested the current implementation and discovered two bugs that prevent it from working correctly:

Bug 1: Container names are being templated when they shouldn't be
The substituteResourceNamesWithPrefix function templates container names like {{ include "chart.resourceName" ... }}. Container names are internal pod identifiers (not cluster-wide resource names) and should remain literal. This happens because container names with dashes (e.g., osiris-manager, controller-test) match the same regex pattern intended for resource names like project-webhook-service.

Bug 2: Image templating fails with custom container names
Since substituteResourceNamesWithPrefix runs before templateImageReference, the container name gets templated first, breaking the image regex match. This means images don't get templated at all for custom container names.

I've tested fixes for both issues locally and they work well. Rather than ask you to debug and fix these iteratively, I've decided to submit a separate PR (#5463) with the complete working solution. This seemed cleaner than going back and forth on fixes.

I hope you understand - your work on adding getDefaultContainerName() was a great starting point! I just built on that foundation to make it production-ready.

Thanks for tackling this feature! 🙏

[camilamacedo86]

Close in favor of: #5463