Add use-cluster-ip annotation for ingress resources

[j1m-ryan]

Proposed changes

#4665

This adds the nginx.org/use-cluster-ip boolean annotation to the ingress resource. It defaults to false.

Manual Tests

• Works for an ingress that is neither minion nor master
  
• Is ingored when a master ingress is annotated with it

E0103 12:08:06.928220       1 ingress.go:619] Ingress Resource default/cafe-ingress-master with the annotation 'nginx.org/mergeable-ingress-type' set to 'master' cannot contain the 'nginx.org/use-cluster-ip' annotation(s). They will be ignored

• Does not follow minion inheritence
• Works for minions on the individual level (coffee minion is annotated with use-cluster-ip: true, but the tea minion is not below)

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

[netlify]

👷 Deploy request for nginx-kubernetes-ingress pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	ba94de9

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (85e15ec) 52.10% compared to head (838f3b7) 52.11%.
Report is 7 commits behind head on main.

❗ Current head 838f3b7 differs from pull request most recent head ba94de9. Consider uploading reports for the commit ba94de9 to get more accurate results

Files	Patch %	Lines
internal/configs/annotations.go	50.00%	1 Missing and 2 partials ⚠️
internal/configs/ingress.go	84.21%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4862   +/-   ##
=======================================
  Coverage   52.10%   52.11%           
=======================================
  Files          60       60           
  Lines       17376    17394   +18     
=======================================
+ Hits         9054     9065   +11     
- Misses       8008     8011    +3     
- Partials      314      318    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jjngx]

👍🏻

[pdabelf5]

@j1m-ryan has this been tested outside the default namespace?

[j1m-ryan]

@j1m-ryan has this been tested outside the default namespace?

I haven't, good catch @pdabelf5 , I'll do this now

[btsuhako]

this is a welcomed feature, we're planning to use this once it's released.

for context in the issue below, i'm using the edge release of the ingress controller with no Helm values overrides, everything is set to the default

we're doing some testing, and possibly found a bug. that or some documentation needs to be added

the following manifests work fine:

---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: podinfo
  namespace: podinfo
  annotations:
    nginx.org/use-cluster-ip: "True"
spec:
  ingressClassName: nginx
  rules:
  - host: localhost
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: podinfo
            port:
              number: 9898
---
apiVersion: v1
kind: Service
metadata:
  name: podinfo
  labels:
    helm.sh/chart: podinfo-6.5.3
    app.kubernetes.io/name: podinfo
    app.kubernetes.io/version: "6.5.3"
    app.kubernetes.io/managed-by: Helm
spec:
  type: ClusterIP
  ports:
    - port: 9898
      targetPort: http
      protocol: TCP
      name: http
    - port: 9999
      targetPort: grpc
      protocol: TCP
      name: grpc
  selector:
    app.kubernetes.io/name: podinfo

the following manifest does not work. note the comment in the backend.service.port part of the Ingress

---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: podinfo
  namespace: podinfo
  annotations:
    nginx.org/use-cluster-ip: "True"
spec:
  ingressClassName: nginx
  rules:
  - host: localhost
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: podinfo
            port:
              name: http # cannot use named port

it seems like the ingress controller won't generate a valid config with the above

$ k exec -it deploy/nginxinc-ingress-nginx-ingress-controller -c nginx-ingress -- nginx -T
2024/01/22 21:34:39 [emerg] 47#47: invalid port in upstream "podinfo.podinfo.svc.cluster.local:0" in /etc/nginx/conf.d/podinfo-podinfo.conf:3
nginx: [emerg] invalid port in upstream "podinfo.podinfo.svc.cluster.local:0" in /etc/nginx/conf.d/podinfo-podinfo.conf:3
nginx: configuration file /etc/nginx/nginx.conf test failed
command terminated with exit code 1

if it helps, the following does work:

---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: podinfo
  namespace: podinfo
  annotations:
    nginx.org/use-cluster-ip: "False" # use Pod IP
spec:
  ingressClassName: nginx
  rules:
  - host: localhost
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: podinfo
            port:
              name: http # named port now works

is this intentional behavior of the nginx.org/use-cluster-ip annotation or a bug? if it's a bug, let me know if I need to create an issue. It seems like something is lost when mapping the named port to container the port number when using the nginx.org/use-cluster-ip: "True" annotation

[j1m-ryan]

Hi @btsuhako, thank you for reporting this. I can see in the implementation that the name'd ports are not handled, and it defaults to number. I have replicated the error you experienced myself. I made an issue for it here #4970
Thanks again.

[brianehlert]

Does the trailing comment on the port line cause any error by itself?

[btsuhako]

Does the trailing comment on the port line cause any error by itself?

The error happens with and without the comment on that line. I added the comment to call out the difference from the working manifest