apinf / platform

Apinf - Open source API management platform with multi proxy and protocol support
https://apinf.com/
European Union Public License 1.1
74 stars 33 forks source link

Drop URL validation on ElasticSearch host in proxy settings #2074

Closed shaliko closed 7 years ago

shaliko commented 7 years ago

For docker-compose and OpenShift/Kubernetes deploy we have a hack for able use Elasticsearch URL.

For OpenShift/Kubernetes (or cloud services) Elasticsearch can be available by host name (not valid URL) like "elasticsearch".

apinf_-_api_management_platform

Suggestion: Drop URL validation and allow user use url safe string as hostname.

Examples:

"elasticsearch" "elasticsearch-2-4" "elastic-search"

shaliko commented 7 years ago

After finish that story we can drop hack from docker file https://github.com/apinf/platform/blob/develop/docker-compose.yml#L14-L15.

brylie commented 7 years ago

Some quick notes/questions,

  1. can't the internal elasticsearch url be structured to pass regex validation? E.g. http://elasticsearch.local?
    • in other words, is it the protocol in the string that is breaking things, or something else?
  2. we are currently refactoring our codebase, so that the Elasticsearch URL will be irrelevant ASAP (#2036)
shaliko commented 7 years ago
  1. Sometimes you can't use dot symbol (OpenShift). openshift_web_console
  2. If that in near feature, my request is not relevant.
bajiat commented 7 years ago

@shaliko #2036 will not be implemented this sprint (36), I have only asked for a research on the changes required. Depending on the amount of changes or whether any current functitonality is lost, I am considering sprint 37.

brylie commented 7 years ago

It is also worth considering how the API Umbrella URL field will be affected by OpenShift deployment.

Issue #2036 is opened in order to remove the Elasticsearch url, which will mean we communicate directly with API Umbrella for analytics data. Regardless of when #2036 is resolved, it is our planned trajectory.

brylie commented 7 years ago

We can easily remove the RegEx validation from any URL field, but I just want to understand the issue and explore alternatives (in context of our development roadmap).

brylie commented 7 years ago

E.g. it may be possible to specify a different OpenShift hostname, which would possible resolve this issue:

shaliko commented 7 years ago

"API Umbrella URL" because we still need access to API-Umbrella after setup for getting admin API-keys, we will need to assign regular domain name for API-Umbrella.

The issue can happen for internal services (ElasticSearch) because they should be accessible only from a private network by hostname instead of domains.

In future, if we will able get Admin API keys from "API Umbrella" without visit it from browser and do it only from Platform UI, we will have the same issue. Because "API Umbrella" will be only in private network by name like "api-umbrella".

shaliko commented 7 years ago

@brylie Your link for the settings of setup OpenShift hostname (admin panel), not for services run on openshift (docker images like platform, api-umbrella). That not related to our issue.

brylie commented 7 years ago

In order for services to be exposed externally, an OpenShift Container Platform route allows you to associate a service with an externally-reachable host name.

OpenShift Core Concepts: Route Host Names

The first sentence of that section seems to be talking about services run on OpenShift.

brylie commented 7 years ago

I just think this issue is not urgent, although I can understand why it seems to be a high priority. Rather than take up the issue right away, I am simply recommending we wait until #2036 progresses.

bajiat commented 7 years ago

Can I place this issue in icebox until we find out what will happen with #2036? The current understanding is that the data available through the admin API is really limited and would not serve our dashboard. Either we would need to enhance the admin API or we need to continue using ElasticSearch.

Or will placing this in icebox and waiting mean problems with any OpenShift experiments?

shaliko commented 7 years ago

I am OK with a move that issue to the icebox, fix #2036 will make not relevant that issue.

marla-singer commented 7 years ago

@bajiat Interested

bajiat commented 7 years ago

@brylie wanted have a discussion about this need in the Github issue first

bajiat commented 7 years ago

@marla-singer Can you make the change?

marla-singer commented 7 years ago

@bajiat Yes, I can

marla-singer commented 7 years ago

@frenchbread Now emq scheme has the regex for ElasticSearch field as well. Should it be removed?

frenchbread commented 7 years ago

@marla-singer Yep, similarly to api-umbrella