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

Feature request: support FQDN in upstream #2080

Closed
liuxiran opened this issue Aug 19, 2021 · 17 comments · Fixed by #2118
Closed

Feature request: support FQDN in upstream #2080

liuxiran opened this issue Aug 19, 2021 · 17 comments · Fixed by #2118
Labels
enhancement New feature or request
Milestone

Comments

@liuxiran
Copy link
Contributor

Feature request

Please describe your feature

Currently we can only configure upstream nodes in UI by Address + (required) Port, but in fact APISIX also support FQDN, see https://apisix.apache.org/docs/apisix/2.7/FAQ/#does-the-upstream-node-support-configuring-the-fqdn-address, we should update our UI to fit this option.

Describe the solution you'd like

We can update our input constraints, make port an optional input, and at the same time we can also consider whether to change host to the suitable name.

@liuxiran liuxiran added this to the 2.7.2 milestone Aug 19, 2021
@juzhiyuan juzhiyuan added the enhancement New feature or request label Aug 19, 2021
@bzp2010
Copy link
Contributor

bzp2010 commented Aug 19, 2021

Hi, @liuxiran.
It seems that the dashboard can now support host input in the form of FQDN. What's the problem?
image
image

@liuxiran
Copy link
Contributor Author

Hi, @liuxiran.
It seems that the dashboard can now support host input in the form of FQDN. What's the problem?
image

refer to https://github.com/apache/apisix/blob/release/2.7/docs/en/latest/FAQ.md#does-the-upstream-node-support-configuring-the-fqdn-address

when we use FQDN, we do not need to enter Port, so fe need to make Port an optional field.

Please let me know if I have misunderstood, thanks @bzp2010

cc @nic-chen @tao12345666333

@bzp2010
Copy link
Contributor

bzp2010 commented Aug 20, 2021

I think at the same time we can give it a default value of port 80. XD

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 1, 2021

Hi @foolwc , do you have time to take this issue?

@foolwc
Copy link
Contributor

foolwc commented Sep 1, 2021

I'm willing to take. Just make the Port has a default value and optional ? @liuxiran

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 1, 2021

I'm willing to take. Just make the Port has a default value and optional ? @liuxiran

thanks @foolwc , assign to you. Only need to make the Port optional, to avoid misuse, we can keep Port field empty as default

@foolwc
Copy link
Contributor

foolwc commented Sep 1, 2021

The upstream JSON schema shows that Port must be an Integer. And if we support FQDN, the nodes will be an Object but not an Array. So I'm confused how to deal with it. Should we give the port a default value? Please give me some lint. Thanks a lot @liuxiran

curl http://127.0.0.1:9080/apisix/admin/routes/1  -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/ip",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "httpbin.default.svc.cluster.local": 1
        }
    }
}'

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 1, 2021

nodes: {
	"anyOf": [{
		"patternProperties": {
			".*": {
				"description": "weight of node",
				"minimum": 0,
				"type": "integer"
			}
		},
		"type": "object"
	}, {
		"items": {
			"properties": {
				"host": {
					"pattern": "^\\*?[0-9a-zA-Z-._]+$",
					"type": "string"
				},
				"metadata": {
					"description": "metadata of node",
					"type": "object"
				},
				"port": {
					"description": "port of node",
					"minimum": 1,
					"type": "integer"
				},
				"priority": {
					"default": 0,
					"description": "priority of node",
					"type": "integer"
				},
				"weight": {
					"description": "weight of node",
					"minimum": 0,
					"type": "integer"
				}
			},
			"required": ["host", "port", "weight"],
			"type": "object"
		},
		"type": "array"
	}]
}

Hi @foolwc here is our upstream nodes schema, you can see that we support two type of nodes:

  • currently we use the second one, which required host port and weight
  • when we use FQDN, we can switch to the first one, that is:
{
    ${host1}: ${weight1},
    ${host2}: ${weight2},
    ...
}

@foolwc
Copy link
Contributor

foolwc commented Sep 2, 2021

There is a case: if we add two hosts, one is FQDN without port, the other is normal IP with port. How to deal with that, saved with Array or Object just like below ? Thanks for your explanation @liuxiran

{
     "httpbin.default.svc.cluster.local": 1
     "httpbin2.default.svc.cluster.local:8080": 1
}

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 2, 2021

{
"httpbin.default.svc.cluster.local": 1
"httpbin2.default.svc.cluster.local:8080": 1
}

Object is good for me, @tzssangglass @nic-chen for double check , thanks

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 2, 2021

{
"httpbin.default.svc.cluster.local": 1
"httpbin2.default.svc.cluster.local:8080": 1
}

Object is good for me, @tzssangglass @nic-chen for double check , thanks

Hi @foolwc please use object as the data struct, no matter enter IP:port or host

{
    `${IP/host}${: port?}`: ${weight}
}

any question please comment here, thanks

@nic-chen
Copy link
Member

nic-chen commented Sep 2, 2021

{
"httpbin.default.svc.cluster.local": 1
"httpbin2.default.svc.cluster.local:8080": 1
}

Object is good for me, @tzssangglass @nic-chen for double check , thanks

LGTM

@tzssangglass
Copy link
Member

LGTM

@starsz
Copy link
Contributor

starsz commented Sep 2, 2021

{
"httpbin.default.svc.cluster.local": 1
"httpbin2.default.svc.cluster.local:8080": 1
}

Object is good for me, @tzssangglass @nic-chen for double check , thanks

Hi @foolwc please use object as the data struct, no matter enter IP:port or host

{
    `${IP/host}${: port?}`: ${weight}
}

any question please comment here, thanks

LGTM.

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 2, 2021

Hi @foolwc , do you still have any questions on this issue? or anything else I can do to help?

@foolwc
Copy link
Contributor

foolwc commented Sep 2, 2021

No question, thanks. It's ok to assign to others if this feature is urgent or someone else can do :) I don't have much time on weekdays and maybe delay your work

@liuxiran
Copy link
Contributor Author

liuxiran commented Sep 2, 2021

No question, thanks. It's ok to assign to others if this feature is urgent or someone else can do :) I don't have much time on weekdays and maybe delay your work

Thanks very much for your information, since it is one of our milestone task, and tomorrow is the deadline, I 'll assign to others, we have other issues you can try the one you want to, thanks again to drive this issue needs to be clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants