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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

The configuration issue to end all configuration issues 馃殏 #258

Open
hiimbex opened this Issue Sep 15, 2017 · 18 comments

Comments

Projects
None yet
9 participants
@hiimbex
Copy link
Member

hiimbex commented Sep 15, 2017

Configuration for probot has always been difficult. For context, originally, the goal of probot was to use only information from GitHub itself, with no outside data storage. However, this lack of data persistence has caused some problems and will continue to create issues down the line.

This proposal has 2 main parts: Individual Config Files for Repos along with their naming conventions and Organizational Level configurations.

馃殏 JOIN ME ON THIS LONG ISSUE JOURNEY 馃殏

Individual Config Files for Repos

The original probot apps each had individual configuration files usually closely tied to the name of the app that lived in .github/ folders. One example being stale, and its configuration settings lived in a .github/stale.yml file. During the great intern takeover of 2017, I decided this idea was silly. Each app having it's own individual file was not fun when you had 6 or 7 apps running on a single repo and each config file was only a few lines long.

So I created behaviorbot which uses a mutual/shared .github/config.yml for all of the apps within the org. People generally think this is a good idea, so I'd move to adopt future apps to have their settings as such:

# .github/config.yml file

stale:
  staleKey1: staleValue1
  # accessed by config.stale.staleKey1
  staleKey2: staleValue2
welcome:
  welcomeKey1: welcomeValue1
  # accessed by config.welcome.welcomeKey1
  welcomeKey2: welcomeValue2
# etc.

Obviously we don't want to break support for older apps, so we should also support:

# .github/stale.yml file

staleKey1: staleValue1
# accessed by config.stale.staleKey1
staleKey2: staleValue2

(and the kinda weird/slightly different way I named things in the behaviorbot .github/config.yml)

  • It is important that we be able to access these both via config.stale.staleKey1 and have the API on probot's end check for the existence of either of these and use that to populate the config we are returned. (Note: config.staleKey1 is not as clean of a solution given stale could have one of the same key names as welcome, so we must for .github/stale.yml files place them into a stale object.)

  • The only other relevant discussion here is the naming of the standardized configuration file. Currently the convention used is .github/config.yml; however, it is worth taking into consideration whether we want this file to be in a .github folder given this isn't a GitHub sponsored or owned project. It is also worth considering a name like probot-config.yml to add to our own branding and clarify that this is a probot specific configuration. 馃
    My personal opinion is to keep using .github/config.yml given the existing apps that depend on it and it's already implemented/being accepted. And, honestly, none of the arguments for swapping naming conventions are particularly convincing to me. I'm open to other opinions though.

Organizational Level configurations

Right now doing any configuring at an organizational level sucks. Some folks have written scripts to do this, but adding a config file to every repo you want a bot to run on in an organization is really difficult, so we should aim to fix that! 馃洜

Potential Solutions/Ideas:

  1. probot/settings#29 lays out a plan for a model using inheritance in which every repository has a config file, but it simply contains a line that says inherit: <repo name>, where the repo named in the configuration contains the actual config file settings that would be applied to the other repos.
    Pros:

    • Allows to easily opt in or out of specific repos within an org (ie you want only half of your repositories to use that app and that shared config or you want half your repos to use 1 configuration and the other half to use another)
    • Editing one configuration file effectively "updates" all files inheriting from it

    Cons:

    • You still have to add a configuration file to every repo you want the app to act on
  2. behaviorbot/welcome#9 and some out loud discussion during office hours discussed creating a repository upon app installation (for example your-org/probot-settings) that would always be used by the apps when searching for settings. The settings here would apply to all repositories in the organization.
    Pros:

    • Easily edit your config from one location that has org wide effect
    • Potentially have individual repos with their own config files that act as overrides for repos that need different settings
    • Helps probot branding by making our settings more visible? (only if this repo is public, not private)

    Cons:

    • We would need to come up with a way for disabling for an individual repository easily?
    • Some organizations might not like having a repository added just to hold our settings
  3. 馃毃 Data persistence 馃毃 use some sort of simple key/value store to store data for the configuration which would be bound by some hashing or encrypting of the private key. This would be accessed through a simple web UI that we would redirect to upon an app's installation.
    Pros:

    • Easy to retrieve info for each repo on our end
    • Easy for the user to edit their settings at any time

    Cons:

    • We would prefer to avoid this kind of data persistence for security reasons and the pain of building out such a web UI. (In the past this seemed a lot more undoable, but as probot grows, I see the cons for this one shrinking)

tl;dr We need to standardize the way we do configs in order to provide better user experience. We can only do this once since many apps will start using these standards, it will become hard to undo/change.

Open to any further thoughts/ideas/opinions/implementations/troutslaps 馃悷 about anything discussed here.

@Ben3eeE

This comment has been minimized.

Copy link

Ben3eeE commented Sep 16, 2017

troutslaps 馃悷

馃槅


I have no idea how anything works but one of the most useful things I have found as a developer is listening to an idea from someone who doesn't know anything and working around that to make something useful so here is some random idea I can think of:

  • You install the app to one repository in your org
  • This repo has all the configuration for the app
  • This repo also includes a config file for which repos in the org the bot should exist or which repos in the org the bot should ignore

This is similar to the your-org/probot-settings but it solves both cons by:

  • Easily being able to ignore certain repos
  • The repository can be any name
@hiimbex

This comment has been minimized.

Copy link
Member

hiimbex commented Sep 16, 2017

Thanks for the feedback @Ben3eeE! Totally agree that all feedback especially from new people is always helpful.

You install the app to one repository in your org...This repo also includes a config file for which repos in the org the bot should exist or which repos in the org the bot should ignore

Unfortunately, in order for the bot to be listening to webhooks for repos, we do need the user to actually install it on every repository they want the app to act on, otherwise GitHub doesn't send the webhooks for that repository.

You can, however, install an app at an organizational level which would apply to every repo in the org, but if that repo doesn't have a config file or a set repo to look for one, searching through potentially hundreds of repos to see if the file exists could be costly in API calls, especially for larger organizations (since I believe you can have an unlimited number of public repos in any organization or user account). So I'm concerned about creating a feature that might time out for larger organizations, although I'm not exactly sure the specifics or possibility of an API call like that.

If I recall correctly, we ruled out that idea early on in the discussion, but I can't remember if the above was the exact reasoning. @bkeepers might remember better?

@Ben3eeE

This comment has been minimized.

Copy link

Ben3eeE commented Sep 18, 2017

@hiimbex Thank for you the reply. The your-org/probot-settings idea makes more sense to me now that I understand this limitation.

@tcbyrd

This comment has been minimized.

Copy link
Contributor

tcbyrd commented Sep 19, 2017

Just thinking out loud, if there's appetite for building out some sort of minimal web UI for this, it doesn't necessarily mean we have to use a separate database for persistence reasons. Could it just be an easy way to edit the data in the a probot-settings repo?

This would solve for:

We would need to come up with a way for disabling for an individual repository easily?

And if a user doesn't want to use a probot-settings repo, if the data format is normalized into some kind of key/value pair, they could always choose to back their own installation with a database they control.

@hiimbex

This comment has been minimized.

Copy link
Member

hiimbex commented Sep 19, 2017

Yea, a while back I opened #187 to talk about the idea or redirecting to a form that would do that but still assuming the config would live on GitHub somewhere. We could then open a PR to org/probot-settings, so I'm +1 for that as well. The obvious starting point is developing the API for configs at an organizational level in that repo and what their syntax should be.

And if a user doesn't want to use a probot-settings repo, if the data format is normalized into some kind of key/value pair, they could always choose to back their own installation with a database they control.

Could you explain further how you envision this aspect working?

@tcbyrd

This comment has been minimized.

Copy link
Contributor

tcbyrd commented Sep 19, 2017

Could you explain further how you envision this aspect working?

馃 I haven't really thought about it much more beyond that, but now that you ask! 馃槂

I just came across this PR to add database examples to the documentation: #242

I've never worked with yaml files outside of flat files in repositories, so I'm just not sure how the *.yml files would map to an object in a MongoDB/Firebase database. Following the example set by the PR above, if we can just document how to do that, that's 馃憣 by me. Just not sure if there would have to be some intermediary step where the settings are serialized into a JSON object.

@jan-auer

This comment has been minimized.

Copy link

jan-auer commented Sep 21, 2017

TL;DR: Bots should be aware of orga-wide config and be able to update it (e.g. via web interfaces or Slack).


Just to add one more dimension to this: We were also thinking about creating a probot app that (optionally) posts stuff to Slack users in direct messages. In that case we need to maintain a mapping between Slack users and Github users. While this is some sort of configuration, it is also dynamic and changes more often than other configs.

Yes, usually this info would go in a database. However, treating it as organization-wide configuration and putting it in a repo makes maintaining it much easier. Also, it allows us to configure overrides for certain repos. So there's definitely no need for a database on our end...

Essentially, this would mean:

  • Every probot app has its own default settings
  • There's a central repo that contains orga-wide settings and configuration. E.g. orga/probot-settings
  • Each repo within that orga can still contain their .github/whatever.yml that will be merged with the global settings and default settings
  • Given sufficient permissions, the github app can update its own settings on both the orga and repo level, except without a PR

Adding a database could still make sense on top of this, however we've not come across a use case where we really need one, so far. Databases hide information and obfuscate the app's internal state from orga admins if they choose to use one of the off-the-shelf probot apps (thus having no access to the database itself).


Apart from that, my personal opinion is that the settings repo should have one yml file per probot app, just like in the .github folder in each repo, as opposed to one large config.yml. Other than that, going by convention and naming this repo probot-settings or probot-config is probably fine for most. Furthermore, this repo should probably be private by default (if probot creates it), since it might contain sensitive information.

Who doesn't like that can still create their own combined app and set a custom config repo via an env variable (e.g. SETTINGS_REPO). For instance, the repository of the combined app could then host configs as well.

@stale

This comment has been minimized.

Copy link

stale bot commented Nov 20, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 20, 2017

@tunnckoCore

This comment has been minimized.

Copy link

tunnckoCore commented Nov 20, 2017

Bad bot, bad bot! Shut up and sit down, doggy dog :D

@stale stale bot removed the wontfix label Nov 20, 2017

@sentiment-bot

This comment has been minimized.

Copy link

sentiment-bot bot commented Nov 20, 2017

Please be sure to review the code of conduct and be respectful of other users. cc/ @probot/maintainers
Keep in mind, this repository uses the Contributor Covenant.

@j-f1

This comment has been minimized.

Copy link
Contributor

j-f1 commented Nov 20, 2017

A wild repro of #337 has been sighted in this vicinity!

@stale

This comment has been minimized.

Copy link

stale bot commented Jan 19, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 19, 2018

@Mte90

This comment has been minimized.

Copy link

Mte90 commented Jan 21, 2018

no please, don't close it!

@stale stale bot removed the wontfix label Jan 21, 2018

@stale

This comment has been minimized.

Copy link

stale bot commented Mar 22, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 22, 2018

@Mte90

This comment has been minimized.

Copy link

Mte90 commented Mar 22, 2018

no please again, don't close it!

@stale stale bot removed the wontfix label Mar 22, 2018

@tunnckoCore

This comment has been minimized.

Copy link

tunnckoCore commented Mar 23, 2018

Yea, still trying to find time to read it and join :D

@j-f1

This comment has been minimized.

Copy link
Contributor

j-f1 commented Mar 23, 2018

Can someone with write access add the pinned label so the issue doesn鈥檛 get marked as stale?

@JasonEtco JasonEtco added the pinned label Mar 23, 2018

@cco3

This comment has been minimized.

Copy link

cco3 commented May 16, 2018

Additional con for solution 2 (at least the way it is presented here), requires the administration permission, which is fairly heavy duty. Of course, the automated creation of an org-level settings repository could be optional.

Regardless, I very much like solution 2 since it doesn't require adding configurations in individual repositories. In our case, not only do we have the issue of having many repositories in the same org that should have identical configurations, but we also have mirrors we would like to install an app on, and creating a config upstream for a downstream mirror is undesirable.

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