feat: add github_code_owner table from .CODEOWNERS file in a repository #200

aminvielledebatAtBedrock · 2022-09-27T14:03:30Z

Description

Capture the content of .CODEOWNERS file (in root, or in .github or docs directories), and expose it in a new table

How

Use the Github API client.Repositories.GetContents

Example query results

Results

select 
    row_number() over (order by pattern, users, teams)
    ,repository_full_name
    ,pattern
    ,users
    ,teams
    ,pre_comments
    ,line_comment
from github.github_code_owner where repository_full_name = 'my-orga/code-owners'

row_number	repository_full_name	pattern	users	teams	pre_comments	line_comment
1	amy-orga/code-owners	.cloud	["@super-user","@super-user"]		["# another comment","# hello"]	line comment here
2	amy-orga/code-owners	.cloud/terraform	["@super-user"]		["# another comment","# hello"]	line comment here 2
3	amy-orga/code-owners	.github	["@super-user"]	["@my-orga/my-team"]	["# fooo Comment","# here"]
4	amy-orga/code-owners	README	["@super-user"]		["# fooo Comment","# here"]

Dod

~~Unit tests are written and everything is fine~~ (don't find any test in this repo 😢)
update documentation
test it in a production environnement : Yes with more other 1000 repositories

cbruno10 · 2022-09-27T19:52:52Z

Hey @aminvielledebatAtBedrock , thanks for working on this PR!

I think adding a way to get information from a repo's CODEOWNERS makes sense. GitHub treats the CODEOWNERS file as a key file, so I'm thinking maybe this could be a new table entirely, instead of being hidden among the other columns in a repo. If it were a table, it'd also be easier to query, with one line in the file per row instead of in a large JSON value.

I think a suitable table name would be github_code_owner and for columns:

Instead of path, should that column be called pattern? In the CODEOWNERS syntax doc, they refer to each rule as having a path and owners, e.g.,
```
*.txt @octo-org/octocats
```
Since teams can also be set as owners, maybe we could separate out users, e.g., @cbruno10, user@test.com, into a users column and teams, e.g., @octo-org/octocats, into a teams column (both are list of strings).
Each rule can also have a comment (single or multi-line), so could we include those in a few comments:
- {Name: "pre_comments", Type: proto.ColumnType_JSON, Description: "Specifies the comments added above a key."}
- {Name: "head_comment", Type: proto.ColumnType_STRING, Description: "Specifies the comment in the lines preceding the node and not separated by an empty line."}
- {Name: "line_comment", Type: proto.ColumnType_STRING, Description: "Specifies the comment at the end of the line where the node is in."}
- {Name: "foot_comment", Type: proto.ColumnType_STRING, Description: "Specifies the comment following the node and before empty lines."}
I think order is important, as the last matching pattern takes the most precedence, so could we also add a line column, which is an int and represents which line number it's on (I'm not sure if a rule can traverse multiple lines though)?

If you have any questions, please let me know, thanks!

aminvielledebatAtBedrock · 2022-09-28T08:13:33Z

Good point for the new table @cbruno10 . I will add it !

About comments, what's the difference between head_coment and pre_comment for you ? IMO, it's the same thing 😢
And, how do you want to manage this kind of comments ?

# In this example, @octocat owns any file in the `/apps`
# directory in the root of your repository except for the `/apps/github`
# subdirectory, as its owners are left empty.
/apps/ @octocat
/apps/github

Do you think head_comment and pre_comment should be replicated for pattern /apps/ and /app/github.

Last point about foot_comment : i don't see any foot comment in the documentation. Do you think it's relevant ?
Same question : do we have to replicate this comment for each pattern ?

Thanks for your support !

cbruno10 · 2022-09-29T18:54:45Z

@aminvielledebatAtBedrock - I agree with your comments about the comment columns, so I think we can probably simplify to just pre_comments and line_comment for now.

Also, for the example you pasted, I believe yes, the values for the pre_comments column for the /apps/ and /app/github rows should be the same.

aminvielledebatAtBedrock · 2022-09-29T20:08:40Z

🙏 thank you so much for your responses @cbruno10 !
I updated my code with the following columns :

func gitHubCodeOwnerColumns() []*plugin.Column {
	return []*plugin.Column{
		// Top columns
		{Name: "full_name", Type: proto.ColumnType_STRING, Description: "The full name of the repository, including the owner and repo name."},
		// Other columns
		{Name: "pattern", Type: proto.ColumnType_STRING, Description: "The pattern used to identify what code a team, or an individual is responsible for"},
		{Name: "users", Type: proto.ColumnType_JSON, Description: "Users responsible for code in the repo", Transform: transform.FromField("Users")},
		{Name: "teams", Type: proto.ColumnType_JSON, Description: "Teams responsible for code in the repo", Transform: transform.FromField("Teams")},
		{Name: "comments", Type: proto.ColumnType_JSON, Description: "Specifies the comments added above a key.", Transform: transform.FromField("Comments")},
	}
}

I also removed line comments because i don't see any example in the documentation, or in .gitignore files (CODEOWNERS have the same syntax)

If everything looks good to you, I can add the missing documentation.

cbruno10 · 2022-09-29T20:13:28Z

@aminvielledebatAtBedrock Thanks for the quick changes!

I think it'd still be good to have the pre_comments and line_comment columns. Though there aren't a lot of examples out there, GitHub announced support for line columns in this blog post, and being able to differentiate between them is sometimes helpful.

Also, can you please rename full_name to repository_full_name, as I believe all of our tables use that column name, so this table should be consistent with them.

aminvielledebatAtBedrock · 2022-09-30T09:03:47Z

You're absolutely right @cbruno10 ! I didn't see this blog post and this new feature.
Thank you for your support about this PR !

Documentation added
Column full_name renamed to repository_full_name
pre_comments and line_command as described

Feel free to ask me other changes 👍

cbruno10

@aminvielledebatAtBedrock Please see additional suggestions/questions, thanks!

cbruno10 · 2022-09-30T20:04:04Z

docs/tables/github_code_owner.md

+### Get All your CodeOwners rules about a specific repository
+
+```sql
+select 


@aminvielledebatAtBedrock Can you please format this query using https://www.freeformatter.com/sql-formatter.html? There's more information about our doc example standards in https://steampipe.io/docs/develop/table-docs-standards#style-conventions-1.

Thanks for the links !

cbruno10 · 2022-09-30T20:06:09Z

docs/tables/github_code_owner.md

+    ,teams
+    ,pre_comments
+    ,line_comment
+from github.github_code_owner where repository_full_name = 'my-orga/code-owners'


Suggested change

from github.github_code_owner where repository_full_name = 'my-orga/code-owners'

from github_code_owner where repository_full_name = 'my-orga/code-owners'

For examples, we usually don't include the connection name, just the table name is sufficient (you can run queries like this too).

Sorry about that, you're right. Fixed

github/table_github_code_owner.go

cbruno10 · 2022-09-30T20:08:55Z

github/table_github_code_owner.go

+		client := connect(ctx, d)
+		opt := &github.RepositoryContentGetOptions{}
+		// stop on the first found CODEOWNERS file
+		var paths = []string{"CODEOWNERS", ".github/CODEOWNERS", "docs/CODEOWNERS"}


Can a repo have multiple CODEOWNERS files (even in GitHub throws up warnings in the UI)? If so, which one is used by the table (looks like it's the order defined in the array)? Is there a precedence that GitHub has on which CODEOWNERS file it will use?

I did some tests on a repo : there is no composition between files (some rules from a file, and others from another) and there is a precedence :

.github/CODEOWNERS

CODEOWNERS

docs/CODEOWNERS

I changed my code to respect this order.

Thanks for checking @aminvielledebatAtBedrock , can you please also include a comment that has a link where it specifies this for future reference?

Comment added !
Thanks for the review !

cbruno10

@aminvielledebatAtBedrock Thanks again for the quick updates! I left one more round of suggestions, but I believe after these are addressed, the PR will be ready for merge. Thanks!

cbruno10 · 2022-10-03T15:08:46Z

docs/tables/github_code_owner.md

+  pre_comments,
+  line_comment 
+from
+  github.github_code_owner 


Suggested change

github.github_code_owner

github_code_owner

github/table_github_code_owner.go

cbruno10 · 2022-10-03T15:29:36Z

docs/tables/github_code_owner.md

+from
+  github.github_code_owner 
+where
+  repository_full_name = 'my-orga/code-owners' 


Suggested change

repository_full_name = 'my-orga/code-owners'

repository_full_name = 'github/docs'

I think linking to a real example instead of having users search out repos with a CODEOWNERS file will make it easier for users to use this table and understand's returned

cbruno10 · 2022-10-03T15:33:39Z

github/table_github_code_owner.go

+
+//// TABLE DEFINITION
+
+func tableGithubCodeOwner() *plugin.Table {


Suggested change

func tableGithubCodeOwner() *plugin.Table {

func tableGitHubCodeOwner() *plugin.Table {

cbruno10 · 2022-10-03T20:49:10Z

@aminvielledebatAtBedrock Thanks again so much for submitting this PR and putting up with all of my reviews 😅

We're doing another GitHub plugin release later this week, so this table will be included in it.

aminvielledebatAtBedrock · 2022-10-03T20:55:25Z

@cbruno10 Thanks again for your support and your patience 😆
It's my first PR on steampipe and i'm very glad to work with you :-)

For the github release : i'm working on another table if you're interested : github_repository_content. To be continued

aminvielledebatAtBedrock force-pushed the feat/add-code-owners branch 3 times, most recently from fbd4f3b to d6003a8 Compare September 29, 2022 20:05

aminvielledebatAtBedrock changed the title ~~feat: add code_owners column from .CODEOWNERS file in tables github_my_repository and github_repository~~ feat: add github_code_owner table from .CODEOWNERS file in a repository Sep 29, 2022

aminvielledebatAtBedrock force-pushed the feat/add-code-owners branch 4 times, most recently from c99f7a3 to 9b37c68 Compare September 30, 2022 08:59

aminvielledebatAtBedrock marked this pull request as ready for review September 30, 2022 09:03

cbruno10 requested changes Sep 30, 2022

View reviewed changes

aminvielledebatAtBedrock force-pushed the feat/add-code-owners branch 3 times, most recently from 1d9d1fd to 790c7c2 Compare October 2, 2022 20:04

aminvielledebatAtBedrock requested a review from cbruno10 October 2, 2022 20:29

aminvielledebatAtBedrock force-pushed the feat/add-code-owners branch from 790c7c2 to 1464037 Compare October 3, 2022 07:22

cbruno10 requested changes Oct 3, 2022

View reviewed changes

aminvielledebatAtBedrock force-pushed the feat/add-code-owners branch from 1464037 to d2bd67f Compare October 3, 2022 19:49

feat: add github_code_owner based on CODEOWNERS files

b4aa1fd

aminvielledebatAtBedrock force-pushed the feat/add-code-owners branch from d2bd67f to b4aa1fd Compare October 3, 2022 20:20

aminvielledebatAtBedrock requested a review from cbruno10 October 3, 2022 20:29

cbruno10 approved these changes Oct 3, 2022

View reviewed changes

cbruno10 merged commit 09afbe3 into turbot:main Oct 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add github_code_owner table from .CODEOWNERS file in a repository #200

feat: add github_code_owner table from .CODEOWNERS file in a repository #200

aminvielledebatAtBedrock commented Sep 27, 2022 •

edited by cbruno10

Loading

cbruno10 commented Sep 27, 2022 •

edited

Loading

aminvielledebatAtBedrock commented Sep 28, 2022 •

edited

Loading

cbruno10 commented Sep 29, 2022

aminvielledebatAtBedrock commented Sep 29, 2022

cbruno10 commented Sep 29, 2022 •

edited

Loading

aminvielledebatAtBedrock commented Sep 30, 2022

cbruno10 left a comment

cbruno10 Sep 30, 2022

aminvielledebatAtBedrock Oct 2, 2022

cbruno10 Sep 30, 2022

aminvielledebatAtBedrock Oct 2, 2022

cbruno10 Sep 30, 2022

aminvielledebatAtBedrock Oct 2, 2022

cbruno10 Oct 3, 2022

aminvielledebatAtBedrock Oct 3, 2022

cbruno10 left a comment

cbruno10 Oct 3, 2022

cbruno10 Oct 3, 2022

cbruno10 Oct 3, 2022

cbruno10 Oct 3, 2022

cbruno10 commented Oct 3, 2022

aminvielledebatAtBedrock commented Oct 3, 2022 •

edited

Loading

	from github.github_code_owner where repository_full_name = 'my-orga/code-owners'
	from github_code_owner where repository_full_name = 'my-orga/code-owners'

	repository_full_name = 'my-orga/code-owners'
	repository_full_name = 'github/docs'


		//// TABLE DEFINITION

		func tableGithubCodeOwner() *plugin.Table {

feat: add github_code_owner table from .CODEOWNERS file in a repository #200

feat: add github_code_owner table from .CODEOWNERS file in a repository #200

Conversation

aminvielledebatAtBedrock commented Sep 27, 2022 • edited by cbruno10 Loading

Description

How

Example query results

Dod

cbruno10 commented Sep 27, 2022 • edited Loading

aminvielledebatAtBedrock commented Sep 28, 2022 • edited Loading

cbruno10 commented Sep 29, 2022

aminvielledebatAtBedrock commented Sep 29, 2022

cbruno10 commented Sep 29, 2022 • edited Loading

aminvielledebatAtBedrock commented Sep 30, 2022

cbruno10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbruno10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbruno10 commented Oct 3, 2022

aminvielledebatAtBedrock commented Oct 3, 2022 • edited Loading

aminvielledebatAtBedrock commented Sep 27, 2022 •

edited by cbruno10

Loading

cbruno10 commented Sep 27, 2022 •

edited

Loading

aminvielledebatAtBedrock commented Sep 28, 2022 •

edited

Loading

cbruno10 commented Sep 29, 2022 •

edited

Loading

aminvielledebatAtBedrock commented Oct 3, 2022 •

edited

Loading