migueleliasweb / go-github-mock

A library to aid unittesting code that uses Golang's Github SDK
MIT License
100 stars 25 forks source link

feat: Support GitHub Enterprise Server #62

Open ponkio-o opened 8 months ago

ponkio-o commented 8 months ago

@migueleliasweb Hi, thank you for development this library!

I'm using GitHub Enterprise Server (v3.10) in my company, so I want mock for this. Could you supporting GitHub Enterprise Server?


I note that I have investigated the current code and API schema a bit.

It seems, GitHub API Schema exist for each GitHub edition.

The json has each for API schema independently. It's probably same definition exist between these files. For example, the /repos/{owner}/{repo}/actions/artifacts endpoint exist on all editions.

migueleliasweb commented 8 months ago

Hi @ponkio-o , it looks like this can be achieved but would require some substantial rework on some of the code generation tooling and for the server to load the correct definitions.

Do you have a plan on how to implement this?

ponkio-o commented 8 months ago

Do you have a plan on how to implement this?

Sorry, I don't have now. For now, I checked to run correctly that applying the following diff on our GitHub Enterprise Server v3.10.

- const GITHUB_OPENAPI_DEFINITION_LOCATION = "https://github.com/github/rest-api-description/blob/main/descriptions/api.github.com/api.github.com.json?raw=true"
+ const GITHUB_OPENAPI_DEFINITION_LOCATION = "https://github.com/github/rest-api-description/blob/main/descriptions/ghes-3.10/ghes-3.10.json?raw=true"

After I ran below command to generate mock code.

$ go run ./main.go
func setupGitHubMock() *github.Client {
    mockedHTTPClient := gmock.NewMockedHTTPClient(
        gmock.WithRequestMatchEnterprise(
            gmock.GetEnterprisesActionsRunnersByEnterprise,
            github.Runners{
                TotalCount: 1,
                Runners: []*github.Runner{
                    {
                        ID:     github.Int64(1),
                        Name:   github.String("mock-runner-01"),
                        Status: github.String("online"),
                        Busy:   github.Bool(false),
                        Labels: []*github.RunnerLabels{
                            {
                                ID:   github.Int64(1),
                                Name: github.String("mock-small"),
                            },
                        },
                    },
                },
            },
        ),
    )

    client, err := github.NewClient(mockedHTTPClient).WithEnterpriseURLs("https://github.example.jp", "https://github.example.jp")
    if err != nil {
        log.Fatal(err)
    }

    return client
}
...some code

I don't development Golang library until now, so I don't have idea for implementation. But, I think that we have some point to consider that when implement this. If you have any good ideas, please let me know.

The GitHub has different schema for each server version and edition. See descriptions/.

So what do you think about splitting the directory by edition (version/ediction) and creating a separate endpointpattern.go for each? I'm not familiar with Golang, so I don't know if such a configuration is possible.

.
├── hack
└── src
    ├── gen
    └── mock
        ├── api.github.com
        ├── ghec
        ├── ghes-2.18
        ├── ghes-2.19
        └── ghes-2.20

In this case, it would only be necessary to create a file for each directory that is being generated by gen.go.

One point of concern is, how to handling below code. These are generated by manual operation?? https://github.com/migueleliasweb/go-github-mock/blob/be5e2c518c3de9efc002f4abcc30ac2ed58fcff7/src/gen/gen_mutations.go#L9-L17

migueleliasweb commented 8 months ago

So what do you think about splitting the directory by edition (version/ediction) and creating a separate endpointpattern.go for each? I'm not familiar with Golang, so I don't know if such a configuration is possible.

.
├── hack
└── src
    ├── gen
    └── mock
        ├── api.github.com
        ├── ghec
        ├── ghes-2.18
        ├── ghes-2.19
        └── ghes-2.20

Yeah, it would have to be something similar to this. The trick here is how to provide a clear way for the user to know which user is in use.

I was thinking on something like this:

mockedHTTPClient := mock.NewMockedHTTPClientWithOptions(
    mock.Options(
        mock.WithGHES220Endpoints(),
        mock.WithApiThrottlingOptions(),
    ),
    mock.WithRequestMatch(
        mock.GetUsersByUsername,
        github.User{
            Name: github.String("foobar"),
        },
    ),
)

By creating NewMockedHTTPClientWithOptions, a backwards compatible way could be maintained.

In this case, it would only be necessary to create a file for each directory that is being generated by gen.go.

One point of concern is, how to handling below code. These are generated by manual operation??

https://github.com/migueleliasweb/go-github-mock/blob/be5e2c518c3de9efc002f4abcc30ac2ed58fcff7/src/gen/gen_mutations.go#L9-L17

Yeah, those are manual currently. I think there might be a way to create some logic to automatic handle them.

Let me know what do you think. Cheers.

ponkio-o commented 8 months ago

Thank you for your response!

Yeah, it would have to be something similar to this. The trick here is how to provide a clear way for the user to know which user is in use.

I was thinking on something like this:

mockedHTTPClient := mock.NewMockedHTTPClientWithOptions(
    mock.Options(
        mock.WithGHES220Endpoints(),
        mock.WithApiThrottlingOptions(),
    ),
    mock.WithRequestMatch(
        mock.GetUsersByUsername,
        github.User{
            Name: github.String("foobar"),
        },
    ),
)

Wow! I think good interface the above code! But I have no idea how to implement this...😅

Another method I came up with was to add a version or edition prefix to increase the variety of EndpointPatterns. This method is the easiest to modify because all you have to do increase the number of scraping targets and add prefix.

like this:

GHES310getEnterprisesSecretScanningAlertsByEnterprise

However, this method is so redundant that I thought it best if the method you suggested is feasible 😂

Yeah, those are manual currently. I think there might be a way to create some logic to automatic handle them.

Let me know what do you think. Cheers.

I see. But maybe we can think about it here when we need it.