-
Notifications
You must be signed in to change notification settings - Fork 287
Add appProtocol to cluster service definition #999
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Tom Bojer <[email protected]>
2357351
to
1ef284b
Compare
Is similar but it needs to be set in all services that point to the cluster to make it work. |
Adding @rootxrishabh to please take a look. |
@tombojer, as I understand, hard coding I think once we fix #959, we can change the port name in a major release. |
In our clusters, we use a service mesh to handle mTLS between the pods transparently. Running the OpenSearch cluster in HTTP mode would actually provide us with more visibility into the traffic, which can be quite helpful for debugging and observability. That said, since I'm not sure whether #959 is fixable, this is sufficient to get OpenSearch running, especially if the rest of the community is aligned on using HTTPS. |
We should explore the setting @tombojer can you please check if we do this via CRD to define the cluster protocol? |
Hey, |
Got it, looks like this PR #1000 should solve that and pass the security changes via |
If the protocol can be configured via additionalConfig, is there a way for the operator to parse that and adjust service settings (like ports, probes, etc.) accordingly? Or should all relevant settings be explicitly exposed via the CRD instead? For example, this issue would still persist unless we can either disable TLS in the CRD or update the health check behavior. |
Description
As discussed in issue #958, the
appProtocol: "https"
must be included to ensure compatibility with service meshes such as Istio.Regarding the port name change in the service definition, this could be addressed in a future major release. However, if required, I can modify the pull request to reflect this change directly. That said, changing the port name now may break existing ingresses that reference the port by name.
As highlighted in issue #959, it is possible for the service to only listen on HTTP. Hardcoding the
appProtocol
tohttps
would therefore break compatibility with service meshes in this scenario. I would appreciate some guidance on whether a global approach would be more suitable—one that accounts for services where the cluster might expose either HTTP or HTTPS. This would need to include health checks and any other components that need to determine the protocol being used by the cluster.For testing, I created a KIND cluster, installed Istio, and applied the CR from the example:
Additionally, an Istio-specific
DestinationRule
is required since the OpenSearch cluster uses a self-signed certificate:Now we are able to curl the the OpenSearch cluster.
Issues Resolved
Fixes #751
Fixes #958
Check List
make lint
)If CRDs are changed:
make manifests
) and also copied into the helm chartPlease refer to the PR guidelines before submitting this pull request.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.