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

Add ingress resource, use ClusterIP by default #1562

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

lorantalas
Copy link
Contributor

Description

Set ClusterIP service type for Hub and Router components, NodePort could be set explicitly, but the default is ClusterIP.

Add enabled by default ingress resource, which exposes either Hub or Router service.
In the case of LoadBalancer or NodePort set explicitly, you shall turn off ingress explicitly as well.
To turn off: --set ingress.enabled=false

End-user shall supply the hostname for the ingress. There is an option to use an empty host instead.
Be aware of the consequences of accepting traffic from all hosts.

Motivation and Context

See issue #1546.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@CLAassistant
Copy link

CLAassistant commented May 4, 2022

CLA assistant check
All committers have signed the CLA.

@diemol diemol requested a review from ilpianista May 4, 2022 08:33
@den-is
Copy link
Contributor

den-is commented May 23, 2022

can you please add this little config to ingress object too:

{{- if .Values.ingress.ingressClassName }}
  ingressClassName: {{ .Values.ingress.ingressClassName }}
{{- end }}
  rules:
  ...

?

@diemol
Copy link
Member

diemol commented May 23, 2022

This needs to be rebased, also hoping that @ilpianista has some time to review it.

@lorantalas
Copy link
Contributor Author

@den-is I added an option to set IngressClassName, custom annotations, TLS, and some standard precautions against different Kubernetes API versions.

Set ClusterIP service type for Hub and Router components,
NodePort could be set explicitly, but the default is ClusterIP.

Add enabled by default ingress resource,
which exposes either Hub or Router service.
In the case of LoadBalancer or NodePort set explicitly,
you shall turn off ingress explicitly as well.
To turn off: --set ingress.enabled=false

End-user shall supply the hostname for the ingress.
There is an option to use an empty host instead.
Be aware of the consequences of accepting traffic from all hosts.

Fixes SeleniumHQ#1546
@lorantalas
Copy link
Contributor Author

This needs to be rebased, also hoping that @ilpianista has some time to review it.

@diemol I rebased. Awaiting for review.

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

I am going to merge this and release a new chart so we can use these changes, hopefully you can be around as my helm is a bit rusty, in case issues are reported.

Thank you, @lorantalas!

@diemol diemol merged commit 56bca52 into SeleniumHQ:trunk Jun 7, 2022
@lorantalas
Copy link
Contributor Author

@diemol if the issue is reported please ping me in the comments.

@lorantalas lorantalas mentioned this pull request Jun 7, 2022
6 tasks
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.

4 participants