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.

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

configuring GitHub org in provider declaration doesn't work #697

Closed
thekbb opened this issue Feb 8, 2021 · 9 comments
Closed

configuring GitHub org in provider declaration doesn't work #697

thekbb opened this issue Feb 8, 2021 · 9 comments
Labels
Authentication Provider Type: Bug Something isn't working as documented

Comments

@thekbb
Copy link
Contributor

thekbb commented Feb 8, 2021

Terraform Version

Terraform v0.13.5
+ provider registry.terraform.io/hashicorp/github v4.4.0
+ provider registry.terraform.io/integrations/github v4.4.0

(there are two copies of the provider listed, as I use modules to wrap many resources. see #696)

Affected Resource(s)

github_team

Terraform Configuration Files

I define my github org and my github token in the provider config...

provider "github" {
  token        = <redacted>
  organization = <redacted>
}

terraform {
  required_version = "= 0.13.5"

  required_providers {
    github = {
      source  = "integrations/github"
      version = "= 4.4.0"
    }
  }
}

Expected Behavior

I'd expect it to create the github team

Actual Behavior

The github team can't be created, as the provider doesn't know about the org.

Error: This resource can only be used in the context of an organization, "" is a user.

Steps to Reproduce

  1. terraform apply

Important Factoids

if I set the token and the org as env vars GITHUB_TOKEN and GITHUB_ORGANIZATION everything works

References

Pretty sure #655 is the same or similar.

@jcudit jcudit added Type: Bug Something isn't working as documented Provider labels Feb 8, 2021
@jcudit jcudit added this to the v4.6.0 milestone Feb 8, 2021
@tobypinder
Copy link
Contributor

tobypinder commented Mar 12, 2021

Any updates on the project's stance on this? There does not seem to be a supported path beyond "use environment variables" which is a significant breaking change and is holding up an upgrade from v2.x.x for our org. Upgrading to versions prior to v4.2 is difficult as due to the namespace shift no documentation is available.

Attempting the Environment Variables workaround locally does not appear to be sufficient - the terraform state will now plan but resources are still failure suggesting that even setting GITHUB_TOKEN and GITHUB_ORGANIZATION is insufficient in some cases. Still looking into this and will feed back on individual issues with this once I have a reproducible test case. I got this working, unsure what the issue was but env does appear to be sufficient for now, caveats aside.

The ENV requirement breaks some use cases such as managing resources in multiple github resources from different authenticated contexts (via provider aliases) but that's not something we're struggling with.

The issue with Environment Variables being newly required instead of provider block configuration has been raised multiple times in

Due to the length of time it's been active and the simplicity in triggering it it feels like at the very least documentation should be updated to indicate a lack of support for provider configuration, and the breaking change should be identified on the changelog, since this spans multiple major revisions at this point. Then once current behavior is documented we can work towards provider configuration being either supported or deprecated (the latter of which I don't recommend personally)

@majormoses
Copy link
Contributor

majormoses commented Mar 14, 2021

Hey, I recall some of the old conversations around this when we were looking to make changes. It looks like at least the first major related change to this shipped in #255 and were largely based on the discussions and work in #191. I am curious to checkout a few things as we are still configuring the following in our org and it's working with the latest release:

provider "github" {
  version      = "~> 4.2"
  token        = var.github_token
  organization = var.github_organization
}

variable "github_organization" {
  description = "The github org you want to manage"
  default     = "REDACTED"
}

variable "github_token" {
  description = "export TF_VAR_github_token=$MY_TOKEN"
}

It sounds like we are silently swallowing that config and perhaps we already have GITHUB_TOKEN env var in our CI setup? I will check on that this week. If so I agree that's a disturbing behavior change I was not aware of. Notice that this is supposed to work even though the token name is not GITHUB_TOKEN, instead I used TF_VAR_github_token which should auto be picked up and use as input. I highly doubt we would have set GITHUB_ORGANIZATION in CI so I think this setup might still be working with this config path but I could be wrong.

@rogersd
Copy link

rogersd commented Mar 16, 2021

Just to throw my two cents in, I'm also having this problem, but only intermittently. Sometimes it works completely, sometimes it seems to pick up organization, sometimes token, sometimes neither, sometimes both and it works properly.

There's no rhyme nor reason to it, as I have five identical environments that all use the same config, and it completely worked in one, but the other four had various issues.

I don't have any of the GITHUB environment variables set, but I might have to add them to work around this.

@majormoses
Copy link
Contributor

🤔 that/this is odd indeed. I locally ensured that no env vars set, it forced a prompt. I could not replicate this with a number of states in my org. I checked our CI and we are not deploying an env var named GITHUB_TOKEN or GITHUB_ORGANIZATION.

Due to the length of time it's been active and the simplicity in triggering it it feels like at the very least documentation should be updated to indicate a lack of support for provider configuration, and the breaking change should be identified on the changelog, since this spans multiple major revisions at this point. Then once current behavior is documented we can work towards provider configuration being either supported or deprecated (the latter of which I don't recommend personally)

I am unfortunately unable to replicate this behavior and tried multiple sets of configuration options such as hardcoding the values directly in the provider config and using terraform variables. I tested using env vars, local tfvars, and specifying a default value in variables.tf.

The error you are seeing is being triggered here:

