Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

allow customization of service http and https port names through helm #7318

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

arussellf5
Copy link
Contributor

Proposed changes

This change allows users to customize the names of the NIC service's http and https ports through a helm chart. It adds defaults which are the same as the previous hardcoded values in the helm chart, thereby ensuring backwards compatibility.

This change will enable users of NIC to more smoothly integrate with NGINXaaS's AKS functionality. Our load balancer for kubernetes discovers the upstream name and upstream type that it needs to update by parsing service port names, so we need to allow users of our service to easily customize port names.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@arussellf5 arussellf5 requested a review from a team as a code owner February 6, 2025 23:36
@github-actions github-actions bot added the helm_chart Pull requests that update the Helm Chart label Feb 6, 2025
@shaun-nx shaun-nx added the needs triage An issue that needs to be triaged label Feb 10, 2025
@puneetsarna
Copy link
Contributor

puneetsarna commented Feb 10, 2025

As an FYI: This unlocks a product use-case where we want NLK to integrate seemlesly with NIC in that, we don't want users making any changes to their configs outside of what's proposed in this MR (to set port names), and then NIC should just integrate with NLK and NGINXaaS. A lot of customers are very likely to already have NIC running in their environment and we want our stuff to just be plug and play.

@haywoodsh
Copy link
Contributor

@arussellf5
Copy link
Contributor Author

The helm schema should also be updated to reflect the change: https://github.com/nginx/kubernetes-ingress/blob/817956be61494e09d6c3357df458630306685ee0/charts/nginx-ingress/values.schema.json

Thanks @haywoodsh for catching this! I've updated the schema.

@puneetsarna
Copy link
Contributor

Is there any documentation update that we may want to do or does that get done as part of the changelog (for helm or the product in general)?

@haywoodsh
Copy link
Contributor

haywoodsh commented Feb 12, 2025

We also have this doc on our website documenting all the customizable Helm values but we are happy to update it ourselves after this PR is merged. https://github.com/nginx/kubernetes-ingress/blob/817956be61494e09d6c3357df458630306685ee0/site/content/installation/installing-nic/installation-with-helm.md#configuration

The product changelog should be automatically generated upon release

@arussellf5
Copy link
Contributor Author

@AlexFenlon @haywoodsh thanks for your reviews! Would one of you please merge this for me? I don't have the permission to do so.

@arussellf5 arussellf5 requested a review from a team as a code owner February 13, 2025 16:56
@github-actions github-actions bot added documentation Pull requests/issues for documentation dependencies Pull requests that update a dependency file go Pull requests that update Go code labels Feb 13, 2025
arussellf5 and others added 2 commits February 13, 2025 09:57
… chart

This change allows users to customize the names of the NIC service's http and https ports through a helm chart. It adds defaults which are the same as the previous hardcoded values in the helm chart, thereby ensuring backwards compatibility.

This change will enable users of NIC to more smoothly integrate with NGINXaaS's AKS functionality. Our load balancer for kubernetes discovers the upstream name and upstream type that it needs to update by parsing service port names, so we need to allow users of our service to easily customize port names.
@github-actions github-actions bot removed documentation Pull requests/issues for documentation dependencies Pull requests that update a dependency file go Pull requests that update Go code labels Feb 13, 2025
@haywoodsh haywoodsh enabled auto-merge (squash) February 13, 2025 17:51
@haywoodsh haywoodsh merged commit 3dc3616 into nginx:main Feb 13, 2025
134 of 152 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
helm_chart Pull requests that update the Helm Chart needs triage An issue that needs to be triaged
Projects
Status: Done 🚀
Development

Successfully merging this pull request may close these issues.

5 participants