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

refactor(admin): secrets and ssls #8911

Merged
merged 15 commits into from
Mar 1, 2023

Conversation

An-DJ
Copy link
Contributor

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

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
apisix/admin/resource.lua Show resolved Hide resolved
apisix/admin/resource.lua Outdated Show resolved Hide resolved
apisix/admin/resource.lua Outdated Show resolved Hide resolved
@@ -148,23 +233,36 @@ function _M:put(id, conf, sub_path, args)
end


function _M:delete(id)
function _M:delete(id, conf, sub_path)
Copy link
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
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
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
Contributor Author

Choose a reason for hiding this comment

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

Done.

apisix/admin/resource.lua Outdated Show resolved Hide resolved
@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