Skip to content

Change ingress example to use Traefik#1248

Open
welteki wants to merge 1 commit intoopenfaas:masterfrom
welteki:traefik-ingress
Open

Change ingress example to use Traefik#1248
welteki wants to merge 1 commit intoopenfaas:masterfrom
welteki:traefik-ingress

Conversation

@welteki
Copy link
Member

@welteki welteki commented Feb 11, 2026

Description

Update ingress examples and documentation to use Traefik instead of Nginx as the ingress controller reference.

Changes:

  • Update values.yaml ingress annotations and ingressClassName example to Traefik
  • Update SNS connector README ingress instructions to use Traefik

Why is this needed?

  • I have raised an issue to propose this change (required)

Ingress Nginx will be retired in March 2026. Traefik is now the recommended ingress controller. The examples and docs should reflect this.

How Has This Been Tested?

Reviewed the changed documentation and Helm values for correctness.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Use traefik for ingress as the default example. Ingress nginx will be
retired in march 2026.

Signed-off-by: Han Verstraete (OpenFaaS Ltd) <[email protected]>
@reviewfn
Copy link

reviewfn bot commented Feb 11, 2026

AI Pull Request Overview

Summary

  • Updated ingress configuration examples in values.yaml to use Traefik instead of Nginx
  • Modified SNS connector README to reflect Traefik as the recommended ingress controller
  • Replaced Nginx-specific annotations and class references with Traefik equivalents
  • Changes are documentation-only, affecting Helm chart values and installation guides

Approval rating (1-10)

8/10 - Solid documentation updates but lacks migration guidance for existing Nginx users

Summary per file

Summary per file
File path Summary
chart/openfaas/values.yaml Updated ingress annotations and ingressClassName to Traefik examples (max 20 words)
chart/sns-connector/README.md Changed ingress setup instructions from Nginx to Traefik (max 20 words)

Overall Assessment

The pull request appropriately updates OpenFaaS documentation and examples to recommend Traefik over Nginx as the ingress controller. The changes are technically correct and consistent across both modified files. However, the PR lacks explicit migration guidance, which could cause issues for existing users who followed the previous Nginx-based examples. Consider adding a brief migration note or updating related documentation to address potential breaking changes for current deployments.

Detailed Review

Detailed Review

chart/openfaas/values.yaml

  • Correctness: The removal of the deprecated kubernetes.io/ingress.class: nginx annotation aligns with Kubernetes best practices. The new Traefik annotations (traefik.ingress.kubernetes.io/router.entrypoints: websecure and traefik.ingress.kubernetes.io/router.tls: "true") provide appropriate examples for HTTPS configuration.
  • Consistency: Changing ingressClassName from nginx to traefik maintains consistency with the shift to Traefik as recommended controller.
  • Impact: Low impact as these are example configurations, but existing users may need to adjust if they copied the old values.
  • Risk: Potential for confusion if users have mixed ingress controllers; the examples now assume Traefik deployment.
  • Suggestion: Add a comment noting that examples are Traefik-specific and users should verify their ingress controller setup.

chart/sns-connector/README.md

  • Correctness: All changes accurately reflect Traefik usage - installation command updated to arkade install traefik2, cert-manager issuer class changed to traefik, and ingress spec includes ingressClassName: traefik.
  • Consistency: The textual updates from "Ingress Nginx" to "Traefik" are clear and consistent throughout.
  • Migration: The removal of the deprecated kubernetes.io/ingress.class: nginx annotation is good, but the PR doesn't address how existing users should migrate their ingress resources.
  • Testing: No functional code changes, so testing focus should be on documentation accuracy. The PR mentions reviewing changes for correctness, which appears done.
  • Security/Performance: No concerns; documentation-only changes.
  • Risk: Existing SNS connector deployments using Nginx may require manual ingress updates. Consider adding a migration section to the README or PR description.

General Issues:

  • The PR is classified as "New feature" in the checklist, but this is primarily a documentation update. Consider reclassifying as "Documentation" or "Enhancement".
  • No tests added, which is appropriate for documentation changes, but verify that the examples work in practice.
  • Challenge assumption: Is Traefik universally "recommended"? Ensure this aligns with official OpenFaaS guidance.
  • Potential regression: Users upgrading charts may encounter ingress issues if they haven't switched to Traefik. Propose adding version compatibility notes.

AI agent details.

Agent processing time: 38.856s
Environment preparation time: 8.303s
Total time from webhook: 57.156s

@welteki welteki marked this pull request as draft February 11, 2026 13:23
@welteki welteki marked this pull request as ready for review February 16, 2026 11:24
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