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

plugin(hmac-auth): update the schema #2441

Closed
liuxiran opened this issue Oct 16, 2020 · 12 comments · Fixed by #2509
Closed

plugin(hmac-auth): update the schema #2441

liuxiran opened this issue Oct 16, 2020 · 12 comments · Fixed by #2509
Assignees
Labels

Comments

@liuxiran
Copy link
Contributor

Part of #2082

Current schema

{
	"additionalProperties": false,
	"properties": {
		"disable": {
			"type": "boolean"
		}
	},
	"title": "work with route or service object",
	"type": "object"
}

the plugin page looks:
2020-10-16 20-39-49屏幕截图


Expected schema

according to the doc and test case

{
  "type": "object",
  "required": [
    "access_key",
    "secret_key"
  ],
  "properties": {
    "access_key": {
      "type": "string",
      "title": "Accesskey"
    },
    "secret_key": {
      "type": "string",
      "title": "Secretkey"
    },
    "algorithm": {
      "type": "string",
      "enum": [
        "hmac-sha1",
        "hmac-sha256",
        "hmac-sha512"
      ],
      "default": "hmac-sha256",
      "title": "Algorithm"
    },
    "clock_skew": {
      "type": "number",
      "title": "clockskew",
      "default": 0
    },
    "signed_headers": {
      "type": "array",
      "title": "A list of strings",
      "items": {
        "type": "string",
        "default": "bazinga"
      }
    }
  }
}

the form looks:

2020-10-16 21-06-48屏幕截图

@liuxiran
Copy link
Contributor Author

liuxiran commented Oct 17, 2020

hmac-auth define two schemas:

maybe this is the cause of the problem, and this case has already made part of the plugins unuseable in dashboard.

so is it possible to return differet schema depending on the different param or return all schemas that caller coulde Judge which one to use?

@juzhiyuan @moonming @membphis @spacewander

@juzhiyuan
Copy link
Member

@membphis Please take a look

@spacewander
Copy link
Member

I think we should pass the schema type to get the correct schema, since currently we already have three kinds of schema:

  1. metadata
  2. consumer
  3. route/service

@membphis
Copy link
Member

  • metadata
  • consumer

How about this API style?

/apisix/admin/schema/plugins/{plugin_name}/metadata
/apisix/admin/schema/plugins/{plugin_name}/consumer

@spacewander
Copy link
Member

My suggestion is to put metadata/consumer into the arguments, if the argument is missing, choice the default plain schema.

@imjoey
Copy link
Member

imjoey commented Oct 20, 2020

My suggestion is to put metadata/consumer into the arguments, if the argument is missing, choice the default plain schema.

+1 . IMHO, query parameters would be more flexible.

@membphis
Copy link
Member

My suggestion is to put metadata/consumer into the arguments, if the argument is missing, choice the default plain schema.

this way is much better. agree +1

@liuxiran
Copy link
Contributor Author

liuxiran commented Oct 22, 2020

ok, based on the results of the discussion above, I think this issue contains at least two modifications:

  • admin-api provides api like:

    • /apisix/admin/schema/plugins/{plugin_name} to get plugin schema
    • /apisix/admin/schema/plugins/{plugin_name}?schema_type=consumer_schema to get plugin consumer_schema
  • dashboard calls the corresponding api to get the correct schema.

e.g:

  1. consumer wants to config hmac-auth plugin, then call /apisix/admin/schema/plugins/hmac-auth?schema_type=consumer_schema to get the config schema

  2. route wants to config hmac-auth plugin, then call /apisix/admin/schema/plugins/hmac-auth to get the config schema

my partner and I will take this issue :)

@juzhiyuan
Copy link
Member

yep 😁

@membphis
Copy link
Member

/apisix/admin/schema/plugins/{plugin_name}?schema_type=consumer_schema

to

/apisix/admin/schema/plugins/{plugin_name}?schema_type=consumer

simpler

@liuxiran
Copy link
Contributor Author

/apisix/admin/schema/plugins/{plugin_name}?schema_type=consumer_schema

to

/apisix/admin/schema/plugins/{plugin_name}?schema_type=consumer

simpler

got it~!

@Jaycean
Copy link
Member

Jaycean commented Oct 23, 2020

Please assign to me

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

Successfully merging a pull request may close this issue.

6 participants