Skip to content

EPMDEDP-17242: fix: generate the scaffold chart README instead of hand-writing it - #301

Closed
geekhubuser wants to merge 1 commit into
masterfrom
EPMDEDP-17242
Closed

EPMDEDP-17242: fix: generate the scaffold chart README instead of hand-writing it#301
geekhubuser wants to merge 1 commit into
masterfrom
EPMDEDP-17242

Conversation

@geekhubuser

@geekhubuser geekhubuser commented Jul 29, 2026

Copy link
Copy Markdown

Description

The deploy-templates/README.md that the operator writes into every scaffolded application was
not generated — it was rendered from README.tmpl, a hand-transcribed copy of what helm-docs
produces from values.tmpl. Two sources of truth for one file, with nothing enforcing that they
agree.

Adding the HTTPRoute exposure (f3a3d76) updated the values and transcribed the new rows by hand,
and the transcription was wrong in two ways:

  1. Sort positionhelm-docs sorts the table alphanumerically (httproute < image <
    imagePullSecrets < ingress). The httproute rows were put in the slot the ingress rows
    occupy, which is correct for ingress and wrong for httproute.
  2. Documented mapvalues.tmpl carried a # -- comment on the gateway: map, so
    helm-docs emits a single collapsed object row and never descends into .name / .namespace.
    The template hardcoded the two child rows instead.

The result: on a cluster where the operator runs with INGRESS_CONTROLLER_TYPE=envoy, every newly
scaffolded project shipped a README that helm-docs disagreed with, so the helm-docs task failed
in that project's review pipeline — for every language. It was latent on nginx clusters because the
ingress rows happen to sit in the right place.

This PR fixes the cause rather than the transcription: README.tmpl is now generated and CI
fails when it is stale.

  • hack/scaffolddocs renders the scaffold through the operator's own util.CopyTemplate, runs
    helm-docs over the result the same way the review pipeline does, and substitutes the
    codebase-specific values back as template actions. Both ingress-controller variants are rendered
    for every platform and merged into a single template, each variant-specific run of rows behind
    the guard that selects it. The controller is selected independently of the platform
    (INGRESS_CONTROLLER_TYPE vs PLATFORM_TYPE), so covering both everywhere is what keeps a
    scaffold that starts branching from being documented for only one of them — a platform whose
    scaffold ignores the selection renders both variants identically and merges to unguarded output,
    which is why OpenShift's template is unchanged.
  • make scaffold-docs regenerates the templates; make validate-scaffold-docs fails when the
    committed ones are stale, and runs as its own job on every PR.
  • values.tmpl now documents the parent Gateway per field, which also uncovers sectionName — it
    was commented out and its # -- description was dead, because the collapsed map hid it. It is
    guarded by {{- with }} in httproute.yaml, so the empty default renders nothing.
  • CopyHelmChartTemplates now closes the three chart files it creates. The leaked handles are what
    made t.TempDir() cleanup fail across the suite on Windows.

README.tmpl for OpenShift is regenerated byte-for-byte identical to the committed one, which is an
independent check that the generator reproduces a template that was already correct.

Fixes the helm-docs failures blocking the Tekton smoke suite.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (non-breaking change which improves an existing feature or documentation)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)

How Has This Been Tested?

Reproduced first. Rendering the scaffold from master with the sandbox's configuration
(envoy / main-gateway / envoy-gateway-system) and running helm-docs reproduced the pipeline
failure byte-for-byte — the same 13 changed lines seen in
review-go-beego-rls-crt-bld-release-1-2-5dsgq and review-js-crt-br-smv-bld-new-knb9g.

Verified against the exact image the task uses, jnorwood/helm-docs:v1.13.1, invoked as the
task invokes it (--chart-search-root deploy-templates from the repo root), for every combination:

Scenario Result
kubernetes + envoy no drift
kubernetes + nginx no drift
kubernetes, ingress controller unset no drift, falls back to ingress
openshift + envoy no drift
openshift + nginx no drift

Before this change, kubernetes + envoy was the failing case; the rest passed and still pass.

helm-docs v1.13.1 (the task) and v1.14.2 (this repo's pin) were confirmed to produce identical
output for this chart, so the pin validates what the pipeline checks.

Rendered manifests, to confirm the newly exposed sectionName behaves:

  • --set httproute.enabled=trueparentRefs carries no sectionName
  • --set httproute.gateway.sectionName=httpssectionName: https is rendered
  • nginx variant still renders its Ingress; the dormant httproute.yaml renders nothing and does
    not error when its values block is absent

Generator is idempotent — a second run reproduces byte-identical output.

Tests: go test ./hack/... ./pkg/util/... passes. On the full suite this change takes the
failure count from 4 to 3; the three that remain fail identically on a clean master and are
Windows-only t.TempDir() teardown leaks in unrelated tests (TestIsDirectoryEmpty,
TestPrepareTemplates_ShouldSkipSonarConfig, TestGitProvider_CommitChanges) plus the envtest
webhook suite. golangci-lint is clean on every file touched.

Checklist:

  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Pull Request contains one commit. I squash my commits.

Additional context

Tests added. pkg/util asserts, for each platform and exposure, that the scaffolded README's
rows keep the alphanumeric order helm-docs sorts by and document the exposure the platform
selected — that is defect 1, caught without needing the helm-docs binary. hack/scaffolddocs
covers the merge itself: variant-specific rows get guarded, a key both variants document with
different values keeps one guarded row each, a single-variant platform stays unguarded, and the
generator refuses to emit rather than guess when variants differ outside the values table or when a
row is shared by only some of them. A further test fails if a scaffold template gains a
{{.Field}} the generator does not supply — that failure mode is silent otherwise, since the field
renders empty and its row simply disappears.

Defect 2 is specific to helm-docs semantics and is caught by the validate-scaffold-docs CI job
rather than by a unit test, which is deliberate: reimplementing those semantics in a test would
recreate the duplication this PR removes.

Backport. The defect ships in release/2.34 (2.34.0, pinned by edp-install release/3.14) as
well as master. release/2.33 is unaffected. A cherry-pick and a 2.34.1 patch are needed for
clusters on 3.14 that enable Envoy.

Already-scaffolded repositories are not repaired by this change — PutDeployConfigs pushes the
templates once per codebase, guarded by Status.Git == ProjectTemplatesPushedStatus. Projects
created on an Envoy cluster before the fix keep the stale README and need a one-off
helm-docs && git commit. The autotest fixtures are recreated each run and heal on their own.

@geekhubuser
geekhubuser requested a review from SergK July 29, 2026 09:32
@geekhubuser
geekhubuser requested a review from a team as a code owner July 29, 2026 09:32
@epmd-edp

epmd-edp commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Pipeline review-codebase-operator-master-lxv4g ✅ Passed

Status Task Duration
github-set-pending-status 6s
fetch-repository 15s
init-values 4s
get-cache 2m8s
commit-validate 5s
dockerfile-lint 7s
helm-lint 7s
helm-docs 31s
build 1m9s
sonar 30s
save-cache 30s
buildkit-build 27s
github-report-pipeline-status 7s

…d-writing it

The README of the scaffolded application chart was a hand-transcribed copy of what helm-docs generates from
values.tmpl. Adding the HTTPRoute exposure updated the values but transcribed the table wrongly, so on Envoy
clusters every scaffolded project shipped a README helm-docs disagreed with, failing the helm-docs task of
its review pipeline regardless of language.

- Generate README.tmpl from the rendered chart with helm-docs, so the operator ships exactly what the review
  pipeline verifies and the table can no longer drift from the values next to it
- Guard it in CI: make validate-scaffold-docs regenerates the templates and fails when they are stale
- Document the parent Gateway per field and expose sectionName, which the collapsed map hid
- Close the chart files the scaffold writes; the leaked handles broke test cleanup on Windows
@SergK

SergK commented Jul 30, 2026

Copy link
Copy Markdown
Member

addressed in #302 - let's use simplify approach

@SergK SergK closed this Jul 30, 2026
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.

3 participants