Skip to content

Conversation

@MingzhouLin
Copy link

@MingzhouLin MingzhouLin commented Nov 9, 2025

Fixes: #3017

Background: I want to expose extra services(management service for example) through ingress, but there is no way to do so by only editing the configuration.

So, I add this PR to allow user add services through config file.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • N/A 💡 Added comments for complex logic
  • N/A 🧾 Updated CHANGELOG.md (if needed)
  • N/A 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

@snazy
Copy link
Member

snazy commented Nov 10, 2025

See #3017 (comment)

Comment on lines 53 to 61
{{- range .paths }}
- path: {{ .path }}
pathType: {{ .pathType }}
backend:
service:
name: {{ $fullName }}
name: {{ default $fullName .backend.service.name }}
port:
number: {{ $svcPort }}
number: {{ default $svcPort .backend.service.port }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, shouldn't we just copy the entire .paths here?

Suggested change
{{- end }}
{{ tpl (toYaml .paths) $ | nindent 10 }}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! updated

@adutra
Copy link
Contributor

adutra commented Nov 10, 2025

@MingzhouLin I think #3017 is quite debatable, but I wouldn't mind repurposing this PR to allow people to enter the whole path object rather than just portions of it.

port:
number: {{ $svcPort }}
{{- end }}
{{ tpl (toYaml .paths) $ | nindent 10 }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MingzhouLin I think I didn't really think about the consequences of my suggestion, but looking at the result, it seems to me that it's not fair to ask users to input the whole path object, if they don't need to.

One of the issues is that now users must input the service name (backend.service.name) and port (backend.service.port) – but these fields have good default values.

Would you be OK for you if we do this instead:

          {{- range .paths }}
          - path: {{ .path }}
            pathType: {{ .pathType }}
            {{- if .backend }}
            backend:
              {{- tpl (toYaml .backend) $ | nindent 14 }}
            {{- else }}
            backend:
              service:
                name: {{ $fullName }}
                port:
                  number: {{ $svcPort }}
            {{- end }}
          {{- end }}

In the above example, if you customize backend it will be used, otherwise a default backend is generated.

Sorry for the back and forth 😬

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.

Can't add additional backend services to Helm chart ingress

3 participants