github / safe-settings

ISC License
569 stars 137 forks source link

Same repository that is part of multiple suborgs #277

Open Chris-V opened 1 year ago

Chris-V commented 1 year ago

Prerequisites:

New Feature

What I am looking for exactly is a way to add required_status_checks from multiple base files to a repository. We extensively use monorepos and reusable workflows. Having a way to not repeat ourselves too much with the repo settings would facilitate maintenance quite a bit.

So I tried adding a single repo to multiple suborgs but the required_status_checks are not merged and only the checks from one suborg are synced. Every other checks are removed from the branch protections. Using suborgs for that purpose may not be appropriate though. What if a suborg requires 1 approval and the other one asks for 2 approvals? How to decide which one should be applied first?

I think a way to extend other config files and apply the extensions in the order they are specified could solve the issue. Many linter configs work this way. ie:

.github/repos/example.yml

"_extend":
  - webapp-react-frontend
  - webapp-bff

branches:   # Note that these are already merged to "root" and "one of the suborgs" settings
  - name: default
    protection:
      required_status_checks:
        contexts:
          - "Something very specific to this repo"

.github/templates/webapp-react-frontend.yml

branches:
  - name: default
    protection:
      required_status_checks:
        contexts:
          - "React / ESLint"

.github/templates/webapp-bff.yml

branches:
  - name: default
    protection:
      required_status_checks:
        contexts:
          - "BFF / Compile"
svg153 commented 1 year ago

Hi @Chris-V.

I experienced something similar in the #247, but @markjm fixes (I haven't tested it yet) in #262

Probably it's not working for sub-orgs? Are you using the new version 2.0.0?

:)

decyjphr commented 1 year ago

I think what you are looking for is to have multiple sub-org configs be applied to a single repo and merge the configs of multiple sub-orgs to that repo config.

Chris-V commented 1 year ago

Hi @Chris-V.

I experienced something similar in the #247, but @markjm fixes (I haven't tested it yet) in #262

Probably it's not working for sub-orgs? Are you using the new version 2.0.0?

:)

Yes, I'm testing against 2.0.0. Everything is merged properly except all of the suborgs. We are essentially limited to a single suborg per repo.

I think what you are looking for is to have multiple sub-org configs be applied to a single repo and merge the configs of multiple sub-orgs to that repo config.

In a way, but my understanding is that it may not be clear upfront in which order suborg configs will be applied. Hence my suggestion of an explicit "extends" keyword. Although for our usage (merge required_status_checks), a merge of the suborgs would be enough.

martinm82 commented 1 year ago

What do you think about such an organization of the settings: https://github.com/github/safe-settings/discussions/370?

esteluk commented 1 year ago

I think there's definitely value, but it's somewhat orthogonal to the main use case I'd see from this particular issue. eg. for at my org we'd have both discipline-specific and business-suborganisation specific configuration that it would be nice to be able to intelligently merge together.

anderssonjohan commented 1 year ago

A little heads up on merging required_status_checks. I could not apply my checks because the GitHub API rejected the payload defined as:

contexts: []
checks: [ { ... }, { ... } ]

To my surprise it turned out that I was having a suborg file with suborgteams. The suborgteams feature turns out to be like an alias for all the repositories defined on the given team. With multiple uses of suborgteams for the same team it's a bit unclear what config is used. It seems to apply the last config (in alphabetical order).

In my case I had contexts: [] in that config with suborgteams and the schema for the endpoint in the GitHub API accepts either contexts or checks (contexts are deprecated btw). So these two can't be merged and used simultaneously, ie., you have to use contexts (with name and appid) consistently in all safe-settings config files.

An option to #262 could be to allow inclusion of partial configurations. Something similar to this: https://stackoverflow.com/a/9577670/2447296

By using that a user could define behaviors in separate files such as in this example:

Example using include for python project checks:

Org-level checks baseline for all Python projects: .github/behaviors/python_project_checks.yml

- context: Block Autosquash Commits
  app_id: 15368
- context: Commitlint
  app_id: 15368
- context: Black, isort, Ruff, and Pytest
  app_id: 15368
- context: SonarCloud Code Analysis
  app_id: 12526

Team-level baseline: .github/suborgs/team1.yml

teams:
  - name: team1-developers
    permission: admin
  - name: all-developers
    permission: push

branches:
  - name: default
    protection:
      required_pull_request_reviews:
        required_approving_review_count: 1
        dismiss_stale_reviews: true
      required_linear_history: true
      allow_deletions: false
      block_creations: true
      required_status_checks:
        # Required. Require branches to be up to date before merging.
        strict: true
        checks: []
      enforce_admins: false

.github/repos/team1-python-service123.yml

branches:
  - name: default
    protection:
      required_pull_request_reviews: {}
      required_status_checks:
        checks:
          - !include behaviors/python_project_checks.yml