app
app copied to clipboard
Changes in .github repository are not propagated
Sorry, I didn't get a chance to chime in on https://github.com/probot/settings/pull/90, but it doesn't actually work.
As @jwsloan pointed out in https://github.com/probot/probot-config/issues/10#issuecomment-404657030, 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 - 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 amyOrg/.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 inmyOrg/inhereting-repository/.github/settings.yml
- the
_extends: .github
property inmyOrg/inhereting-repository/.github/settings.yml
loads the new config frommyOrg/.github/settings.yml
- the
-
probot-settings
applies the new settings tomyOrg/inhereting-repository
- 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.
@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?
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
@bkeepers @pholleran What are your thoughts on this:
- We add
updated_at
andupdated_by
tosettings.yml
. - The
settings
app is installed on the .github repo as well as all of your nomal repos. - The
probot-config
app exposes what thebase repo
is. - The
settings
app looks to see if the currentpush
is on thebase 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 setsupdated_by
toprobot-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.
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.
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
I think this would still be very helpful
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
Any updates on that workflow @pholleran ? I believe also the settingsModified
check makes it imposible do use the opt-out
workflow for probot-config
.
That would be a great addition. +1
This is a big blocker for the use case of applying settings across many repositories.
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.
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.
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.
A hacky workaround would be a GitHub Action workflow that, periodically, does a git commit --allow-empty
on the child settings file.
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.
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
I really like the @alvarosanchez solution, looks like a nice easy change, and just getting it to run on all push events
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.
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.
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.
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?
- if processing from the initial run that was triggered based on the change to the file in the
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.
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.
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.
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.
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.
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.
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.
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.