Add optional ingress to githubapp chart

[wartraxx51]

---

PR Submission Checklist for internal contributors

• The PR Title

  • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
  • contains a reference JIRA issue number like SQPIT-764
  • answers the question: If merged, this PR will: ... ³

• The PR Description

  • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

---

What's new in this PR?

Issues

Briefly describe the issue you have solved or implemented with this pull request. If the PR contains multiple issues, use a bullet list.

Causes (Optional)

Briefly describe the causes behind the issues. This could be helpful to understand the adopted solutions behind some nasty bugs or complex issues.

Solutions

Briefly describe the solutions you have implemented for the issues explained above.

Dependencies (Optional)

If there are some other pull requests related to this one (e.g. new releases of frameworks), specify them here.

Needs releases with:

• GitHub link to other pull request

Testing

Test Coverage (Optional)

• I have added automated test to this contribution

How to Test

Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.

Notes (Optional)

Specify here any other facts that you think are important for this issue.

Attachments (Optional)

Attachments like images, videos, etc. (drag and drop in the text box)

---

PR Post Submission Checklist for internal contributors (Optional)

• Wire's Github Workflow has automatically linked the PR to a JIRA issue

---

PR Post Merge Checklist for internal contributors

• If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

---

References

1. https://sparkbox.com/foundry/semantic_commit_messages
2. https://github.com/wireapp/.github#usage
3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

[claude]

Code Review — Add optional ingress to githubapp chart

Thanks for this! The ingress template closely follows the standard Helm scaffold conventions and correctly reuses the chart's shared helpers (githubapp.fullname, githubapp.labels). It's cleanly gated behind ingress.enabled and the TLS block is properly optional. A few observations below.

🟡 Suggestions

1. Empty default host (host: "") is a risky default

hosts:
  - host: ""
    paths:
      - path: /
        pathType: Prefix

If someone enables the ingress (ingress.enabled=true) without overriding hosts, this renders an ingress rule with an empty host, which most controllers (nginx) treat as a catch-all / wildcard. That can unintentionally capture traffic. Safer defaults would be either an empty list (hosts: []) so nothing renders until configured, or a placeholder like the upstream scaffold's chart-example.local that makes the requirement obvious.

2. Reference the service port by name rather than number

port:
  number: {{ $.Values.service.port }}

service.yaml defines a named port (name: http). Referencing the backend port by name (port: { name: http }) is generally more resilient to port-number changes and matches how the Service/probes already reference http. Not a bug — just more robust.

3. NOTES.txt isn't updated for ingress
When the ingress is enabled, NOTES.txt still only prints the port-forward / service instructions. Consider adding a short block that prints the ingress URL(s) when ingress.enabled so operators know how to reach the app.

🔵 Minor / FYI

• API version is hardcoded to networking.k8s.io/v1. That's GA since Kubernetes 1.19, so it's fine for modern clusters — flagging only in case older clusters are in scope (the upstream scaffold uses a semverCompare/Capabilities guard).
• {{- with .Values.ingress.className }} correctly lets the class be disabled by clearing it — good.

✅ Security

No concerns. TLS is supported and gated; no secrets or sensitive data are templated here. Just be aware that the empty-host default (point 1) combined with ingress.enabled could expose the service more broadly than intended.

🧪 Test coverage

The PR checklist notes no automated tests were added. Since this is chart-only, a lightweight guard would help catch regressions — e.g. a helm lint + helm template --set ingress.enabled=true step in CI, or a rendered-manifest snapshot. I verified the template renders to valid YAML by hand for both enabled=false (no output) and enabled=true (valid Ingress) with the default values.

Overall this is a solid, conventional addition. Addressing the empty-host default (#1) is the main thing I'd recommend before merge.