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.

Sign up for GitHub

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

Jump to bottom

Correct documentation of provider configuration, and deprecate organization argument #735

Merged
merged 4 commits into from
Apr 9, 2021
Merged

Correct documentation of provider configuration, and deprecate organization argument #735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
00dc0e0
Remove dead function
tibbes Mar 19, 2021
aa6f167
Remove redundant call to os.Getenv
tibbes Mar 19, 2021
c1a4049
Simplify github.Config struct
tibbes Mar 19, 2021
878ebb8
Deprecate organization and GITHUB_ORGANIZATION
tibbes Mar 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 10 additions & 72 deletions github/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@ import (
)

type Config struct {
Token string
Owner string
Organization string
BaseURL string
Insecure bool
Individual bool
Anonymous bool
Token string
Owner string
BaseURL string
Insecure bool
}

type Owner struct {
Expand Down Expand Up @@ -53,6 +50,10 @@ func (c *Config) AuthenticatedHTTPClient() *http.Client {

}

func (c *Config) Anonymous() bool {
return c.Token == ""
}

func (c *Config) AnonymousHTTPClient() *http.Client {
client := &http.Client{Transport: &http.Transport{}}
return RateLimitedHTTPClient(client)
Expand Down Expand Up @@ -126,7 +127,7 @@ func (c *Config) ConfigureOwner(owner *Owner) (*Owner, error) {
func (c *Config) Meta() (interface{}, error) {

var client *http.Client
if c.Anonymous {
if c.Anonymous() {
client = c.AnonymousHTTPClient()
} else {
client = c.AuthenticatedHTTPClient()
Expand All @@ -146,73 +147,10 @@ func (c *Config) Meta() (interface{}, error) {
owner.v4client = v4client
owner.v3client = v3client

if c.Anonymous {
if c.Anonymous() {
return &owner, nil
} else {
return c.ConfigureOwner(&owner)
}

}

// Clients configures and returns a fully initialized GithubClient and Githubv4Client
func (c *Config) Clients() (interface{}, error) {
var owner Owner
var ts oauth2.TokenSource
var tc *http.Client

ctx := context.Background()

ts = oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: c.Token},
)
tc = oauth2.NewClient(ctx, ts)
tc.Transport = NewEtagTransport(tc.Transport)
tc.Transport = NewRateLimitTransport(tc.Transport)
tc.Transport = logging.NewTransport("Github", tc.Transport)

// Create GraphQL Client
uv4, err := url.Parse(c.BaseURL)
if err != nil {
return nil, err
}
uv4.Path = path.Join(uv4.Path, "graphql")
v4client := githubv4.NewEnterpriseClient(uv4.String(), tc)

// Create Rest Client
uv3, err := url.Parse(c.BaseURL)
if err != nil {
return nil, err
}
if uv3.String() != "https://api.github.com/" {
uv3.Path = uv3.Path + "v3/"
}
v3client, err := github.NewEnterpriseClient(uv3.String(), "", tc)
if err != nil {
return nil, err
}
v3client.BaseURL = uv3

owner.v3client = v3client
owner.v4client = v4client

owner.name = c.Owner
if owner.name == "" {
// Discover authenticated user
user, _, err := owner.v3client.Users.Get(ctx, "")
if err != nil {
return nil, err
}
owner.name = user.GetLogin()
} else {
remoteOrg, _, err := owner.v3client.Organizations.Get(ctx, owner.name)
if err == nil {
if remoteOrg != nil {
owner.id = remoteOrg.GetID()
owner.IsOrganization = true
}
} else {
return nil, err
}
}
return &owner, nil
}
26 changes: 11 additions & 15 deletions github/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestAccConfigMeta(t *testing.T) {

t.Run("returns an anonymous client for the v3 REST API", func(t *testing.T) {

config := Config{Anonymous: true, BaseURL: "https://api.github.com/"}
config := Config{BaseURL: "https://api.github.com/"}
meta, err := config.Meta()
if err != nil {
t.Fatalf("failed to return meta without error: %s", err.Error())
Expand All @@ -41,9 +41,8 @@ func TestAccConfigMeta(t *testing.T) {
t.Run("returns a v3 REST API client to manage individual resources", func(t *testing.T) {

config := Config{
Token: testToken,
BaseURL: "https://api.github.com/",
Anonymous: false,
Token: testToken,
BaseURL: "https://api.github.com/",
}
meta, err := config.Meta()
if err != nil {
Expand All @@ -62,9 +61,8 @@ func TestAccConfigMeta(t *testing.T) {
t.Run("returns a v4 GraphQL API client to manage individual resources", func(t *testing.T) {

config := Config{
Token: testToken,
BaseURL: "https://api.github.com/",
Anonymous: false,
Token: testToken,
BaseURL: "https://api.github.com/",
}
meta, err := config.Meta()
if err != nil {
Expand All @@ -87,10 +85,9 @@ func TestAccConfigMeta(t *testing.T) {
t.Run("returns a v3 REST API client to manage organization resources", func(t *testing.T) {

config := Config{
Token: testToken,
BaseURL: "https://api.github.com/",
Organization: testOrganization,
Anonymous: false,
Token: testToken,
BaseURL: "https://api.github.com/",
Owner: testOrganization,
}
meta, err := config.Meta()
if err != nil {
Expand All @@ -109,10 +106,9 @@ func TestAccConfigMeta(t *testing.T) {
t.Run("returns a v4 GraphQL API client to manage organization resources", func(t *testing.T) {

config := Config{
Token: testToken,
BaseURL: "https://api.github.com/",
Organization: testOrganization,
Anonymous: false,
Token: testToken,
BaseURL: "https://api.github.com/",
Owner: testOrganization,
}
meta, err := config.Meta()
if err != nil {
Expand Down
44 changes: 25 additions & 19 deletions github/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ func Provider() terraform.ResourceProvider {
"owner": {
Type: schema.TypeString,
Optional: true,
DefaultFunc: OwnerOrOrgEnvDefaultFunc,
DefaultFunc: schema.EnvDefaultFunc("GITHUB_OWNER", nil),
Description: descriptions["owner"],
},
"organization": {
Type: schema.TypeString,
Optional: true,
DefaultFunc: OwnerOrOrgEnvDefaultFunc,
DefaultFunc: schema.EnvDefaultFunc("GITHUB_ORGANIZATION", nil),
Description: descriptions["organization"],
Deprecated: "Use owner (or GITHUB_OWNER) instead of organization (or GITHUB_ORGANIZATION)",
},
"base_url": {
Type: schema.TypeString,
Expand Down Expand Up @@ -113,29 +114,34 @@ func init() {

func providerConfigure(p *schema.Provider) schema.ConfigureFunc {
return func(d *schema.ResourceData) (interface{}, error) {
anonymous := true
if d.Get("token").(string) != "" {
anonymous = false
}
owner := d.Get("owner").(string)

individual := true
if d.Get("organization").(string) != "" {
individual = false
// BEGIN backwards compatibility
// OwnerOrOrgEnvDefaultFunc used to be the default value for both
// 'owner' and 'organization'. This meant that if 'owner' and
// 'GITHUB_OWNER' were set, 'GITHUB_OWNER' would be used as the default
// value of 'organization' and therefore override 'owner'.
//
// This seems undesirable (an environment variable should not override
// an explicitly set value in a provider block), but is necessary
// for backwards compatibility. We could remove this backwards compatibility
// code in a future major release.
env, _ := OwnerOrOrgEnvDefaultFunc()
if env.(string) != "" {
owner = env.(string)
}
// END backwards compatibility

owner := d.Get("owner").(string)
if !individual {
owner = d.Get("organization").(string)
org := d.Get("organization").(string)
if org != "" {
owner = org
}

config := Config{
Token: d.Get("token").(string),
Organization: d.Get("organization").(string),
BaseURL: d.Get("base_url").(string),
Insecure: d.Get("insecure").(bool),
Owner: owner,
Individual: individual,
Anonymous: anonymous,
Token: d.Get("token").(string),
BaseURL: d.Get("base_url").(string),
Insecure: d.Get("insecure").(bool),
Owner: owner,
}

meta, err := config.Meta()
Expand Down
2 changes: 1 addition & 1 deletion github/provider_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func testAccCheckOrganization() error {

func OwnerOrOrgEnvDefaultFunc() (interface{}, error) {
if organization := os.Getenv("GITHUB_ORGANIZATION"); organization != "" {
return os.Getenv("GITHUB_ORGANIZATION"), nil
return organization, nil
}
return os.Getenv("GITHUB_OWNER"), nil
}
Expand Down
19 changes: 16 additions & 3 deletions website/docs/index.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,21 @@ The following arguments are supported in the `provider` block:

* `token` - (Optional) A GitHub OAuth / Personal Access Token. When not provided or made available via the `GITHUB_TOKEN` environment variable, the provider can only access resources available anonymously.

* `base_url` - (Optional) This is the target GitHub base API endpoint. Providing a value is a requirement when working with GitHub Enterprise. It is optional to provide this value and it can also be sourced from the `GITHUB_BASE_URL` environment variable. The value must end with a slash, for example: `https://terraformtesting-ghe.westus.cloudapp.azure.com/`
* `base_url` - (Optional) This is the target GitHub base API endpoint. Providing a value is a requirement when working with GitHub Enterprise. It is optional to provide this value and it can also be sourced from the `GITHUB_BASE_URL` environment variable. The value must end with a slash, for example: `https://terraformtesting-ghe.westus.cloudapp.azure.com/`

* `owner` - (Optional) This is the target GitHub individual account to manage. It is optional to provide this value and it can also be sourced from the `GITHUB_OWNER` environment variable. For example, `torvalds` is a valid owner. When not provided and a `token` is available, the individual account owning the `token` will be used. When not provided and no `token` is available, the provider may not function correctly. Conflicts with `organization`.
* `owner` - (Optional) This is the target GitHub organization or individual user account to manage. For example, `torvalds` and `github` are valid owners. It is optional to provide this value and it can also be sourced from the `GITHUB_OWNER` environment variable. When not provided and a `token` is available, the individual user account owning the `token` will be used. When not provided and no `token` is available, the provider may not function correctly.

* `organization` - (Optional) This is the target GitHub organization account to manage. It is optional to provide this value and it can also be sourced from the `GITHUB_ORGANIZATION` environment variable. For example, `github` is a valid organization. Conflicts with `owner`and requires `token`, as the individual account corresponding to provided `token` will need "owner" privileges for this organization.
* `organization` - (Deprecated) This behaves the same as `owner`, which should be used instead. This value can also be sourced from the `GITHUB_ORGANIZATION` environment variable.

For backwards compatibility, if more than one of `owner`, `organization`,
`GITHUB_OWNER` and `GITHUB_ORGANIZATION` are set, the first in this
list takes priority.

1. Setting `organization` in the GitHub provider configuration.
2. Setting the `GITHUB_ORGANIZATION` environment variable.
3. Setting the `GITHUB_OWNER` environment variable.
4. Setting `owner` in the GitHub provider configuration.

~> It is a bug that `GITHUB_OWNER` takes precedence over `owner`, which may
be fixed in a future major release. For compatibility with future releases,
please set only one of `GITHUB_OWNER` and `owner`.