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

Changes in .github repository are not propagated #95

Open
bkeepers opened this issue Jul 12, 2018 · 32 comments
Open

Changes in .github repository are not propagated #95

bkeepers opened this issue Jul 12, 2018 · 32 comments

Comments

@bkeepers
Copy link
Contributor

Sorry, I didn't get a chance to chime in on #90, but it doesn't actually work.

As @jwsloan pointed out in probot/probot-config#10 (comment), we'll need to add special support so that if settings.yml is updated in a .github repository, the changes need to be propagated to all repositories that inherit that config.

cc @pholleran @JasonEtco

@bkeepers bkeepers added the bug label Jul 12, 2018
@pholleran
Copy link
Contributor

pholleran commented Jul 13, 2018

@bkeepers - while I agree that would be nice, I am 🆗 with a workflow where the inheriting repo checks for and applies the latest permissions upon the next push.

Here's my understanding of how this "should" work: (recognizing that it does not currently work)

  • changes are applied to settings.yml in a myOrg/.github repository
  • a user makes a push to the myOrg/inheriting-repository
  • the push event triggers a webhook to probot-settings
  • probot-settings checks for updated settings in myOrg/inhereting-repository/.github/settings.yml
    • the _extends: .github property in myOrg/inhereting-repository/.github/settings.yml loads the new config from myOrg/.github/settings.yml
  • probot-settings applies the new settings to myOrg/inhereting-repository

@bkeepers
Copy link
Contributor Author

  • a user makes a push to the myOrg/inheriting-repository

Just to clarify, the push would have to be to the .github/settings.yml file. All other pushes are ignored.

@pholleran
Copy link
Contributor

@bkeepers - would it be reasonable, then, to not ignore all other pushes? Would this be lighter touch than trying to propagate changes down to the inheriting repos?

@pholleran
Copy link
Contributor

or, alternatively, would it make sense to explore a "hacky" solution with probot/metadata in which we utilize a closed issue to store the last synchronized commit SHA of myOrg/.github/settings.yml and modify settingsModified to compare the current SHA of myOrg/.github/settings.yml with the one stored in metadata to determine if a change has occurred?

https://github.com/probot/settings/blob/94a7ef31540c056ae2a39f5e28757b079868d61e/index.js#L10-L13

@jwsloan
Copy link
Collaborator

jwsloan commented Jul 16, 2018

@bkeepers @pholleran
What are your thoughts on this:

  • We add updated_at and updated_by to settings.yml.
  • The settings app is installed on the .github repo as well as all of your nomal repos.
  • The probot-config app exposes what the base repo is.
  • The settings app looks to see if the current push is on the base repo. If it is, it loops through all of the other repos it is installed on (I think it can do that).
  • Within the loop, it puts in a PR to each of those repositories.
  • The PR changes updated_at and sets updated_by to probot-config.
  • The description of the PR includes a link to the triggering commit to .github/settings.yml.
  • The maintainers of each repo decide to accept or not accept that change.

This gives each repo the ability to override a change they don't want to accept.

@jwsloan
Copy link
Collaborator

jwsloan commented Jul 16, 2018

Actually... Maybe instead of the updated fields, we just keep track of base_commit, which is the latest hash from probot-config. And we update that with a PR.

@stale
Copy link

stale bot commented Oct 14, 2018

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Oct 14, 2018
@travi
Copy link
Member

travi commented Oct 15, 2018

I think this would still be very helpful

@stale stale bot removed the wontfix label Oct 15, 2018
@stale
Copy link

stale bot commented Feb 4, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Feb 4, 2019
@stale stale bot closed this as completed Mar 6, 2019
@pecigonzalo
Copy link

Any updates on that workflow @pholleran ? I believe also the settingsModified check makes it imposible do use the opt-out workflow for probot-config.

@kairichard
Copy link

That would be a great addition. +1

@jpoehnelt
Copy link

This is a big blocker for the use case of applying settings across many repositories.

@travi
Copy link
Member

travi commented Apr 24, 2020

it is understood that this is a highly desirable feature, but it also one that needs to be implemented carefully because it has much broader impact than the current behavior. this is high in the priority list, but also needs focused time to do well, which is difficult to come by currently. i have a few features that i plan to get in place before this, in order to make it safer to introduce this behavior, so i cannot promise any timeframe for this being available.

@iphydf
Copy link

iphydf commented Apr 24, 2020

Just cross-posting from #179:

Do you think we could make this configurable in case I don't want to name my .github repository ".github"? The main reason for that is that I have a master repo which has submodules with all the repos in the org. I currently named my "base" repo template, but I'm not married to that. However, I can't name it ".github", because then I can't have a submodule for it in the main stack repo, at least not have it named the same way as the .github repo itself. I could have a submodule "template" that points at the repo ".github", but it would be anomalous compared to everything else.

@hybridherbst
Copy link

I'd like to chime in with our usecase here.
I wanted to set up consistent labelling between various of our repos. While GitHub allows setting default labels for new repos now, maintaining an evolving set of labels across repositories is still cumbersome.

Then, I found probot/settings, which sounded like the perfect solution!

  • I created a new repo which only has the label settings
  • I added a .github/settings.yml to the other repos with just a single line, _extends:base-settings
  • I tested ... didn't work ... found this issue here (2 years old now). Oops.

I thought I found the solution to label automation, but this issue basically makes the above workflow kind of useless as-is as I still need to change that single-line yaml on each repo that should have the updated labels.

Besides what others have mentioned in terms of automation (which I understand is complicated), I would already be happy if there was a button/command/action (ideally org level) that just re-runs probot and applies the settings file again, as if a push had happened.

@alvarosanchez
Copy link

A hacky workaround would be a GitHub Action workflow that, periodically, does a git commit --allow-empty on the child settings file.

@alvarosanchez
Copy link

alvarosanchez commented Jul 9, 2020

Another possibility would be for this app to react on all push events, not only those affecting the settings file.

So I guess it would be to remove this lines: https://github.com/probot/settings/blob/master/index.js#L19-L27

GitHub
Pull Requests for GitHub repository settings. Contribute to probot/settings development by creating an account on GitHub.

@ap0phi5
Copy link

ap0phi5 commented Jul 29, 2020

Whilst not ideal, a periodic check might work- I believe https://github.com/probot/stale has, by default, an hourly scheduler.

GitHub
A GitHub App built with Probot that closes abandoned Issues and Pull Requests after a period of inactivity. - probot/stale

@jay-oswald
Copy link

I really like the @alvarosanchez solution, looks like a nice easy change, and just getting it to run on all push events

@lechen26
Copy link

any news on that?
i thought Probot/setting will help me achieve single place for repo settings on my org but seems like the duplication of operations still exists.

@micimize
Copy link
Contributor

micimize commented May 4, 2021

What about searching for repos that _extend a changed config, and then calling syncSettings for each?

IMO, this should maybe be an octokit-plugin-config concern, or a standalone helper. A robust implementation of config(...).extenders would search org the org recursively for _extends: {this config patterns}, and return their paths+context. Then settings could just do config.extenders.forEach(syncSettings)

* Edited to correct my misunderstanding of the _extends keyword.

@wraithgar
Copy link

Is there a spec for how you would want this implemented? I've seen several suggestions for ways to get this behavior but nothing definitive. I know the behavior wanted is "changes in _extends-ed configs are propagated to repos that consume them." I also see that @travi has features they want implemented first to make it safer to do this feature, which makes me think there is already some design in mind for how this would be solved.

With a spec defined this could be something our team can devote some resources to.

@travi
Copy link
Member

travi commented Sep 27, 2021

there is not a formal spec at this point, but i expect something like the following could solve this fairly well:

  • trigger based on a change to the .github/settings.yml in the .github repo
  • apply changes to the .github repo (this may not always be the goal of defining settings here, but is current behavior that i think is out of scope for getting propogation working)
  • determine which repos in the org extend from the file in the .github repo (i think some of the internals of octoherd could help here)
  • trigger processing of settings for each repo found that extends the settings from the .github repo. i'm not sure if this step should process all of these actions from the same run, or if it would be possible to trigger an event that could trigger a separate run for each of those repos. a few things to consider:
    • if processing from the initial run that was triggered based on the change to the file in the .github repo, octoherd internals could likely help here as well
    • if processing with separate runs, we would need to make sure that processing would not bail because the file from that repo did not change. also, would we lose the ability to report results back to somewhere that an end user could see and debug with?

I also see that @travi has features they want implemented first to make it safer to do this feature

while this is true, i have not managed to make the progress that i've hoped and dont want the shaving of an unkept yak to hold back progress here if we are able to get some momentum behind this.

status checks

mainly, i've wanted to enable reporting status checks that could show failures to help folks debug when things don't go well. i think there could be benefit of reporting a status for each api call that is made based on the contents of the file for the repo that the run was triggered for. i think that gets more complicated when considering a run triggered by the .github repo file. in that case, it probably makes sense to, at most, report a status per repo being updated.

if we could consider incorporating some of that functionality as part of that effort, i would be supportive, but i don't want to hold this up in the absence of that.

testing confidence

the other big consideration i want to keep in front of us is having appropriate test coverage around the behaviors of a feature like this so that maintenance does not become more complex moving forward. if all checks are green, it should be safe to merge a PR into the project. if that is not the case, testing needs improvement. the complexity of testing rises a fair amount when we start considering behavior outside of a single repository, so i just want to make sure that we account for handling that appropriately.

there are still some gaps in coverage that i have not closed since taking over maintenance of the project. i'm not suggesting that we need to close all gaps, but we do need to make sure that no new gaps emerge as a result of this effort. i've also not iterated enough on the integration tests to work out some of the pain points. i normally prefer to use cucumber for that style test, but would probably avoid changing frameworks for now. i am open to refactoring of the existing tests to remove some of the testing pain that still exists as long as we still accomplish the goals of the various testing layers.

summary

does this help clarify the direction that has been on my mind? does it align with your goals? i need to find the time to pull out some better issues for .github repo settings propagation and status checks and reference issues like this to get a plan better organized, but feel free to continue the conversation here until i get things cleaned up a bit.

@wraithgar
Copy link

Only triggering on changes to .github/settings.yml in the .github repo means that is the only file that can be extended upon in a way that will propagate. The _extends keyword can extend from anywhere, not just that file.

@travi
Copy link
Member

travi commented Sep 27, 2021

The _extends keyword can extend from anywhere, not just that file.

yes and no, depending how you are leveraging the settings app. if using the hosted version extension is limited to that file because we keep our request for permissions pretty narrow. without requesting more permission from all users of the hosted instance, extending from other files would require hosting your own instance.

are you already hosting your own instance? could you describe your approach to extension so that we are clear about the goal? while the hosted instance is limited, i agree that we should consider more flexibility with this implementation.

@wraithgar
Copy link

wraithgar commented Sep 27, 2021

Our approach is that our org @npm is shared between two teams: the registry and the cli. In order that we don't step on each others' toes with regards to things like issue labels and repo settings we have generated a separate settings file that the cli repos will inherit. It lives outside the .github folder in the .github repo so there is no confusion or collision w/ the registry team.

https://github.com/npm/arborist/blob/main/.github/settings.yml

---
_extends: '.github:npm-cli/settings.yml'

This is working as expected using the current github settings app. The only issue is that we are "locked" in to whatever settings were in that file whenever we last commited the .github/settings.yml file in any given repo.

As I tried to think about possible solutions for this that would be possible to implement, my initial (potentially naive) thoughts were that one of two things could work.

  • a periodic check / recompiling of any repo's .github/settings.yml file to re-apply the config if there is a need (how to determine if there is a need is open to discussion, timestamps of files involved, config diff, let the existing config sync handle things)
  • a way for a github action to trigger the syncSettings code in probot for a given repo. Then the scheduling of updates can be something that can be implemented on a per-repo basis (either through scheduled actions or custom action events)

My thinking went along these lines because the probot-settings app currently only listens to a very limited set of events from the repos it is enabled on, and it seemed like a pretty drastic change to open that up to more events.

GitHub
npm's tree doctor. Contribute to npm/arborist development by creating an account on GitHub.

@travi
Copy link
Member

travi commented Sep 27, 2021

This is working as expected using the current github settings app.

it looks like i've been assuming, incorrectly, that this wouldn't be possible in the hosted app. however, it does look like it is configured to grant the permissions needed for this. thank you for highlighting this.

one of two things could work

i do believe that those are options, but i don't believe that they would be the ideal approach. while updates on a periodic basis would be better than the very manual approach currently required, i think pushing the changes to the children projects based on updates to parent configs would be a more ideal solution. i would also like to keep the behavior as self-contained in the app as we reasonably can so that users arent required to do a bunch of additional config of actions, etc to get the functionality.

there is an additional benefit to the periodic approach that we shouldnt dismiss, though. there are currently no events that we could listen for when settings are changed through the web ui to become out of sync with what is defined in the settings.yml. having a periodic refresh could help ensure that the window of time where the settings are out of sync is smaller than today, or even with a push approach in place. ideally, i'd prefer there was a way to disable the web config for the settings managed through the config file, or at least have events that could result in this app sendings PRs to update the config based on changes in the ui, but those both seem unrealistic for now.

i'll have to give some thought to how the app could know which parent configs to trigger on when using more than the default, but i think it would be best to have a push approach when they do change. it could make sense to enable some sort of more-manual trigger, too. i'm open to discussing the balance further, but it does feel like a manual trigger is a little bit more of a work-around than a full solution, so i'm not sure that i would be comfortable suggesting that as the full solution.

@wraithgar
Copy link

wraithgar commented Sep 27, 2021

there are currently no events that we could listen for when settings are changed through the web ui to become out of sync with what is defined in the settings.yml

probot-settings is already doing this but only limiting its re-application of config to when the default_branch was changed https://github.com/probot/settings/blob/master/index.js#L35

The api route this is documented as being tied to does contain most of what the settings app can edit, labels notwithstanding. We would also likely need to listen to some sort of labels event to get this.

GitHub
Pull Requests for GitHub repository settings. Contribute to probot/settings development by creating an account on GitHub.

@travi
Copy link
Member

travi commented Sep 27, 2021

probot-settings is already doing this but only limiting its re-application of config to when the default_branch was changed

you're right. clearly, some of these details have become a bit fuzzy and i'm not remembering the details clearly. however, it does still highlight my point. i would prefer to expand this approach of reacting to changes so that the app enforces the config as the source of truth. in addition to just re-syncing, i think opening a PR after re-syncing would provide better feedback about why the changes made through the ui disappeared. that also gives some guidance about how they could stick.

you're also correct that we would also need to listen for labels changes. in addition, we would need to listen for changes to each of the "plugin" topics. there is work to be done there, but we could do better.

we're probably getting a little of track, but i agree that these do all have an impact on this topic. the combination of all of this has been a big factor holding back implementation of this feature.

i do believe that perfection is the enemy of good. i'm open to discussing the balance further to get us moving forward. i just want to make sure that we do keep an eye on trade-offs and whether they are the right ones for moving toward better and keeping maintainability reasonable.

@flugg
Copy link

flugg commented Apr 12, 2022

Any progress on this @travi ? Let us know if there's anything the community can help out with, I'd be happy to be of assistance.

@mortenlj
Copy link

mortenlj commented Apr 12, 2022

I have recently come across https://github.com/github/safe-settings , which I think covers many of the same use cases as probot/settings does. Mentioning it here in case it solves the needs for some of the people waiting for this issue ...

GitHub
Contribute to github/safe-settings development by creating an account on GitHub.

@nitrocode
Copy link
Contributor

Thanks @mortenlj for sharing. It seems like if .github repo settings are modified, they are then propagated across the org.

Events

The App listens to the following webhook events:

  • push: If the settings are created or modified, that is, if push happens in the default branch of the admin repo and the file added or changed is .github/settings.yml or .github/repos/.ymlor .github/suborgs/.yml, then the settings would be applied either globally to all the repos, or specific repos. For each repo, the settings that is actually applied depend on the default settings for the org, overlayed with settings for the suborg that the repo belongs to, overlayed with the settings for that specific repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.