Commit Graph
9 Commits
Author SHA1 Message Date
volker.raschekandCopilot eeacda2a25 test: align the file names of the helm unit tests with the chart conventions
The chart conventions demand camelCase file names, forbid short names like `svc` and require the unit tests to
mirror the template structure. The unit tests followed none of that consistently. Four casings existed side by
side - camelCase (extraInitContainers.yaml), kebab-case (checksum-annotations.yaml), a snake and kebab mix
(init_directory_structure.sh-rootless.yaml) and all caps (HA.yaml) - so there was no pattern to copy when
adding a suite, which is how the inconsistency kept growing.

Three groups of problems are fixed:

- svc-configuration.yaml used exactly the short name the conventions forbid for files and keys.
- Several file names repeated their own directory, for example deployment/deployment-additional-config.yaml and
  metric-secret/metrics-secret-servicemonitor-disabled.yaml, which names the subject three times. The directory
  already carries that information, so the file name now only carries the scenario. The default scenario of a
  template is named basics.yaml.
- Four suites lived in a directory of a template they do not assert on. They were filed under deployment/
  because the deployment is rendered as a side effect, not because it is the subject:

      deployment/deprecations.yaml                -> deprecation/deployment.yaml
      deployment/inline-config.yaml               -> config/inlineConfig.yaml
      deployment/storage-class-configuration.yaml -> persistentVolumeClaim/storageClass.yaml
      deployment/svc-configuration.yaml           -> service/configuration.yaml

  storage-class-configuration.yaml is the clearest case: a persistentVolumeClaim directory already existed next
  to it. The suite titles of the two suites whose subject changed are adjusted accordingly.

The Makefile passed `--file unittests/helm/values-conflicting-checks.yaml`, but that file does not exist in the
tree. `helm unittest` ignores a missing path without a warning, so the flag has been dead for an unknown amount
of time while still suggesting the checks run. It is removed; the `unittests/helm/**/*.yaml` glob would pick
the file up anyway if it ever returns.

The naming rule is written down in the copilot instructions, together with the constraint that the glob matches
only a single directory level, so a suite can never be placed directly in unittests/helm/.

All 49 suites and 267 tests still pass.

Co-authored-by: Copilot <copilot@github.com>
2026-10-03 22:19:39 +02:00
volker.raschekandCopilot fef4190a74 test: normalize the suite titles of the helm unit tests
Bash / bash-unittest (push) Successful in 14s
Bash / shellcheck (push) Successful in 20s
changelog / changelog (push) Successful in 16s
Helm / helm-lint (push) Successful in 15s
Helm / helm-unittest (push) Successful in 32s
Markdown linter / markdown-lint (push) Successful in 15s
Markdown linter / markdown-link-checker (push) Successful in 56s
The contributor guidelines never defined how a `suite` title has to look, only how an `it` title has to look.
Without a rule five competing schemes grew side by side: `config template | cache config`, `deployment
template (probes)`, `ServiceMonitor template (basic)`, `Test Gateway API httpRoute.yaml` and `Dependency checks
| Customization integrity | valkey`. 24 of the 49 titles started with a lowercase letter, which contradicts the
sentence rule that already applies to the test titles.

More than cosmetics, the missing rule produced three concrete defects:

- `deployment template` was used by both deployment-additional-config.yaml and extraInitContainers.yaml, and
  `Init template` by both init_directory_structure.sh.yaml and its rootless counterpart. `helm unittest`
  identifies a failing suite by its title, so a failure in either file printed the same header and the report
  did not say which file to open.
- deployment/inline-config.yaml was titled `config template`, although it lives in the deployment directory and
  asserts on secret_inlineConfig.yaml. It read like a sixth member of the `config template | ...` group.
- persistentVolumeClaim.yaml was titled `PVC template`, using exactly the short name the chart conventions
  forbid for files and keys.

All suites now follow `Test <subject> <scenario>`, for example `Test deployment probes` or `Test gateway API
httpRoute`. The subject is the template under test, the scenario is what distinguishes the suite from its
siblings, so every title is unique and sorts next to its related suites. The `Test ` prefix of the Gateway API,
ingress and route suites is kept, because it is the only part of the old schemes that already read as a
sentence; the redundant `.yaml` suffix and the `tests` suffix of the storage class suite are dropped.

The rule is written down in the Unit Tests section of the copilot instructions, so the next suite does not have
to guess which of the five schemes was meant.

Co-authored-by: Copilot <copilot@github.com>
2026-10-03 21:21:46 +02:00
volker.raschekandCopilot 61d04282c8 feat(ingress)!: rename className to ingressClassName
The value only ever populated the `spec.ingressClassName` field of the Ingress resource, so the shortened name was a
needless deviation from the Kubernetes API. `ingress.className` carried a `DEPRECATED` marker since #679, but that
marker was a leftover: the rename to the canonical name was proposed there, rejected during review and the follow-up
commits only removed the half-merged template fallback. The marker stayed behind for a year while the value remained
the one and only way to set the Ingress class, which made the documentation actively misleading.

The deprecation guard uses `hasKey` instead of a truthiness check on purpose. The previous chart default was an empty
string, so vendored values files tend to carry `ingress.className: ""` verbatim. A truthiness check would silently
ignore those and leave users wondering why their Ingress class is no longer applied.

Co-authored-by: Copilot <copilot@github.com>
2026-10-03 18:43:05 +02:00
volker.raschekandCopilot 552fe8c56e refactor(templates): move the templates out of the gitea subdirectory
Helm / helm-lint (push) Successful in 9s
changelog / changelog (push) Successful in 29s
Helm / helm-unittest (push) Failing after 21s
Markdown linter / markdown-lint (push) Successful in 17s
Markdown linter / markdown-link-checker (push) Successful in 56s
The `templates/gitea` subdirectory did not group anything meaningful, since every template of this chart belongs to
Gitea. It only duplicated the chart name in every path and forced the unit tests to spell out
`templates/gitea/<name>.yaml`, while `_helpers.tpl` and `NOTES.txt` already lived directly in `templates`.

All templates now live in `templates`, which matches the layout of the bundled sub-charts and the Helm defaults.

The checksum helper in `templates/_secrets.tpl` built its include path from `$root.Template.BasePath` and therefore
carried the subdirectory in a `printf` format string instead of a literal path. Without adjusting it the chart failed to
render with "no template gitea/templates/gitea/secret_config.yaml associated with template gotpl".

Co-authored-by: Copilot <copilot@github.com>
2026-09-14 20:28:17 +02:00
volker.raschekandCopilot 761921faed test(ingress): align assertions with the release name and namespace
The suite header was changed to the release `gitea-unittests` in the namespace `testing`, but the assertions still
expected the previous `gitea-unittest` and `gitea-debug`. That made four cases fail on the rendered resource name, the
`app.kubernetes.io/instance` label and the backend Service name.

Renovate comments had additionally been copied into the expected `app.kubernetes.io/version` and `version` label values.
They only matched by accident, because YAML strips the trailing comment from an unquoted scalar. Removing them keeps the
assertions honest about what is actually compared.

Co-authored-by: Copilot <copilot@github.com>
2026-09-14 15:24:55 +02:00
volker.raschek c510e06609 fix(Makefile): update targets and add link checker 2026-09-13 22:51:05 +02:00
volker.raschekandCopilot e17a4e7a7b refactor(ingress)!: skip the Ingress when the HTTP Service is disabled
changelog / changelog (push) Successful in 16s
check-and-test / check-and-test (push) Failing after 1m51s
An Ingress that points at a Service which the chart does not render is broken by definition: the backend reference
cannot resolve and the ingress controller reports the rule as unavailable. The render condition therefore now also
requires `service.http.enabled` and lives in the new `gitea.ingress.enabled` helper, so the same rule can be reused
by other templates instead of being duplicated.

The namespace is taken from `.Release.Namespace` again. The `namespace` value is not a documented chart parameter,
and letting a single resource opt out of the release namespace breaks `helm uninstall` and Argo CD pruning, because
neither tracks objects outside the release namespace.

The `ingress.className` default changes from an empty string to `nginx`. An empty class makes the cluster fall back
to the default IngressClass, which silently produces a different result per cluster; naming the controller the chart
is tested against makes the rendered output predictable.

The scattered ingress suites are consolidated into a single `unittests/helm/ingress/ingress.yaml` that pins the
release name, namespace and appVersion, as required by the testing conventions, and covers the enable/disable matrix,
annotations, labels, TLS and a custom HTTP port.

BREAKING CHANGE: The Ingress is no longer rendered when `service.http.enabled` is `false`. `ingress.className` now
defaults to `nginx` instead of the cluster's default IngressClass. The undocumented `namespace` value no longer
applies to the Ingress.

Co-authored-by: Copilot <copilot@github.com>
2026-09-13 20:20:00 +02:00
pat-s edc42f69a9 fix: ingress unittests 2025-05-16 15:40:13 +02:00
a94eec4238 refactor: migrate ingress definition out of beta (#679)
### Description of the change

Redesigned ingress configuration to align better with implicit best practices.

### Benefits

- Smarter defaults
- More tests
- Remove [deprecated API Versions](extensions/v1beta1) (e.g. `extensions/v1beta1`) and always use `networking.k8s.io/v1`

### Possible drawbacks

Hopefully 🙃 none

### Applicable issues

fix #674

### Additional information

- Define `ingress.annotations` via helpers
- Move tests from `tests/deployment` to `tests/ingress`
- Use own tests file for ingress tpl tests
- Ensure defaults of `path` and `pathType` are always rendered
- Set top-level default value for `ingress. pathType`
- Change default of `ingress.hosts[0].paths` to `[]` to ensure proper rendering via template

### âš  BREAKING

I think all of these changes should be backward comp with existing ingress definitions, but surely worth highlighting in the changelog of the release.

### Checklist

<!-- [Place an '[X]' (no spaces) in all applicable fields. Please remove unrelated fields.] -->

- [x] Parameters are documented in the `values.yaml` and added to the `README.md` using [readme-generator-for-helm](https://github.com/bitnami-labs/readme-generator-for-helm)
- [x] Breaking changes are documented in the `README.md`
- [x] Templating unittests are added

Co-authored-by: justusbunsi <justusbunsi@noreply.gitea.com>
Co-authored-by: justusbunsi <sk.bunsenbrenner@gmail.com>
Reviewed-on: https://gitea.com/gitea/helm-gitea/pulls/679
Co-authored-by: pat-s <patrick.schratz@gmail.com>
Co-committed-by: pat-s <patrick.schratz@gmail.com>
2025-05-16 13:08:27 +00:00