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

Add API route to list org secrets #26485

Merged
merged 21 commits into from Aug 15, 2023
Merged

Add API route to list org secrets #26485

merged 21 commits into from Aug 15, 2023

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented Aug 14, 2023

  • Add a new function CountOrgSecrets in the file models/secret/secret.go
  • Add a new file modules/structs/secret.go
  • Add a new function ListActionsSecrets in the file routers/api/v1/api.go
  • Add a new file routers/api/v1/org/action.go
  • Add a new function listActionsSecrets in the file routers/api/v1/org/action.go

go-sdk: https://gitea.com/gitea/go-sdk/pulls/629

- Add a new function `CountOrgSecrets` in the file `models/secret/secret.go`
- Add a new file `modules/structs/secret.go`
- Add a new function `ListActionsSecrets` in the file `routers/api/v1/api.go`
- Add a new file `routers/api/v1/org/action.go`
- Add a new function `listActionsSecrets` in the file `routers/api/v1/org/action.go`

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 14, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 14, 2023
@appleboy appleboy changed the title feat: add new functions and files related to secrets management feat(secrets): add new functions and files related to secrets management Aug 14, 2023
models/secret/secret.go Outdated Show resolved Hide resolved
appleboy and others added 9 commits August 14, 2023 13:09
- Add a new API endpoint for listing an organization's actions secrets

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
- Add a new file `routers/api/v1/swagger/action.go`
- Add a new struct `swaggerResponseSecretList` in the `action.go` file
- Add a new file `templates/swagger/v1_json.tmpl`
- Modify the `v1_json.tmpl` file to include a new property `Secret` in the `properties` section of the `User` object
- Modify the `v1_json.tmpl` file to include a new definition `SecretList` in the `definitions` section

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
- Change the function name from `FindSecrets` to `CountSecrets` in `models/secret/secret.go`
- Change the function name from `CountOrgSecrets` to `CountSecrets` in `routers/api/v1/org/action.go`

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
routers/api/v1/api.go Outdated Show resolved Hide resolved
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@lunny lunny added type/enhancement An improvement of existing functionality modifies/api This PR adds API routes or modifies them labels Aug 14, 2023
@lunny lunny added this to the 1.21.0 milestone Aug 14, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 14, 2023

// ListActionsSecrets list an organization's actions secrets
func ListActionsSecrets(ctx *context.APIContext) {
// swagger:operation GET /orgs/{org}/actions/secrets organization orgListActionsSecrets
Copy link
Member

Choose a reason for hiding this comment

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

Erm… User secrets exist too already.

Copy link
Member

Choose a reason for hiding this comment

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

That and we were not sure if we keep them action-only.
I think it is better to omit the actions/ part from the URL.

Copy link
Member

Choose a reason for hiding this comment

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

Or do you explicitly only want to return the action secrets here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@delvh delvh Aug 14, 2023

Choose a reason for hiding this comment

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

So, what do we do in this case?
Deviate from the GitHub standard?
Add extra routes, and mark this one as deprecated with a comment only for compatibility with the GitHub API?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry if my previous response was unclear. Please provide more details or elaborate on what you want me to explain. Thank you.

Copy link
Member

Choose a reason for hiding this comment

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

  1. We try to be compatible with GitHub API where possible, so far so good
  2. I think this is the wrong URL for that route, I suggest /orgs/{org}/secrets//users/{user}/secrets instead

That leaves the question what to do with the missing URL.
I suggest adding three routes in this PR: The two mentioned above, and the one you already implemented.
However, as the third should only be used by existing scripts and nothing else, I recommend marking this route as // deprecated: true with a comment that it is only intended for GitHub API compatibility and that our routes are the other two above instead.

Copy link
Member

Choose a reason for hiding this comment

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

As I said in #24200 (comment) , we can provide other API routes for more purposes but share the data table.

So LGTM.

routers/api/v1/org/action.go Outdated Show resolved Hide resolved
modules/structs/secret.go Outdated Show resolved Hide resolved
@delvh delvh changed the title feat(secrets): add new functions and files related to secrets management Add API routes to list org/user secrets Aug 14, 2023
- Rename the `Created` field to `created_at` in the `Secret` struct

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
- Change the property name from "created" to "created_at" in the User object

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@wolfogre
Copy link
Member

If I read the code correctly, there is no route to list user secrets, only org secrets. I'm OK with that if we don't want to do that in this PR, but please update the title.

@appleboy appleboy changed the title Add API routes to list org/user secrets Add API routes to list org secrets Aug 15, 2023
@appleboy
Copy link
Member Author

appleboy commented Aug 15, 2023

@wolfogre done for changed the PR title.

…n status

- Remove unnecessary code for checking organization membership and admin status in `ListActionsSecrets` function

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 15, 2023
@delvh delvh changed the title Add API routes to list org secrets Add API route to list org secrets Aug 15, 2023
@appleboy
Copy link
Member Author

@delvh any feedback? or we can merge the PR.

@delvh
Copy link
Member

delvh commented Aug 15, 2023

No block from my side, if we want to be compatible with the GitHub API.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 15, 2023
@silverwind silverwind merged commit 79d74d2 into go-gitea:main Aug 15, 2023
24 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 15, 2023
@appleboy appleboy deleted the api branch August 15, 2023 12:56
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 17, 2023
* upstream/main:
  Sync repo's IsEmpty status correctly (go-gitea#26517)
  [skip ci] Updated translations via Crowdin
  Remove fomantic transition module (go-gitea#26469)
  Explain SearchOptions and fix ToSearchOptions (go-gitea#26542)
  Update go dependencies (go-gitea#26534)
  Differentiate better between user settings and admin settings (go-gitea#26538)
  Add missing triggers to update issue indexer (go-gitea#26539)
  Improve deadline icon location in milestone list page (go-gitea#26532)
  Use unique class for breadcrumb divider (go-gitea#26524)
  Fix typo of RunerOwnerID (go-gitea#26508)
  Improve clickable area in repo action view page (go-gitea#26115)
  Fix dark theme highlight for "NameNamespace" (go-gitea#26519)
  Remove duplicate CSS import for chroma/base.css (go-gitea#26523)
  Fix project filter bugs (go-gitea#26490)
  Fix display problems of members and teams unit (go-gitea#26363)
  Use `hidden` over `clip` for text truncation (go-gitea#26520)
  Add API route to list org secrets (go-gitea#26485)
  Set "type=button" for editor's toolbar buttons (go-gitea#26510)
  Apply to become a maintainer (go-gitea#26514)
  Detect ogg mime-type as audio or video (go-gitea#26494)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Nov 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants