Skip to content

fix(nativeHelm): support OCI repoUrl override for ORG_CHART deploys#125

Open
vigneshrajsb wants to merge 3 commits intomainfrom
fix/org-chart-oci-repo-url
Open

fix(nativeHelm): support OCI repoUrl override for ORG_CHART deploys#125
vigneshrajsb wants to merge 3 commits intomainfrom
fix/org-chart-oci-repo-url

Conversation

@vigneshrajsb
Copy link
Contributor

@vigneshrajsb vigneshrajsb commented Mar 10, 2026

Problem

When a service config specifies a custom helm.chart.repoUrl (e.g., an OCI ECR registry), the generated helm upgrade command was ignoring it for services that use the org chart (goodrx-app). The command would use just the bare chart name with no repo reference:

helm upgrade --install lifecycle-docs-<uuid> goodrx-app --namespace ... --version 2.2.1 ...

Root Cause

determineChartType returns ChartType.ORG_CHART when chartName === orgChartName && helm.docker is set. In constructHelmCommand, the else branch (which handles ORG_CHART) unconditionally used the bare chartPath and never checked chartRepoUrl:

} else {
  command += ` ${chartPath}`;  // always "goodrx-app", repoUrl ignored
}

The generateHelmInstallScript function also only ran helm repo add for ChartType.PUBLIC, skipping ORG_CHART entirely.

Note: mergeChartConfig correctly gives service-level repoUrl priority over global config — the value was properly resolved, just never used.

Fix

constructHelmCommand — add OCI detection to the ORG_CHART branch:

} else {
  const isOciChart = chartRepoUrl?.startsWith('oci://');
  if (isOciChart) {
    command += ` ${chartRepoUrl}`;
  } else {
    command += ` ${chartPath}`;
  }
}

generateHelmInstallScript — extend helm repo add to also cover ORG_CHART with a non-OCI custom repoUrl:

if (chartType === ChartType.PUBLIC || chartType === ChartType.ORG_CHART) {

After this fix, the generated command correctly uses the OCI URL as the chart reference:

helm upgrade --install lifecycle-docs-<uuid> oci://441168312025.dkr.ecr.us-west-2.amazonaws.com/goodrx-app --namespace ... --version 2.2.1 ...

constructHelmCommand now detects OCI URLs in the ORG_CHART branch and
uses the full chartRepoUrl as the chart reference instead of the bare
chart name. generateHelmInstallScript extended to run helm repo add for
ORG_CHART when a non-OCI custom repoUrl is present, matching the
existing PUBLIC chart behavior.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vigneshrajsb vigneshrajsb marked this pull request as ready for review March 10, 2026 01:36
@vigneshrajsb vigneshrajsb requested a review from a team as a code owner March 10, 2026 01:36
vigneshrajsb and others added 2 commits March 10, 2026 09:50
… helm 3.7.x

Set HELM_EXPERIMENTAL_OCI=1 unconditionally in the helm deploy container env
vars. This is required for OCI registry support on helm 3.7.x and is a no-op
on helm 3.8+ where OCI support became stable.
…art deploys

Introduces a registry auth module that detects ECR registries from OCI
repo URLs and injects an init container to authenticate before the main
helm deploy step runs. The init container uses the AWS CLI to generate
a temporary ECR password and calls `helm registry login`, which is then
reused by `helm install`/`helm upgrade` in the main container.

- registryAuth.ts: detectRegistryAuth, createRegistryAuthInitContainer,
  generateRegistryLoginScript
- jobFactory.ts: HelmJobConfig gains registryAuth field; init container
  injected after clone-repo when auth is required
- utils.ts: generateHelmInstallScript prepends helm registry login when
  registryAuth is set
- helm.ts: generateHelmManifest detects ECR auth and threads it through

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant