chore(chart): Add missing tests for readinessProbe by jcpunk · Pull Request #5769 · external-secrets/external-secrets

jcpunk · 2025-12-23T16:27:24Z

Problem Statement

Various linters complain about ports that the container uses, but aren't defined. For example, the readiness port is set, but the port is not explicitly listed in the definition.

Related Issue

Fixes #...

Proposed Changes

All the ports used by the deployment should be explicitly defined to ensure traffic patterns are explicitly defined.

Format

Please ensure that your PR follows the following format for the title:

feat(scope): add new feature
fix(scope): fix bug
docs(scope): update documentation
chore(scope): update build tool or dependencies
ref(scope): refactor code
clean(scope): provider cleanup
test(scope): add tests
perf(scope): improve performance
desig(scope): improve design

Where scope is optionally one of:

charts
release
testing
security
templating

Checklist

I have read the contribution guidelines
All commits are signed with git commit --signoff
My changes have reasonable test coverage
All tests pass with make test
I ensured my PR is ready for review with make reviewable

Fix: Explicitly Define Container Ports in Helm Chart

Ensures all container ports referenced in readiness, liveness, and startup probes are explicitly declared in the port specifications for the cert-controller, controller, and webhook deployments.

Changes:

cert-controller-deployment.yaml: Added explicit port definitions for metrics (8080), readiness probe (8081, configurable), and optional startup probe (8083, configurable)
deployment.yaml: Added explicit port definitions for metrics (8080) and optional liveness probe port (configurable)
webhook-deployment.yaml: Added explicit port definitions for metrics (8080), webhook (10250), and readiness probe (8081, configurable)
Test files: Expanded test coverage to verify port definitions for readiness, liveness, and startup probes with proper name, protocol, and containerPort assertions

Rationale: Resolves linter warnings by explicitly declaring all ports used by the deployments, making traffic patterns clearly visible in the container specification.

coderabbitai · 2025-12-23T16:27:36Z

Walkthrough

This PR expands test coverage in the cert_controller_test.yaml file to verify container port configurations for various probe scenarios, including readiness probe port overrides, metrics port interactions, and startup probe enablement.

Changes

Cohort / File(s)	Change Summary
Test Coverage Expansion `deploy/charts/external-secrets/tests/cert_controller_test.yaml`	Added assertions for readiness probe port behavior when overridden (ports[1] with name "ready", protocol "TCP", containerPort 8082). Added verification for startup probe port behavior when enabled (ports[2] with name "startup", protocol "TCP", containerPort 8083). Added tests validating port interactions between metrics and readiness/startup probes, including assertions for non-existence of subsequent port entries.

Possibly related PRs

chore(charts): define and use named ports for probes #5775: Directly related modification to cert_controller_test.yaml asserting probe-related container ports and named-port behavior for readiness and startup probes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Skarlso · 2026-01-09T15:36:57Z

/ok-to-test sha=55282b9b31468eb2dd8a26b86730bd921b638e35

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml

eso-service-account-app · 2026-01-09T15:55:22Z

[Bot] - ✅ e2e for 55282b9b31468eb2dd8a26b86730bd921b638e35 passed

jcpunk · 2026-01-09T19:32:36Z

I'll get to work on the tests.

The PR itself might need some reworking after #5775 (but since they are both my PRs it is easy for me to track them).

jcpunk · 2026-01-09T19:41:45Z

In theory I have tests that check for the changes under all the branches. In practice....

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

deploy/charts/external-secrets/tests/cert_controller_test.yaml (1)
103-111: Add assertion to verify no startup port exists.

For consistency with the test at lines 243-247, consider adding an assertion to verify that ports[2] doesn't exist when only the readiness port is overridden.
✨ Suggested addition
       - equal:
           path: spec.template.spec.containers[0].ports[1].containerPort
           value: 8082
+      - notExists:
+          path: spec.template.spec.containers[0].ports[2]

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55282b9 and a54f2f9.

📒 Files selected for processing (2)

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml
deploy/charts/external-secrets/tests/cert_controller_test.yaml

🚧 Files skipped from review as they are similar to previous changes (1)

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml

🔇 Additional comments (2)

deploy/charts/external-secrets/tests/cert_controller_test.yaml (2)

243-247: Well-tested scenario for startup probe using readiness port.

The assertions correctly validate that when the startup probe uses the readiness port, only the readiness port entry exists and no separate startup port is created.

264-272: Comprehensive validation for custom startup port.

The assertions correctly validate all aspects of the startup port configuration when a custom port is specified.

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

deploy/charts/external-secrets/tests/cert_controller_test.yaml (1)

94-113: These assertions are correct, but brittle due to hard-coded array indexes (args[7], ports[1], ports[2]).
Any re-ordering of args or ports will cause test failures even if the rendered manifests remain correct. Use array membership assertions instead:

For args: contains with the scalar value "--healthz-addr=:8082"

For ports: contains with an object matching {name: readiness, protocol: TCP, containerPort: 8082}

For absence checks: replace notExists on ports[2] with notExists on ports[?(@.name=="startup")] to match by property rather than index

helm-unittest supports both contains assertions on arrays of scalars and objects, as well as JSONPath filters like [?(@.name=="startup")].

🧹 Nitpick comments (1)

deploy/charts/external-secrets/tests/cert_controller_test.yaml (1)

231-249: Startup-probe-enabled case: consider asserting the full readiness port object, not just ports[1].name.
Right now the test would still pass if ports[1] is named readiness but points at the wrong port/protocol.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a54f2f9 and a7a65e2.

📒 Files selected for processing (2)

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml
deploy/charts/external-secrets/tests/cert_controller_test.yaml

🚧 Files skipped from review as they are similar to previous changes (1)

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml

🔇 Additional comments (1)

deploy/charts/external-secrets/tests/cert_controller_test.yaml (1)

250-274: Verify numeric rendering when values are provided as strings (certController.startupProbe.port: "8083").
The test expects the rendered manifest to use numeric 8083 (good for k8s schema), which requires the template to cast to int; please double-check the deployment template is doing | int for all port fields it emits (containerPort + probe ports).

jcpunk · 2026-01-22T15:58:35Z

rebased off HEAD

Signed-off-by: Pat Riehecky <riehecky@fnal.gov>

jcpunk · 2026-01-23T14:23:26Z

#5775 included a lot of this code, so I've updated the patch to just include the missing tests.

Skarlso · 2026-01-24T08:53:16Z

Perfect. :)

sonarqubecloud · 2026-01-24T08:54:11Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

…secrets#5769)" This reverts commit 824da30.

…-secrets#5769)" This reverts commit b4e4aa3.

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

…secrets#5769)" This reverts commit 824da30. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

…secrets#5769)" This reverts commit 824da30. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

…secrets#5769)" This reverts commit 824da30. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [external-secrets](https://github.com/external-secrets/external-secrets) | minor | `1.2.1` → `1.3.2` | --- ### Release Notes <details> <summary>external-secrets/external-secrets (external-secrets)</summary> ### [`v1.3.2`](https://github.com/external-secrets/external-secrets/releases/tag/v1.3.2) [Compare Source](external-secrets/external-secrets@v1.3.1...v1.3.2) Image: `ghcr.io/external-secrets/external-secrets:v1.3.2` Image: `ghcr.io/external-secrets/external-secrets:v1.3.2-ubi` Image: `ghcr.io/external-secrets/external-secrets:v1.3.2-ubi-boringssl`  #### What's Changed ##### General - chore: release helm chart for v1.3.1 by [@Skarlso](https://github.com/Skarlso) in [#5860](external-secrets/external-secrets#5860) - chore(chart): Add missing tests for readinessProbe by [@jcpunk](https://github.com/jcpunk) in [#5769](external-secrets/external-secrets#5769) - docs: Update FluxCD example by [@umizoom](https://github.com/umizoom) in [#5862](external-secrets/external-secrets#5862) - fix(ci): Removed the unused check for Windows in Makefile by [@HauptJ](https://github.com/HauptJ) in [#5870](external-secrets/external-secrets#5870) - docs(release): Add actual dates for EOL of 1.x releases in stability and support page by [@n4zukker](https://github.com/n4zukker) in [#5889](external-secrets/external-secrets#5889) - docs: Passbolt provider maintenance ownership by [@stripthis](https://github.com/stripthis) in [#5886](external-secrets/external-secrets#5886) - chore: Update Passbolt MaintenanceStatus to MaintenanceStatusMaintained by [@stripthis](https://github.com/stripthis) in [#5887](external-secrets/external-secrets#5887) - fix(security): sanitize json.Unmarshal errors to prevent secret data … by [@moolen](https://github.com/moolen) in [#5884](external-secrets/external-secrets#5884) - fix: webhook initialization order by [@gusfcarvalho](https://github.com/gusfcarvalho) in [#5901](external-secrets/external-secrets#5901) - chore: Cleanup flags by [@evrardj-roche](https://github.com/evrardj-roche) in [#5845](external-secrets/external-secrets#5845) - fix: onepasswordsdk shared tenant by altering the provider in the client cache by [@Skarlso](https://github.com/Skarlso) in [#5921](external-secrets/external-secrets#5921) ##### Dependencies - chore(deps): bump github/codeql-action from 4.31.10 to 4.31.11 by [@dependabot](https://github.com/dependabot)\[bot] in [#5873](external-secrets/external-secrets#5873) - chore(deps): bump pymdown-extensions from 10.20 to 10.20.1 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in [#5877](external-secrets/external-secrets#5877) - chore(deps): bump markdown from 3.10 to 3.10.1 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in [#5880](external-secrets/external-secrets#5880) - chore(deps): bump ubi9/ubi from `22e9573` to `1f84f5c` by [@dependabot](https://github.com/dependabot)\[bot] in [#5871](external-secrets/external-secrets#5871) - chore(deps): bump actions/setup-python from 6.1.0 to 6.2.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#5872](external-secrets/external-secrets#5872) - chore(deps): bump hashicorp/setup-terraform from [`93d5a27`](external-secrets/external-secrets@93d5a27) to [`dcc3150`](external-secrets/external-secrets@dcc3150) by [@dependabot](https://github.com/dependabot)\[bot] in [#5875](external-secrets/external-secrets#5875) - chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 by [@dependabot](https://github.com/dependabot)\[bot] in [#5876](external-secrets/external-secrets#5876) - chore(deps): bump step-security/harden-runner from 2.14.0 to 2.14.1 by [@dependabot](https://github.com/dependabot)\[bot] in [#5878](external-secrets/external-secrets#5878) - chore(deps): bump anchore/sbom-action from 0.21.1 to 0.22.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#5874](external-secrets/external-secrets#5874) - chore(deps): bump packaging from 25.0 to 26.0 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in [#5879](external-secrets/external-secrets#5879) - chore(deps): bump golang from `d9b2e14` to `98e6cff` by [@dependabot](https://github.com/dependabot)\[bot] in [#5907](external-secrets/external-secrets#5907) - chore(deps): bump alpine from `865b95f` to `2510918` in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in [#5914](external-secrets/external-secrets#5914) - chore(deps): bump docker/login-action from 3.6.0 to 3.7.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#5909](external-secrets/external-secrets#5909) - chore(deps): bump actions/cache from 5.0.2 to 5.0.3 by [@dependabot](https://github.com/dependabot)\[bot] in [#5912](external-secrets/external-secrets#5912) - chore(deps): bump actions/attest-build-provenance from 3.1.0 to 3.2.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#5910](external-secrets/external-secrets#5910) - chore(deps): bump hashicorp/setup-terraform from [`dcc3150`](external-secrets/external-secrets@dcc3150) to [`ce70bcf`](external-secrets/external-secrets@ce70bcf) by [@dependabot](https://github.com/dependabot)\[bot] in [#5911](external-secrets/external-secrets#5911) - chore(deps): bump ubi9/ubi from `1f84f5c` to `c8df11b` by [@dependabot](https://github.com/dependabot)\[bot] in [#5908](external-secrets/external-secrets#5908) - chore(deps): bump alpine from 3.23.2 to 3.23.3 in /e2e by [@dependabot](https://github.com/dependabot)\[bot] in [#5915](external-secrets/external-secrets#5915) - chore(deps): bump alpine from `865b95f` to `2510918` by [@dependabot](https://github.com/dependabot)\[bot] in [#5906](external-secrets/external-secrets#5906) - chore(deps): bump pathspec from 1.0.3 to 1.0.4 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in [#5916](external-secrets/external-secrets#5916) - chore(deps): bump babel from 2.17.0 to 2.18.0 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in [#5917](external-secrets/external-secrets#5917) - chore(deps): bump github/codeql-action from 4.31.11 to 4.32.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#5913](external-secrets/external-secrets#5913) #### New Contributors - [@umizoom](https://github.com/umizoom) made their first contribution in [#5862](external-secrets/external-secrets#5862) - [@HauptJ](https://github.com/HauptJ) made their first contribution in [#5870](external-secrets/external-secrets#5870) - [@n4zukker](https://github.com/n4zukker) made their first contribution in [#5889](external-secrets/external-secrets#5889) - [@stripthis](https://github.com/stripthis) made their first contribution in [#5886](external-secrets/external-secrets#5886) **Full Changelog**: <external-secrets/external-secrets@v1.3.1...v1.3.2> ### [`v1.3.1`](https://github.com/external-secrets/external-secrets/releases/tag/v1.3.1) [Compare Source](external-secrets/external-secrets@v1.2.1...v1.3.1) Image: `ghcr.io/external-secrets/external-secrets:v1.3.1` Image: `ghcr.io/external-secrets/external-secrets:v1.3.1-ubi` Image: `ghcr.io/external-secrets/external-secrets:v1.3.1-ubi-boringssl`  For a Full release please referre to <https://github.com/external-secrets/external-secrets/releases/tag/v1.3.0>. This is a fix build for the docker publish flow. #### What's Changed ##### General - fix: ignore the in-toto manifest when promoting the docker build by [@Skarlso](https://github.com/Skarlso) in [#5859](external-secrets/external-secrets#5859) **Full Changelog**: <external-secrets/external-secrets@v1.3.0...v1.3.1> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://kubara.git.onstackit.cloud/STACKIT/kubara/pulls/250

…5769) Co-authored-by: Gergely Bräutigam <skarlso777@gmail.com>

github-project-automation bot added this to External Secrets Dec 23, 2025

github-actions bot added area/chart kind/bug Categorizes issue or PR as related to a bug. labels Dec 23, 2025

github-actions bot added the size/s label Dec 23, 2025

Skarlso reviewed Jan 9, 2026

View reviewed changes

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml Show resolved Hide resolved

Skarlso reviewed Jan 9, 2026

View reviewed changes

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml Outdated Show resolved Hide resolved

jcpunk force-pushed the ready-port branch from 55282b9 to a54f2f9 Compare January 9, 2026 19:40

coderabbitai bot reviewed Jan 9, 2026

View reviewed changes

jcpunk force-pushed the ready-port branch from a54f2f9 to a7a65e2 Compare January 9, 2026 19:58

coderabbitai bot reviewed Jan 9, 2026

View reviewed changes

jcpunk force-pushed the ready-port branch from a7a65e2 to 0ebb9e1 Compare January 22, 2026 15:58

Skarlso previously approved these changes Jan 23, 2026

View reviewed changes

chore(chart): Add missing tests for readinessProbe

bd1a0e3

Signed-off-by: Pat Riehecky <riehecky@fnal.gov>

jcpunk dismissed Skarlso’s stale review via bd1a0e3 January 23, 2026 14:22

jcpunk force-pushed the ready-port branch from 0ebb9e1 to bd1a0e3 Compare January 23, 2026 14:22

jcpunk changed the title ~~fix(chart): ensure ports are defined~~ chore(chart): Add missing tests for readinessProbe Jan 23, 2026

github-actions bot added the kind/chore Categorizes Pull Requests for chore activities (like bumping versions) label Jan 23, 2026

Merge branch 'main' into ready-port

6083892

Skarlso approved these changes Jan 24, 2026

View reviewed changes

Skarlso merged commit 824da30 into external-secrets:main Jan 24, 2026
29 checks passed

github-project-automation bot moved this to Done in External Secrets Jan 24, 2026

jcpunk deleted the ready-port branch January 24, 2026 17:32

nutmos added a commit to nutmos/external-secrets that referenced this pull request Jan 25, 2026

Revert "chore(chart): Add missing tests for readinessProbe (external-…

b4e4aa3

…secrets#5769)" This reverts commit 824da30.

nutmos added a commit to nutmos/external-secrets that referenced this pull request Jan 25, 2026

Reapply "chore(chart): Add missing tests for readinessProbe (external…

28118c6

…-secrets#5769)" This reverts commit b4e4aa3.

nutmos added a commit to nutmos/external-secrets that referenced this pull request Jan 25, 2026

Reapply "chore(chart): Add missing tests for readinessProbe (external…

dd47773

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

nutmos added a commit to nutmos/external-secrets that referenced this pull request Jan 25, 2026

Revert "chore(chart): Add missing tests for readinessProbe (external-…

737a405

…secrets#5769)" This reverts commit 824da30. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

nutmos added a commit to nutmos/external-secrets that referenced this pull request Jan 25, 2026

Reapply "chore(chart): Add missing tests for readinessProbe (external…

bd857bb

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

nutmos added a commit to nutmos/external-secrets that referenced this pull request Feb 11, 2026

Revert "chore(chart): Add missing tests for readinessProbe (external-…

d359d2b

…secrets#5769)" This reverts commit 824da30. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

nutmos added a commit to nutmos/external-secrets that referenced this pull request Feb 11, 2026

Reapply "chore(chart): Add missing tests for readinessProbe (external…

933c42c

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

nutmos added a commit to nutmos/external-secrets that referenced this pull request Feb 18, 2026

Revert "chore(chart): Add missing tests for readinessProbe (external-…

07438cd

…secrets#5769)" This reverts commit 824da30. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

nutmos added a commit to nutmos/external-secrets that referenced this pull request Feb 18, 2026

Reapply "chore(chart): Add missing tests for readinessProbe (external…

9bb2826

…-secrets#5769)" This reverts commit b4e4aa3. Signed-off-by: Nattapong Ekudomsuk <nuttapong_mos@hotmail.com>

dsp0x4 pushed a commit to dsp0x4/external-secrets that referenced this pull request Mar 22, 2026

chore(chart): Add missing tests for readinessProbe (external-secrets#…

5339be4

…5769) Co-authored-by: Gergely Bräutigam <skarlso777@gmail.com>

Uh oh!

Conversation

jcpunk commented Dec 23, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Related Issue

Proposed Changes

Format

Checklist

Fix: Explicitly Define Container Ports in Helm Chart

Uh oh!

coderabbitai bot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Uh oh!

Skarlso commented Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

eso-service-account-app bot commented Jan 9, 2026

Uh oh!

jcpunk commented Jan 9, 2026

Uh oh!

jcpunk commented Jan 9, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

jcpunk commented Jan 22, 2026

Uh oh!

jcpunk commented Jan 23, 2026

Uh oh!

Skarlso commented Jan 24, 2026

Uh oh!

sonarqubecloud bot commented Jan 24, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jcpunk commented Dec 23, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 23, 2025 •

edited

Loading