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

Fix repo access (#476) #477

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

6543
Copy link
Member

@6543 6543 commented Oct 26, 2021

Backport #476

  • fix repo access

  • fix permission syncing

* fix repo access

* fix permission syncing
@6543 6543 added bug Something isn't working server labels Oct 26, 2021
@6543 6543 added this to the 0.14.3 milestone Oct 26, 2021
@6543 6543 mentioned this pull request Oct 26, 2021
@6543 6543 requested a review from anbraten October 26, 2021 19:47
@6543
Copy link
Member Author

6543 commented Oct 26, 2021

test failed I have to ajust it to work with v0.14 :/

@6543 6543 merged commit 3af5f99 into woodpecker-ci:release/v0.14 Oct 26, 2021
@6543 6543 deleted the backport_repo-access-fix branch October 26, 2021 21:27
@alexef
Copy link
Contributor

alexef commented Oct 27, 2021

@6543 this caused the github user to run out of api calls (request limits)

the webhooks started to receive 403 from github, and reported back 404.

Any chance we can make the perms sync less aggressive?

@6543
Copy link
Member Author

6543 commented Oct 27, 2021

I'd like to make that sync on demand later ... but this wont be backported as it would be twice the amount of work.

meanwhile I created an issue for it: #485

for your big org best would be to pin to the old version v0.14.1 until v0.15.0 is here i guess

sorry for the inconvenience - if you have a idear how to fix it for v0.14 just tell :)

@alexef
Copy link
Contributor

alexef commented Oct 28, 2021

hey @6543 no worries.

We're in a delicate situation, as v0.14.1 doesn't contain the fixes for double wildcard nor for depends_on, and v0.14.2 lacks a frontend (see the discord conversation about index.html panicking).

If this change (fetch perms per user per repo) is part of v0.15.0, then we will not be able to upgrade to that either.

I would suggest we make fetching permissions configurable for now (I can try a PR). for large orgs with flat permissions (everyone has rw access to everything, we only use branch protection) the fetching of perms per user/per repo is not needed.

@6543
Copy link
Member Author

6543 commented Oct 28, 2021

@alexef the missing frontend should be fixed now ...

what you propose is a workaround but I'm happy to merge it into v0.14 branch untill properly solved.
So I'll wait for your pull and if landed we can do a new release :)

@6543
Copy link
Member Author

6543 commented Oct 28, 2021

ok let's keep chating via discord or matrix :)

@woodpecker-ci woodpecker-ci locked as off-topic and limited conversation to collaborators Oct 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants