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

[FEATURE] Periodically validate all CODEOWNERS files under the AsyncAPI organization #1322

Open
2 tasks done
mszostok opened this issue Jul 23, 2024 · 4 comments
Open
2 tasks done
Labels
enhancement New feature or request

Comments

@mszostok
Copy link
Contributor

mszostok commented Jul 23, 2024

Why do we need this improvement?

While working on #1269, I noticed such problems related to the CODEOWNERS files under the AsyncAPI organization:

  1. Invalid CODEOWNERS filename (fixed by this pull request).
  2. Invalid user in CODEOWNERS
    • user changed their profile name and while this was updated here, it is still missing from other places.
    • missing write access e.g. damaru-inc useer

Issues report generate on 24.07.2024

Found 11 invalid CODEOWNERS files.

Click to see a detailed report

# Found 11 invalid CODEOWNERS files
                                                                                                                                                                                                             
## python-paho-template https:/asyncapi/python-paho-template                                                                                                                                     
                                                                                                                                                                                                             
  Unknown owner on line 8:  make sure @damaru-inc exists and has write access to the repository                                                                                                               
                                                                                                                                                                                                             
    * @damaru-inc @CameronRushton @asyncapi-bot-eve                                                                                                                                                          
      ^                                                                                                                                                                                                      
                                                                                                                                                                                                             
## .github https:/asyncapi/.github                                                                                                                                                               
                                                                                                                                                                                                             
  Unknown owner on line 9:  make sure @alequetzalli exists and has write access to the repository                                                                                                             
                                                                                                                                                                                                             
    * @derberg @alequetzalli @KhudaDad414 @asyncapi-bot-eve                                                                                                                                                  
               ^                                                                                                                                                                                             
                                                                                                                                                                                                             
## brand https:/asyncapi/brand                                                                                                                                                                   
                                                                                                                                                                                                             
  Unknown owner on line 8:  make sure @mcturco exists and has write access to the repository                                                                                                                  
                                                                                                                                                                                                             
    * @fmvilas @mcturco @asyncapi-bot-eve                                                                                                                                                                    
               ^                                                                                                                                                                                             
                                                                                                                                                                                                             
## training https:/asyncapi/training                                                                                                                                                             
                                                                                                                                                                                                             
  Unknown owner on line 8:  make sure @alequetzalli exists and has write access to the repository                                                                                                             
                                                                                                                                                                                                             
    * @alequetzalli @asyncapi-bot-eve                                                                                                                                                                        
      ^                                                                                                                                                                                                      
                                                                                                                                                                                                             
## java-template https:/asyncapi/java-template                                                                                                                                                   
                                                                                                                                                                                                             
  Unknown owner on line 9:  make sure @JEFFLUFC exists and has write access to the repository                                                                                                                 
                                                                                                                                                                                                             
    * @AGurlhosur @dalelane @dan-r @KieranM1999 @lewis-relph @JEFFLUFC @asyncapi-bot-eve                                                                                                                     
                                                             ^                                                                                                                                               
                                                                                                                                                                                                             
## learning-paths https:/asyncapi/learning-paths                                                                                                                                                 
                                                                                                                                                                                                             
  Unknown owner on line 8:  make sure @Barbanio exists and has write access to the repository                                                                                                                 
                                                                                                                                                                                                             
    @alequetzalli @asyncapi-bot-eve @Barbanio                                                                                                                                                                
                                    ^                                                                                                                                                                        
                                                                                                                                                                                                             
## spec-json-schemas https:/asyncapi/spec-json-schemas                                                                                                                                           
                                                                                                                                                                                                             
  Unknown owner on line 13:  make sure @SrfHead exists and has write access to the repository                                                                                                                 
                                                                                                                                                                                                             
    /bindings/jms/          @rcoppen @SrfHead                                                                                                                                                                
                                     ^                                                                                                                                                                       
                                                                                                                                                                                                             
  Unknown owner on line 16:  make sure @damaru-inc exists and has write access to the repository                                                                                                              
                                                                                                                                                                                                             
    /bindings/solace/       @damaru-inc @CameronRushton                                                                                                                                                      
                            ^                                                                                                                                                                                
                                                                                                                                                                                                             
## bindings https:/asyncapi/bindings                                                                                                                                                             
                                                                                                                                                                                                             
  Unknown owner on line 16:  make sure @damaru-inc exists and has write access to the repository                                                                                                              
                                                                                                                                                                                                             
    /solace/       @damaru-inc @CameronRushton                                                                                                                                                               
                   ^                                                                                                                                                                                         
                                                                                                                                                                                                             
## raml-dt-schema-parser https:/asyncapi/raml-dt-schema-parser                                                                                                                                   
                                                                                                                                                                                                             
  Unknown owner on line 9:  make sure @jstoiko exists and has write access to the repository                                                                                                                  
                                                                                                                                                                                                             
    * @fmvilas @jstoiko @smoya @asyncapi-bot-eve                                                                                                                                                             
               ^                                                                                                                                                                                             
                                                                                                                                                                                                             
