CumulusCI icon indicating copy to clipboard operation
CumulusCI copied to clipboard

Published 20 hours ago • verified publisher icon SFDO-Tooling

Add support for GitHub Enterprise

Open davidjray opened this issue 3 years ago • 1 comments

Fixes #1129

Final Implementation Notes

  1. Split GitHub and GitHubEnterprise support into two distinct services. This maintains the historic GitHub service behavior, and adds support for GitHub Enterprise Server via a new service type.
  2. In order to support internal Github Enterprise instances we must set an ENV variable CUMULUSCI_SYSTEM_CERTS which will allow our connection to support the SSL signed by the internal certificate authority. See here.

Previous work notes:

  1. ~~With this implementation the first GitHub service that have a non-github.com domain configured are selected, therefore, this implementation does not support multiple services under a given enterprise "domain", or respect the default service.~~ Fixed by #3271 and #3275
  2. ~~The default github.com service is selected instead of the first matching service, which differs from the planned implementation, but this is better.~~ #3271 and #3275
  3. ~~We need to refactor the token factory for github services to support the domain supplied when configuring a new service. This means the token must be manually supplied via --token when connecting a new github service.~~ Not needed due to #3271
  4. ~~Supporting self signed certificates is the default for GitHub Enterprise connections, but this should be optional~~ Considered in #3287 but decided against in favor of setting CUMULUSCI_SYSTEM_CERTS

davidjray avatar Jun 24 '22 23:06 davidjray

f6e5e05 Fixes some issues I ran into when smoke testing. These fixes were required to overcome issues when configuring the first new "github_enterprise" service, and also access properties of services stored in the keychain.

davidjray avatar Jul 13 '22 18:07 davidjray

@davidjray @jstvz @jofsky I think from above this branch is ready for merge, no? I can go ahead and approve and merge unless anyone has a concern.

davidmreed avatar Oct 07 '22 17:10 davidmreed

@davidmreed:

I think from above this branch is ready for merge, no? I can go ahead and approve and merge unless anyone has a concern.

Yeah, looks like it slipped the net when the WI was closed. Please approve.

jstvz avatar Oct 07 '22 23:10 jstvz