Support for GitHub Enterprise closes #70 #160

mheiges · 2022-05-06T01:39:41Z

Example query results

Results

Add example SQL query results here (please include the input queries as well)

turbot#70

…rt-update

e-gineer · 2022-05-06T02:04:26Z

Quick note that I'm excited to see support for GitHub enterprise - thanks @mheiges! Have wanted this for a long time, but didn't have the ability to test.

mheiges · 2022-05-06T02:12:01Z

some sample queries run against GHE, mostly verifying that repository_full_name is correct.

steampipe query "select name, repository_full_name from github_search_code  where query = 'filename:build.Dockerfile path:automation/docker'"

+------------------+-------------------------------------------+
| name             | repository_full_name                      |
+------------------+-------------------------------------------+
| build.Dockerfile | myORG/myorg-ui                            |
| build.Dockerfile | myORG/myorg-rpm-service                   |
| build.Dockerfile | myORG/myorg-certification-service         |
| build.Dockerfile | myORG/myorg-report-service                |

steampipe query "select title, repository_full_name from github_search_issue  where query = 'is:open repo:myORG/myorg-foo-service'"
+-------------------------+-----------------------------+
| title                   | repository_full_name        |
+-------------------------+-----------------------------+
| yeah, this is a problem | myORG/myorg-foo-service     |
+-------------------------+-----------------------------+

steampipe query "select title, repository_full_name from github_search_pull_request  where query = 'is:open repo:myORG/myorg-foo-service'"
+----------------------------+-----------------------------+
| title                      | repository_full_name        |
+----------------------------+-----------------------------+
| build with lockfile        | myORG/myorg-foo-service     |
+----------------------------+-----------------------------+

steampipe query "select description, repository_full_name from github_search_label where repository_id = 3753 and query = 'bug'"
+-------------------------+-----------------------------+
| description             | repository_full_name        |
+-------------------------+-----------------------------+
| Something isn't working | myORG/myorg-foo-service     |
+-------------------------+-----------------------------+

cbruno10

@mheiges Sorry for the slight delay in reviewing this PR, I've added a few questions/suggestions, can you please have a look when you get a chance? Also, can you please resolve the merge conflicts? Thanks!

github/utils.go

cbruno10 · 2022-05-23T20:55:56Z

github/utils.go

 	}

 	if token == "" {
-		panic("'token' must be set in the connection configuration. Edit your connection configuration file and then restart Steampipe")
+		panic("token must be configured")


Is there a specific reason you changed the panic message? This is a fairly standard message we use in other plugins as well.

I did not make this change, this came from the github-enterprise-support branch

cbruno10 · 2022-05-23T20:56:06Z

github/utils.go

 	}

 	if token == "" {
-		panic("'token' must be set in the connection configuration. Edit your connection configuration file and then restart Steampipe")
+		panic("token must be configured")
+		//return nil, errors.New("token must be configured")


Suggested change

//return nil, errors.New("token must be configured")

github/utils.go

cbruno10 · 2022-05-23T21:08:45Z

github/utils.go

+			uv3.Path = uv3.Path + "api/v3/"
+		}
+
+		conn, err = github.NewEnterpriseClient(uv3.String(), "", tc)


Is there any future compatibility you're aware of that wouldn't work if we don't pass the upload URL when creating the client?

I don't know. This came from the github-enterprise-support branch, I haven't explored the client requirements.

cbruno10 · 2022-05-23T21:10:50Z

github/utils.go

+		}
+
+		if uv3.String() != "https://api.github.com/" {
+			uv3.Path = uv3.Path + "api/v3/"


Do we need to include this, or does the go-github library automatically attach this suffix, per https://github.com/google/go-github/blob/a58d5f0adf74ef73aadaac476f7285afc77a3b43/github/github.go#L357-L364

seems to be needed. It does seem like the library should handle the suffix but it's not doing so. I don't know why offhand. This came from the github-enterprise-support branch, I haven't explored the client code.

cbruno10 · 2022-05-23T21:12:34Z

github/utils.go

+			uv3.Path = uv3.Path + "api/v3/"
+		}
+
+		conn, err = github.NewEnterpriseClient(uv3.String(), "", tc)


If someone sets base_url as https://github.com/api/v3`, does the enterprise client still work, or does it fail/have another behaviour?

base_url = "https://api.github.com/ works.

base_url = https://github.com/api/v3 returns a 404 status for https://github.com/api/v3/api/v3/search/issues

cbruno10 · 2022-05-23T21:14:23Z

github/table_github_search_code.go

-		rx := regexp.MustCompile(`(?s)` + regexp.QuoteMeta("github.com/") + `(.*?)` + regexp.QuoteMeta("/blob"))
-		replacer := strings.NewReplacer("github.com/", "", "/blob", "")
-		return replacer.Replace(rx.FindString(*code.HTMLURL)), nil
+		rx := regexp.MustCompile(`https?://.+?/(.+?)/blob`)


Have you tested this change (along with other github_search_* table changes) against a GitHub cloud instance for backward compatibility? I saw some query results in #160 (comment), but wasn't sure if those were for an enterprise or cloud instance.

the tests were against github.com (base_url = "https://api.github.com/) and GH Enterprise with custom hostname (base_url = "https://abc.xyz.gov/. I guess our GHE is Server, not Cloud - I'm not clear on the difference.

cbruno10 · 2022-05-25T20:57:12Z

@mheiges Thank you for testing out this branch and making improvements to it as well! We've just released v0.15.0 of this plugin, which includes this PR. If you see any issues, please let us know.

e-gineer and others added 3 commits December 22, 2021 08:37

Support for GitHub Enterprise through a base_url config parameter closes

7b52715

turbot#70

Merge branch 'github-enterprise-support' into github-enterprise-suppo…

ba5ba2d

…rt-update

replace hard-coded github.com with regexes for GHE support

5881574

cbruno10 self-requested a review May 6, 2022 13:23

cbruno10 mentioned this pull request May 22, 2022

Support for GitHub Enterprise #70

Closed

cbruno10 linked an issue May 22, 2022 that may be closed by this pull request

Support for GitHub Enterprise #70

Closed

cbruno10 requested changes May 23, 2022

View reviewed changes

Mark Heiges added 3 commits May 24, 2022 12:38

Merge branch 'main' into github-enterprise-support-update

fab280e

linting

5cd9fad

linting

47c2e30

cbruno10 approved these changes May 25, 2022

View reviewed changes

cbruno10 merged commit eb4dcd3 into turbot:main May 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for GitHub Enterprise closes #70 #160

Support for GitHub Enterprise closes #70 #160

mheiges commented May 6, 2022

e-gineer commented May 6, 2022

mheiges commented May 6, 2022

cbruno10 left a comment •

edited

Loading

cbruno10 May 23, 2022

mheiges May 24, 2022

cbruno10 May 23, 2022

cbruno10 May 23, 2022

mheiges May 24, 2022

cbruno10 May 23, 2022

mheiges May 24, 2022

cbruno10 May 23, 2022

mheiges May 24, 2022

cbruno10 May 23, 2022

mheiges May 24, 2022

cbruno10 commented May 25, 2022

Support for GitHub Enterprise closes #70 #160

Support for GitHub Enterprise closes #70 #160

Conversation

mheiges commented May 6, 2022

Example query results

e-gineer commented May 6, 2022

mheiges commented May 6, 2022

cbruno10 left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbruno10 commented May 25, 2022

cbruno10 left a comment •

edited

Loading