## java-spring-cloud-stream-template https:/asyncapi/java-spring-cloud-stream-template                                                                                                           
                                                                                                                                                                                                             
  Unknown owner on line 8:  make sure @damaru-inc exists and has write access to the repository                                                                                                               
                                                                                                                                                                                                             
    * @damaru-inc @CameronRushton @asyncapi-bot-eve                                                                                                                                                          
      ^                                                                                                                                                                                                      
                                                                                                                                                                                                             
## community https:/asyncapi/community                                                                                                                                                           
                                                                                                                                                                                                             
  Unknown owner on line 8:  make sure @alequetzalli exists and has write access to the repository                                                                                                             
                                                                                                                                                                                                             
    * @alequetzalli @derberg @asyncapi-bot-eve @thulieblack                                                                                                                                                  
      ^                                                                                                                                                                                                      

Tip

To get a fresh report run:

gh extension install github.com/gitangle/gh-codeowners
gh codeowners validate  --owner asyncapi --all --ignore-repos shape-up-process

How will this change help?

Without that change, the MAINTAINERS.yaml file may still reflect an incorrect state, as the related CODEOWNERS file could be outdated, invalid, or even missing.

How could it be implemented/designed?

Start Minimal

  1. Create a single GitHub Action that runs weekly:
    • Checks for issues across all CODEOWNERS files using a query like this:
      query ($owner: String!) {
        organization(login: $owner) {
          repositories(first: 100, visibility: PUBLIC) {
            nodes {
              name
              url
              codeowners {
                errors {
                  path
                  source
                  kind
                  line
                  message
                  suggestion
                }
              }
            }
          }
        }
      }
      Alternatively, you can use this tool: gh-codeowners.
    • Detects if a repository does not have a CODEOWNERS file, eliminating issues like this: github-action-for-cli/pull/397.
  2. If there is an issue, post a Slack message with a link to the GitHub job and a human-readable report of the issue.

Improvements for Later

  1. Consider using more advanced validators to check for:

    • Lines with file patterns that do not exist in the repository.
    • Duplicate lines with the same file pattern.
    • Invalid syntax definitions. This is important because:

      If any line in your CODEOWNERS file contains invalid syntax, the file will not be detected and will not be used to request reviews. Invalid syntax includes inline comments and user or team names that do not exist on GitHub."

  2. Consider auto-fixing CODEOWNERS files. For example, when a user is removed from the organization, update the related CODEOWNERS files to reflect that change.

🚧 Breaking changes

No

👀 Have you checked for similar open issues?

  • I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue?

Yes I am willing to submit a PR!

@mszostok mszostok added the enhancement New feature or request label Jul 23, 2024
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@derberg
Copy link
Member

derberg commented Jul 27, 2024

thanks for the issue

the problem is that even if we get a report that there are errors, we need to find someone who has time to fix it.

wouldn't it be better to have a workflow in each repo that is dedicated to codeowners validation? and it is triggered only if codeowners file is edited. This way we block errors on PR level which I think is best

@mszostok
Copy link
Contributor Author

wouldn't it be better to have a workflow in each repo that is dedicated to codeowners validation? and it is triggered only if codeowners file is edited. This way we block errors on PR level which I think is best

In PRs, this is less useful. You create a PR with a change that already makes sense, and a reviewer checks it too. An extra layer of protection is nice but not essential.

The real problem is with CODEOWNERS files that remain on the main branch without any changes. They become easily outdated. A periodic check can help in this situation.

For example:

  • A username change: Currently, some CODEOWNERS files are outdated and don't reflect changes like alequetzalli -> quetzalliwrites.
  • A user is removed from the organization or a specific GitHub repository, like damaru-inc, jstoiko, and others.

If you want to do this only via PR checks, you will need to wait for a PR to be created to trigger a workflow that checks the CODEOWNERS file. First, such PRs are not that frequent. Second, if a contributor simply wants to add someone as a new codeowner, they also need to deal with issues unrelated to their changes.

(..) we need to find someone who has time to fix it.

Maybe the first version should already come with a simple solution to fix related issues and be executed only once per month. It could be via Slack or CLI in terminal, sth like:

Screen.Recording.2024-07-29.at.20.06.04.mov

I just think that it's hard to automate it and a human is required in the flow to approve or adjust fixes and decide whether to:

  • Remove the user from the CODEOWNERS file
  • Rename the username from x to y
  • Fix their permissions in a given repository

On the other hand, outdated CODEOWNERS should be rare, so the question is whether it makes sense to simplify it so much 🤔

@derberg
Copy link
Member

derberg commented Aug 1, 2024

ok, so my assumption was your rather suggesting validation of codeowners from technical point of view, if data there is accurate, etc

but you mean also issues when someone changed name or someone was not invite or did not accept invite to the repo

yeah, some monthly report could be useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants