Skip to content

refactor(admin): secrets and ssls#8911

Merged
spacewander merged 15 commits into
apache:masterfrom
An-DJ:refactor-secrets-and-ssls
Mar 1, 2023
Merged

refactor(admin): secrets and ssls#8911
spacewander merged 15 commits into
apache:masterfrom
An-DJ:refactor-secrets-and-ssls

Conversation

@An-DJ

@An-DJ An-DJ commented Feb 22, 2023

Copy link
Copy Markdown
Contributor

Description

Continue to review this work after #8820 is merged.

Refactor secrets and ssls.

All admin.resources is refactored.

Close #8569

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@An-DJ An-DJ changed the title Refactor secrets and ssls refactor(admin): secrets and ssls Feb 22, 2023
@An-DJ An-DJ changed the title refactor(admin): secrets and ssls refactor(admin): [WIP] secrets and ssls Feb 22, 2023
@An-DJ An-DJ changed the title refactor(admin): [WIP] secrets and ssls refactor(admin): secrets and ssls Feb 22, 2023
Comment thread apisix/admin/resource.lua
Comment thread apisix/admin/resource.lua Outdated
Comment thread apisix/admin/resource.lua Outdated
Comment thread apisix/admin/resource.lua


function _M:delete(id)
function _M:delete(id, conf, sub_path)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to pass conf in this method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spacewander We should Keep all methods in the same form.

https://github.com/apache/apisix/blob/master/apisix/admin/init.lua#L220

The meaningful first param is id, the second one is conf, and the third one is the sub_path.

Should we?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment that explains why we need a placeholder conf here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread apisix/admin/resource.lua Outdated
@An-DJ
An-DJ requested a review from spacewander February 28, 2023 15:31
@spacewander
spacewander merged commit 7d676f3 into apache:master Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: As a user, I want to refactor the resources under apisix.admin, so that we don't need to repeat

3 participants