func checkOrganization(meta interface{}) error {
if !meta.(*Owner).IsOrganization {
return fmt.Errorf("This resource can only be used in the context of an organization, %q is a user.", meta.(*Owner).name)
}

The owner object is defined here:

type Owner struct {
name string
id int64
v3client *github.Client
v4client *githubv4.Client
StopContext context.Context
IsOrganization bool
}

My ability to grok past that is limited, maybe someone who is more knowledgeable than myself in this matter might be helped by testing and investigation.

@tobypinder
Copy link
Contributor

tobypinder commented Mar 19, 2021

The only insight I can add there is that this seems to be limited to a number of resources (and presumably API calls). Anecdotally it seems related to data github_repository and github_branch, that and the resource github_branch_protection* ones.

I'm not really au fait with the Go internals of terraform but I'm going to try and come back with a reproduction case based on a synthetic setup with 2 orgs/org + personal and use of alias to try and trigger this, since I've also had trouble with reproducing this in a smaller state that I can share.

@tibbes
Copy link
Contributor

tibbes commented Mar 20, 2021

I don't think this is a bug in the provider.

I've spent hours trying to reproduce this, and I can only trigger the error message that @thekbb is seeing with a configuration that uses both the hashicorp/github and integrations/github providers.

@thekbb, @rogersd, @tobypinder, here is the setup I've created that triggers this problem. Please can you compare this to your terraform configurations where you see this issue? I suspect it's possible to trigger this in other ways and I'd like to know how your setups are different.

Terraform configuration to reproduce the problem

Create a tree with three files:

  • main.tf
  • module1/main.tf
  • module2/main.tf

The content of the files is:

./main.tf

provider "github" {
  token = "REDACTED"
  organization = "REDACTED"
}

module "a" {
  source = "./module1"
}

module "b" {
  source = "./module2"
}

./module1/main.tf

terraform {
  required_providers {
    github = {
      source  = "hashicorp/github"
      version = "4.4.0"
    }
  }
}

./module2/main.tf

terraform {
  required_providers {
    github = {
      source  = "integrations/github"
      version = "4.5.1"
    }
  }
}

resource "github_team" "test" {
  name = "Test2"
}

Output

Error: This resource can only be used in the context of an organization, "" is a user.

  on module2/main.tf line 10, in resource "github_team" "test":
  10: resource "github_team" "test" {

Explanation

There are two different providers with the name github in this configuration (hashicorp/github and integrations/github), but there is one provider "github" section in main.tf. This configuration is not passed to both providers. In my tests the configuration is always passed to hashicorp/github (if I move the github_team resource to the other module it is created just fine).

The reason why environment variables work is that the terraform providers are run by terraform so they are child processes and inherit the same environment. Both providers can read the environment just fine, so both are configured correctly. In contrast, the provider "github" section of HCL is read by terraform and explicitly passed to just one provider by RPC. For the other provider, it is as if it has an empty configuration with no token and no organization.

Unfortunately (for diagnosing this issue), the github provider supports not providing a token. It runs in anonymous mode, and can only perform actions that unauthenticated users can perform. That is why the error message says "" is a user --- the empty string means that this must be an unauthenticated request.

@majormoses
Copy link
Contributor

majormoses commented Mar 21, 2021

Error: This resource can only be used in the context of an organization, "" is a user.

--- the empty string means that this must be an unauthenticated request.

Would there be any harm in first checking if the user is "" and if not, provide a slightly different message that helps debugging? I am not really all that familiar with golang but adding the following to the checkOrganization method seems as if it would go a long way in distinguishing between lack of authentication for something that requires it and when the api is used for a resource it can not handle because of the differences between users and orgs:

func checkOrganization(meta interface{}) error {
        if meta.(*Owner).name == "" {
		return fmt.Errorf("This resource can only be used in the context of an organization, %q (an empty string) represents an unauthenticated or anonymous provider. The most likely cause for this is a misconfiguration of the provider instance, please recheck your configuration.", meta.(*Owner).name)
        }
	if !meta.(*Owner).IsOrganization {
		return fmt.Errorf("This resource can only be used in the context of an organization, %q is a user.", meta.(*Owner).name)
	}

	return nil
}

If this makes sense I can open a pull request. I figured this might not be so simple as this might be used in places where it's fine to make some org calls unauthenticated such as listing public repositories. Perhaps it's safe to move inside the other however I suspect there is probably a better place to put these auth type validations separate from if its an org question.

@tibbes
Copy link
Contributor

tibbes commented Mar 22, 2021

@majormoses, absolutely, that error message needs improving in the unauthenticated case! I'm not a maintainer, but I'm sure a pull request would be welcome.

You're right that we might want to check for authentication in more places than just in checkOrganization, so one approach might be:

  • add a new check function next to checkOrganization called checkAuthenticated, containing the extra check you added in the code snippet above
  • update checkOrganization to call checkAuthenticated first, and then if that succeeds continue with the current code in that function

I don't think the new error message needs to mention the empty string or show the value of name, though. That's seems like an implementation detail. Maybe it would be better to say what the user actually has to change (provide token). Perhaps something like:

Managing this resource requires an authenticated connection to GitHub, but no token was provided. See https://registry.terraform.io/providers/integrations/github/latest/docs

@tobypinder
Copy link
Contributor

@tibbes apologies for delay. Given we've applied the env workaround in production states as well it's going to be hard for me to be forensic about what was happening in our states at the time, but it was indeed part of an upgrade involving terraform version upgrades themselves as well as the provider.

I had set up a similar test case based on our environment and been unable to replicate the issue, presumably because this was "new" and thus both modules were using an identical provider.

So this is all a very wishy washy finger in the air way to say I believe your example is correct and that this "heterogenous module conflict" or whatever we're calling it is most likely the underlying culprit that we experienced.

Based on this, I can go back into our production states/CI and attempt to remove the workaround - will report back on success/fail in this regard if useful.

@jcudit jcudit modified the milestones: v4.9.0, v4.10.0 Apr 17, 2021
@thekbb thekbb closed this as completed Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Authentication Provider Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

6 participants