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

Build plugin form according to its jsonschema #1039

Closed
juzhiyuan opened this issue Dec 13, 2020 · 26 comments
Closed

Build plugin form according to its jsonschema #1039

juzhiyuan opened this issue Dec 13, 2020 · 26 comments
Labels

Comments

@juzhiyuan
Copy link
Member

juzhiyuan commented Dec 13, 2020

Feature request

Please describe your feature

It's more convenient to use a form instead of a code mirror to set up plugins, we used the react-jsonschema-form but most of the plugins couldn't be built perfectly.

important issues

@juzhiyuan
Copy link
Member Author

Maybe we could use form-render[1] to build UI first, then export JSON schema, and update the plugin's schema finally?

[1] https://x-render.gitee.io/schema-generator/playground

cc @liuxiran @imjoey

@juzhiyuan juzhiyuan removed the good first issue Good for newcomers label Jan 24, 2021
@liuxiran
Copy link
Contributor

Maybe we could use form-render[1] to build UI first, then export JSON schema, and update the plugin's schema finally?

[1] https://x-render.gitee.io/schema-generator/playground

cc @liuxiran @imjoey

What a cool component library~!

Recently, during my approach to #1241 , I rethought this issue:

  1. build plugin form: (I'm ready to give up this plan)

    A: customized plugin form: Instead of build form from its Jsonschema, a customized plugin form can tell user what each item is and how to fill it better, but it comes with high maintenance costs
    just take proxy-rewrite for example
    image

    B: build form from its jsonschema: someone like request-validation which need to define the schema, could not be build correctly, even if we have already found this cool library. Besides, we need to ask for a more friendly definition of plugin schema in APISIX(e.g: define description for each field item)

  2. continue to optimize our codemirror: In fact, users only need to know intuitively what configuration items the plugin has what the meaning of each config item and how to configure them, no matter the plugin is build with a form or a codemirror config.

The biggest advantage of direct configuration is full coverage of plugin function, we only need to improve its user experience.
I've experienced the config of plugin in Aliyun API Gateway, which also use directly config, I think there are a few things we can refer to:
image

A: Several configuration templates are available, this is very helpful for most users, it will save the cost of study.
B: Each configuration item is described in detail, it tells user how to configure directly
C: The help document is shown in a modal, this makes it easier for users to copy and paste configuration items

cc @juzhiyuan

@juzhiyuan
Copy link
Member Author

Yep!

Plugin Configuration Template is possible to implement, I also prefer this way.

Besides, I will still try to build UI first, then export JSONSchema. 🤣🤣

@imjoey
Copy link
Member

imjoey commented Jan 26, 2021

@juzhiyuan @liuxiran

yep, inspired by Aliyun API gateway, we could support both configuration template and UI form for users. Let us get this done to improve the plugin experience. 😄

@liuxiran
Copy link
Contributor

@juzhiyuan @liuxiran

yep, inspired by Aliyun API gateway, we could support both configuration template and UI form for users. Let us get this done to improve the plugin experience.

ok, I'll look into what configuration template involves, and then split them into points to discuss and confirm one by one.

@juzhiyuan
Copy link
Member Author

Could we store those templates in a JSON or YAML file in Dashboard repo haha?

@liuxiran
Copy link
Contributor

liuxiran commented Jan 26, 2021

Could we store those templates in a JSON or YAML file in Dashboard repo haha?

These templates would be better to be a part of part of the APISIX plugin document, and dashboard obtained by synchronizing data(if possible)

@juzhiyuan
Copy link
Member Author

maybe Apache APISIX will not store those data 😂

@imjoey
Copy link
Member

imjoey commented Jan 26, 2021

Could we store those templates in a JSON or YAML file in Dashboard repo haha?

These templates would be better to be a part of part of the APISIX plugin document, and dashboard obtained by synchronizing data(if possible)

@LiteSun @juzhiyuan with discussion with @liuxiran , we guess the template could directly reuse the schema definition in APISIX, as well as extending the schemas by adding more examples and comments. We could call this The self-explanation API is right the doc and the template. We will then commit a proposal for the detailed framework. Looking forward to your insights.

@juzhiyuan
Copy link
Member Author

OK!! Looking forward to it!

@imjoey
Copy link
Member

imjoey commented Jan 26, 2021

maybe Apache APISIX will not store those data 😂

yep, there is something that we need to reach a conclusion with APISIX. At least, we could add more comments for each field in APISIX schema. 😈

@liuxiran
Copy link
Contributor

While researching this issue, I found a very cool library: json-schema-faker[1], an OSS with a large number of references and stars, used to generate fake data according to its jsonshema. I tested some of our plugins' schema, no matter which draft the schema is, the fake data can cover well.

What the fake data looks like

Take api-breaker for an example:

{
	"break_response_code": 200,
	"healthy": {
		"http_statuses": [200],
		"successes": 3
	},
	"max_breaker_sec": 300,
	"unhealthy": {
		"failures": 3,
		"http_statuses": [500]
	},
	"disable": false
}

How to add comments

The fake data generated by json-schema-faker will be the template of the plugins. When enable a plugin, we could show this template instead of empty fields.

If we want to add comments for the template, we have to use yaml instead of json.

It's such a coincidence that there is an issue[2] in json-schema-faker accurately matches our needs, I tried the method provided by the author, it worked very well

  • add comments for api-breaker: please pay attention to title and deacription fields
{
	"title": "plugin: api-breaker",
	"description": "some information about api-breaker",
	"$comment": "this is a mark for our injected plugin schema",
	"properties": {
                ...
		"break_response_code": {
                        "title": "break_response_code title",
			"maximum": 599,
			"minimum": 200,
			"type": "integer",
			"description": "comment for break_response_code"
		},
		"healthy": {
			"default": {
				"http_statuses": [200],
				"successes": 3
			},
			"properties": {
				"http_statuses": {
					"default": [200],
					"items": {
						"maximum": 499,
						"minimum": 200,
						"type": "integer"
					},
					"minItems": 1,
					"type": "array",
					"uniqueItems": true,
					"description": "comment for http_statuses"
				},
				"successes": {
					"default": 3,
					"minimum": 1,
					"type": "integer",
					"description": "comment for successes"
				}
			},
			"type": "object"
		},
		
		
	},
	"required": ["break_response_code"],
	"type": "object"
}

the yaml data with comments

## plugin: api-breaker
#
# some information about api-breaker


## break_response_code title ##
#
# comment for break_response_code
#
# Minimum value: 200
#
# Maximum value: 599
#
break_response_code: 200

## healthy ##
#
# Default value: [object Object]
#
healthy:
  
  ## http_statuses ##
  #
  # comment for http_statuses
  #
  # Default value: 200
  #
  http_statuses:
    - 200

  ## successes ##
  #
  # comment for successes
  #
  # Default value: 3
  #
  # Minimum value: 1
  #
  successes: 3

## max_breaker_sec ##
#
# comment for max_breaker_sec
#
# Default value: 300
#
# Minimum value: 3
#
max_breaker_sec: 300

## unhealthy ##
#
# Default value: [object Object]
#
unhealthy:
  
  ## failures ##
  #
  # comment for unhealthy failures
  #
  # Default value: 3
  #
  # Minimum value: 1
  #
  failures: 3

  ## http_statuses ##
  #
  # comment for unhealthy http_statuses
  #
  # Default value: 500
  #
  http_statuses:
    - 500

## disable ##
#
# comment for disable
#
disable: false

What we planed to do

  1. Import json-schema-faker to generated fake data when enable a plugin, so that we can have a default config data for plugin instead of an empty field.
  2. Add title description for each field of each plugin, this will be the source of the fake data comments.
  3. Import the method of generated comments for fake data, and at the same time, we need to change the config data from json to yaml
  4. Supplement plugin instructions to increase the configuration of common scenarios

Looking forward to your insights @juzhiyuan @LiteSun @imjoey

reference:
[1]: https://github.com/json-schema-faker/json-schema-faker
[2]: json-schema-faker/json-schema-faker#609

@imjoey
Copy link
Member

imjoey commented Feb 1, 2021

This is very impressive and we love this solution for:

  • Less is more

This does not need any extra configurations or templates to be stored in manager-api.

  • Code is documentation

Since then, the plugin schemas defined in APISIX will be also the documentation of all plugins, current Admin-API could also benefit from this.

  • YAML is much more user-friendly for inline editing than JSON

JSON is good for data exchange among applications, but not for human editing (Dealing with double quote is a nightmare). In contrast, YAML is better for both reading and editing.

@juzhiyuan @LiteSun @membphis What's your opinions? Any feedback is welcome 😄 . Thanks.

@liuxiran
Copy link
Contributor

liuxiran commented Feb 2, 2021

I talked about this issue with @juzhiyuan offline and we have reached the following consensus:

  1. To improve the user experience, we would add plugin configuration templates, which are a range of configurations based on the different use scenario, in the templates, we should add instructions for each configuration item at the same time.

where would these configuration template stored?
Stored in files would be better. Create a template file for each plugin under something called doc/plugin_templates dir in dashboard
what is the template file type?
The type of the template file can be markdown or yaml. yaml would be easier to parse
what would the template look like?
This needs further discussion. may be we can use follows:
take limit-conn for an example

scenario1:                   # limit concurrency per client's IP
   conn: 1                   # the maximum number of concurrent requests allowed
   burst: 0                  # the number of excessive concurrent requests (or connections) allowed to be delayed
   default_conn_delay: 0.1  # the default processing latency of a typical connection
   rejected_code: 503       # returned when the request exceeds conn + burst will be rejected, default is 503
   key: "remote_addr"       # limit the concurrency level

scenario2:                   # limit concurrency per server's IP
   conn: 1                   # the maximum number of concurrent requests allowed
   burst: 0                  # the number of excessive concurrent requests (or connections) allowed to be delayed
   default_conn_delay: 0.1  # the default processing latency of a typical connection
   rejected_code: 503       # returned when the request exceeds conn + burst will be rejected, default is 503
   key: "server_addr"       # limit the concurrency level
...
  1. To support instructions in the configuration templates, we should use yaml instead of json in dashboard plugin codemirror.
  2. in the plugin config page, we would add a select box to select configuration template, and show the template configuration in the codemirror:

image

Welcome any suggestions to improve the program

cc @imjoey @LiteSun @membphis

@liuxiran
Copy link
Contributor

liuxiran commented Feb 20, 2021

The implement of #1039 (comment) will be divided into four parts:

  • FE: support yaml to config plugin in plugin page, json at the same time will still be available.
  • DOC: add plugin templates, in fact this step is the most important part and will be a long term target
  • BE: manager-api provides api to get plugin templates
  • FE: support choose template before config the plugin

we will try to do it step by step, welcome any suggestions to improve the program, thanks very much~

cc @juzhiyuan @LiteSun @imjoey @membphis

@juzhiyuan
Copy link
Member Author

Great! 😉

@membphis
Copy link
Member

LGTM @liuxiran

@nic-chen
Copy link
Member

I have a little doubt, where does the configuration template come from? And how do we maintain them. Thanks. @liuxiran

@liuxiran
Copy link
Contributor

I have a little doubt, where does the configuration template come from? And how do we maintain them. Thanks. @liuxiran

The configuration template plans to define manually in a new dir e.g: plugin_templates in dashboard repo, and we have to maintain them when the configuration rules of the plugin changed.

The template would contain three parts:

  1. usage scenario, which means if users want to use plugin in scenarioA, the can refer scenarioA's template to complete their configuration.
    Just take limit-count for example:
    image

cluster-level precision traffic limit is one of the scenarios for limit-count plugin. we want to extract the common usage scenarios indicated in the document to generate different template.
For some plugins only have one usage scenario, then we would support something called Universal configuration template for them.
Maybe there are no specific usage scenarios labeled in the document for some plugins, then we have to add them for better use.

  1. description of the parameters, which means add comments for every config item, user can config the plugin refer the comments instead of opening the doc page.

  2. default value of the parameters, which means the recommended config(or recommended config policy) for the config item in the target usage scenario

@membphis
Copy link
Member

ping @nic-chen

@nic-chen
Copy link
Member

@liuxiran
Thanks for the explanation.
Maintaining a separate template is troublesome, and after the plugin schema is updated in APISIX, it may be easier to miss here and cannot keep up with the update of the plugin schema.

@liuxiran
Copy link
Contributor

@liuxiran
Thanks for the explanation.
Maintaining a separate template is troublesome, and after the plugin schema is updated in APISIX, it may be easier to miss here and cannot keep up with the update of the plugin schema.

Thanks @nic-chen, and your concern is really a problem indeed.
We originally envisioned automatically generating templates through schema, and we got a scheme to generate template stochasticly, but it can not support to generate template refer to the usage scenario. It's not very friendly for plugin with options.

It would be better to find a way to generate template reder to the different usage scenario, let me try to find it.

@nic-chen
Copy link
Member

Thanks @nic-chen, and your concern is really a problem indeed.
We originally envisioned automatically generating templates through schema, and we got a scheme to generate template stochasticly, but it can not support to generate template refer to the usage scenario. It's not very friendly for plugin with options.

It would be better to find a way to generate template reder to the different usage scenario, let me try to find it.

Look forward to it, and thanks for that.

@fregie
Copy link
Contributor

fregie commented Sep 2, 2021

I think use form to config plugins is the best solution.And we shuold also support to config custom plugins by from,not json or yaml.
And I don't think using template is a good idea, form and annotation will be better.
All schema shuold be defined in plugin itself,that is the plugin_name.lua in apisix, all dashboard need to do is get the schema by apisix, and parse it in form and annotation to config.
Another thing is, dashboard get the schema by apisix control api or etcd(apisix can save the plugin infomation in etcd,but we need consider the info version of plugin)
@liuxiran @nic-chen

@github-actions
Copy link

This issue has been marked as stale due to 30 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 14, 2021
@github-actions
Copy link

This issue has been closed due to lack of activity. If you think that is incorrect, or the issue requires additional review, you can revive the issue at any time.

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

No branches or pull requests

6 participants