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

Show secrets from org and global level #2873

Merged
merged 30 commits into from
Dec 16, 2023

Conversation

anbraten
Copy link
Member

closes #2844

Screenshots

org-level:
image

repo-level:
image

new image filter UI:
image

@anbraten anbraten added ui frontend related enhancement improve existing features labels Nov 26, 2023
@anbraten anbraten requested a review from a team November 26, 2023 02:08
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (ff1f51d) 33.86% compared to head (6e3f233) 33.86%.
Report is 2 commits behind head on main.

❗ Current head 6e3f233 differs from pull request most recent head ecbfaf2. Consider uploading reports for the commit ecbfaf2 to get more accurate results

Files Patch % Lines
server/model/secret.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2873      +/-   ##
==========================================
- Coverage   33.86%   33.86%   -0.01%     
==========================================
  Files         219      219              
  Lines       13856    13858       +2     
==========================================
  Hits         4693     4693              
- Misses       8805     8807       +2     
  Partials      358      358              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qwerty287
Copy link
Contributor

I don't really know why, but this PR breaks repo secrets completely for me.

  1. Creating a new repo secret fails with Error inserting secret "orga-sec-1". UNIQUE constraint failed: secrets.secret_org_id, secrets.secret_repo_id, secrets.secret_name
  2. However, it looks like the secret is created anyways. Checking out main and running there shows the secret.
  3. With this PR, the secret is not shown.

Looks like an issue with the org_id and repo_id cols in db (looking at these directly in DB they're set correctly though).
This is not what I wanted to test, but I can't try my scenarios as I can't create new repo secrets.

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

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

Yes, it seems to work in general now.
Some things I found:

  1. Secrets are sorted by their name (alphabetically) always, but on top it should show repos, then org, then global
    grafik
  2. When opening the repo settings from the gear on the repo header, only the first page of repo secrets is shown (that's a general pagination issue, can be fixed in another PR)

And some general feedback (I'd open a new PR for this):

  • show "user secret" instead of "organization secret" if the repo owner is a user
  • show global secrets on org secrets page

web/src/compositions/usePaginate.ts Outdated Show resolved Hide resolved
@anbraten
Copy link
Member Author

anbraten commented Dec 8, 2023

Sure we can sort them differently.

For the pagination we might consider a load more button in the future instead of the automatic scrolling part.

Showing user secret instead of org secret could be a bit tricky as we need to find out about that somehow, maybe we can by checking the fetched org.

@qwerty287
Copy link
Contributor

Showing user secret instead of org secret could be a bit tricky as we need to find out about that somehow, maybe we can by checking the fetched org.

Yes, that's why I wrote to do it in another PR. I think though we have the org ID of the user already (global var with useConfig()) so we can just check for it. But let's move this to another PR.

@anbraten
Copy link
Member Author

anbraten commented Dec 8, 2023

Yes, that's why I wrote to do it in another PR. I think though we have the org ID of the user already (global var with useConfig()) so we can just check for it. But let's move this to another PR.

But we need to check differently somehow as you can have admin rights to a private repo from someone else.

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

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

It's still not working as intended, it loads this order:

  1. repo, page 1
  2. repo, page 2
  3. org, page 1
  4. global, page 2

Repo has 3 repo secrets, no org secret and one global secret.

@anbraten
Copy link
Member Author

anbraten commented Dec 14, 2023

Some edge cases

  • repo has 3 pages, org 0 & global 0: fetch page 1,2,3 from repo, load empty page 1 from org, load empty page 1 from global
  • repo is empty, org has 3 pages and global 0: load empty page 1 from repo, fetch page 1,2,3 from org, ...
  • repo has 3 pages, org has a secret the same name as repo on page 1 which is on repo page 3: fetch 1,2,3 from repo, fetch page 1 from org and secret wont be shown as we already have it from repo, additional org secrets are shown

web/src/compositions/usePaginate.ts Outdated Show resolved Hide resolved
server/model/secret.go Outdated Show resolved Hide resolved
@qwerty287
Copy link
Contributor

It's working for me correctly now

@anbraten anbraten requested a review from a team December 16, 2023 08:26
@anbraten anbraten enabled auto-merge (squash) December 16, 2023 08:47
@anbraten anbraten merged commit 16803d6 into woodpecker-ci:main Dec 16, 2023
6 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Dec 16, 2023
1 task
6543 pushed a commit that referenced this pull request Dec 26, 2023
## [2.1.0](https://github.com/woodpecker-ci/woodpecker/releases/tag/2.1.0)
- 2023-12-26

### ✨ Features

- Add pull request closed event
[[#2684](#2684)]
- Add depends_on support for steps
[[#2771](#2771)]
- gitlab: support nested repos
[[#2981](#2981)]
- Support go plugins for forges and agent backends
[[#2751](#2751)]

### 📈 Enhancement

- Show default branch on top
[[#3019](#3019)]
- Support more addon types
[[#2984](#2984)]
- Hide PR tab if PRs are disabled
[[#3004](#3004)]
- Switch to ULID
[[#2986](#2986)]
- Ignore pipelines without config
[[#2949](#2949)]
- Link labels to input and select
[[#2974](#2974)]
- Register Agent with hostname
[[#2936](#2936)]
- Update slogan & logo
[[#2962](#2962)]
- Improve error handling when activating a repository
[[#2965](#2965)]
- Add check for storage where repo/org name is empty
[[#2968](#2968)]
- Update pipeline icons
[[#2783](#2783)]
- Kubernetes refactor
[[#2794](#2794)]
- Export changed files via builtin environment variables
[[#2935](#2935)]
- Show secrets from org and global level
[[#2873](#2873)]
- Only update pipelineStatus in one place
[[#2952](#2952)]
- Rename `engine` to `backend`
[[#2950](#2950)]
- Add linting for `log.Fatal()`
[[#2946](#2946)]
- Remove separate root path config
[[#2943](#2943)]
- init CI_COMMIT_TAG if commit ref is a tag
[[#2934](#2934)]
- Update go module path for major version 2
[[#2905](#2905)]
- Unify date/time dependencies
[[#2891](#2891)]
- Add linting for `any`
[[#2893](#2893)]
- Fix vite deprecations
[[#2885](#2885)]
- Migrate to Xormigrate
[[#2711](#2711)]
- Simple security context options (Kubernetes)
[[#2550](#2550)]
- Changes PullRequest Index to ForgeRemoteID type
[[#2823](#2823)]

### 🐛 Bug Fixes

- Hide queue visualization if nothing to show
[[#3003](#3003)]
- fix and lint swagger file
[[#3007](#3007)]
- Fix IPv6 host aliases for kubernetes
[[#2992](#2992)]
- Fix cli lint throwing error on warnings
[[#2995](#2995)]
- Fix static file caching
[[#2975](#2975)]
- Gitea driver: ignore GetOrg error if we get a valid user.
[[#2967](#2967)]
- feat(k8s): Add a port name to service definition
[[#2933](#2933)]
- Fix error container overflow
[[#2957](#2957)]
- ignore some errors on repairAllRepos
[[#2792](#2792)]
- Allow to restart pipelines that has warnings
[[#2939](#2939)]
- Fix skipped pipelines model
[[#2923](#2923)]
- fix: Add `backend_options` to service linter entry
[[#2930](#2930)]
- Fix flags added multiple times
[[#2914](#2914)]
- Fix schema validation with array syntax for clone and services
[[#2920](#2920)]
- Fix prometheus docs
[[#2919](#2919)]
- Fix podman agent container in v2
[[#2897](#2897)]
- Fix bitbucket org fetching
[[#2874](#2874)]
- Only deploy docs on `main`
[[#2892](#2892)]
- Fix pipeline-related environment
[[#2876](#2876)]
- Fix version check partially
[[#2871](#2871)]
- Fix unregistering agents when using agent tokens
[[#2870](#2870)]

### 📚 Documentation

- [Awesome Woodpecker] added yet another autoscaler
[[#3011](#3011)]
- Add cookbook blog and improve docs
[[#3002](#3002)]
- Replace multi-pipelines with workflows on docs frontpage
[[#2990](#2990)]
- Update README badges
[[#2956](#2956)]
- Update 20-kubernetes.md
[[#2927](#2927)]
- Add release documentation to CONTRIBUTING
[[#2917](#2917)]
- Add nix-attic plugin to the index
[[#2889](#2889)]
- Add usage with Tunnelmole to docs
[[#2881](#2881)]
- Improve code blocks in docs
[[#2879](#2879)]
- Add a blog post
[[#2877](#2877)]
- Add documentation on Kubernetes securityContext
[[#2822](#2822)]
- Add default page to categories
[[#2869](#2869)]
- Use same format for Github docs as used for the other forges
[[#2866](#2866)]

### Misc

- chore(deps): update dependency isomorphic-dompurify to v2
[[#3001](#3001)]
- fix(deps): update dependency @intlify/unplugin-vue-i18n to v2
[[#2998](#2998)]
- Fix go in gitpod
[[#2973](#2973)]
- fix(deps): update module google.golang.org/grpc to v1.60.1
[[#2969](#2969)]
- chore(deps): update docker.io/alpine docker tag to v3.19
[[#2970](#2970)]
- Fix broken gated repos
[[#2959](#2959)]
- fix(deps): update golang (packages)
[[#2958](#2958)]
- Update docker.io/techknowlogick/xgo Docker tag to go-1.21.5
[[#2926](#2926)]
- Update docker.io/golang Docker tag to v1.21.5
[[#2925](#2925)]
- Lock file maintenance
[[#2910](#2910)]
- Update web npm deps non-major
[[#2909](#2909)]
- Update docs npm deps non-major
[[#2908](#2908)]
- Update golang (packages)
[[#2904](#2904)]
- Update module github.com/google/go-github/v56 to v57
[[#2899](#2899)]
- Update dependency marked to v11
[[#2898](#2898)]
- Update dependency vite-svg-loader to v5
[[#2837](#2837)]
- Update golang (packages)
[[#2894](#2894)]
- Update web npm deps non-major
[[#2895](#2895)]
- Update web npm deps non-major
[[#2884](#2884)]
- Update docker.io/woodpeckerci/plugin-docker-buildx Docker tag to
v2.2.1 [[#2883](#2883)]
@anbraten anbraten deleted the show-upper-lvl-secrets branch April 16, 2024 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features ui frontend related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

show global & org secrets (read only) in repo secrets tab
4 participants