mszostok / codeowners-validator

The GitHub CODEOWNERS file validator
Apache License 2.0
217 stars 47 forks source link

Option to disable Check if a GitHub owner is in a given organization #73

Open Nuru opened 3 years ago

Nuru commented 3 years ago

Description

Please provide an option to disable only the "Check if a GitHub owner is in a given organization" part of the Valid Owner Checker.

Reasons

I frequently work in repos that have an outside collaborator as a Code Owner (sometimes it is me). I would like to be able to keep the other checks in the Valid Owner Checker active and disable only the "Check if a GitHub owner is in a given organization" part. It is excessive to completely disable all checks for outside collaborators.

mszostok commented 3 years ago

Hi @Nuru

I can add opt to disable that e.g. OWNER_CHECKER_DISABLE_ORG_MEMBER_VALIDATION=true/false. But it will be a short-term solution. Later someone else can as about disabling other sub-check, maybe even disable it only for some owners etc.

Sometimes, I see that people introduce known_violations.txt and compare validator output with this file. In that way, some known issues can be ignored.

With codeowners-validator, it's a bit complicated as the output is not deterministic (parallel execution), so I thought to add native support to ignore/disable certain issues.

Example 1:

Global option to document excluded/ignored rules.

# config.yaml

# Option to ignore known and accepted issues that are detected by checks
issues:
  # Excluding checks per-text
  exclude-rules:
    # Exclude per-text message. Exact match.
    - text: 'Team "not-existing-team" does not exist in organization "gh-codeowners"'
      checks:
        - owner
    # Exclude per-text message. Regex match. In that way u exclude organization check completely for any team
    - regex: 'Team ".*" does not exist in organization "gh-codeowners"'
      checks:
        - owner

Pros:

Cons:

Example 2:

Check-scoped option to document excluded/ignored rules.

# config.yaml

# all available settings of specific linters
checks-settings:

  owner:
    # The owner and repository name separated by slash. Used to check if GitHub owner is in the given organization.
    # (required)
    repository: "gh-codeowners/codeowners-samples"

    # Excluding configuration
    exclude-rules: # support excluded only for some users so outside collaborators are not check but the rest are validated

      # excluded for all as no owners provided
      - rule: valid-definition
      - rule: has-github-account
      - rule: team-exists

      # excluded only for a given owners
      - rule: is-in-organization
        owners:
          - @owner1
          - @owner2
          - @owner3

Pros:

Cons:

/cc @Nuru @osterman @nitrocode

nitrocode commented 3 years ago

I appreciate all the time you put into these examples. As for example 2, we currently ignore each individual using the owner_checker_ignored_owners input. I'd prefer not to ignore each owner individually as it's difficult to scale.

If we're going with either solution, I'd favor example 1 as that will eliminate the need to maintain a list of ignored users. Like you mentioned, if the message changes, then the exception breaks. However, I'm not too concerned about that as 1) that message probably doesn't change often and 2) we would notice immediately if the PR failed for the updated text and would update the exclusion text accordingly.

Nuru commented 3 years ago

@mszostok You are right that people will want to disable other sub-checks. If you look at other "linters" you will see that in general they give some kind of name/label/key to every single check at the finest possible detail level (such that there is no such thing as a "sub-check") and then allow people to enable or disable them one by one. Usually you can say "run all except these" or "run only these". See mega-linter and rubocop for just 2 examples. These linters have been around a long time and have much guidance to offer about how to structure the configuration of a linter such as codeowners-validator.

So I suggest you start there. You do not need to restructure the code that implements the tests, just expand the list of options to cover every test.