Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

To use port number instead of port name #2059

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Apr 5, 2024

fixes: #1984

Objectives:

Current problem:

  • We are hardcoding service.port.name to be http only, necessitating patching.
  rules:
    - host: {{ .Values.console.ingress.host }}
      http:
        paths:
          - path: {{ .Values.console.ingress.path }}
            pathType: {{ .Values.console.ingress.pathType }}
            backend:
              service:
                name: "console"
                port:
                  name: http <------- here

Solution:

  • By utilizing service.port.number, we provide flexibility in port selection, enabling users to choose TLS on or off according to their requirements, without requiring them to patch our ingress.
Screenshot 2024-04-05 at 11 52 47 AM

Documentation:

A list of paths (for example, /testpath), each of which has an associated backend defined with a service.name and a service.port.name or service.port.number. Both the host and path must match the content of an incoming request before the load balancer directs traffic to the referenced Service.

Notice is either one or the other but not both: service.port.name or service.port.number

@cniackz cniackz changed the title use port number instead of name To use port number instead of port name Apr 5, 2024
@cniackz cniackz requested review from allanrogerr and ravindk89 April 5, 2024 14:40
@cniackz cniackz self-assigned this Apr 5, 2024
@cniackz cniackz added the bug fix label Apr 5, 2024
Copy link
Contributor

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

One clarifying comment

helm/operator/values.yaml Outdated Show resolved Hide resolved
@cniackz cniackz requested review from ravindk89 and jiuker April 5, 2024 15:47
@cniackz
Copy link
Contributor Author

cniackz commented Apr 5, 2024

Sorry I don't want to merge until all tests are passing

@cniackz
Copy link
Contributor Author

cniackz commented Apr 5, 2024

Tests will be fixed in: #2062

@cniackz cniackz merged commit f89370a into minio:master Apr 5, 2024
25 of 26 checks passed
@cniackz cniackz deleted the use-port-number branch April 5, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTPS not working for operator console when deployed with Helm chart
3 participants