DeviaVir / terraform-provider-gsuite

A @HashiCorp Terraform provider for managing G Suite resources.
MIT License
271 stars 77 forks source link

[bugfix] Don't use httpclient unless it is set #175

Closed natalysheinin closed 3 years ago

natalysheinin commented 3 years ago

Bugfix for changes introduces in https://github.com/DeviaVir/terraform-provider-gsuite/pull/171

Do tests pass?

go test -i ./... || exit 1
go test: -i flag is deprecated
echo ./... | \
        xargs -t -n4 go test  -timeout=30s -parallel=4
go test -timeout=30s -parallel=4 ./...
?       github.com/DeviaVir/terraform-provider-gsuite   [no test files]
ok      github.com/DeviaVir/terraform-provider-gsuite/gsuite    0.360s

Does binary work as expected?

If metadata endpoint is blocked or email is not configured properly, the following error will show (this is not an error with the code, but a config error):

Error: [ERROR] Error creating group: Post "https://admin.googleapis.com/admin/directory/v1/groups?alt=json&prettyPrint=false": impersonate: status code 400: {
  "error": {
    "code": 400,
    "message": "Request contains an invalid argument.",
    "status": "INVALID_ARGUMENT"
  }
}
natalysheinin commented 3 years ago

@DeviaVir do you prefer me building a client in the else if c.ImpersonatedUserEmail != "" { instead of this approach?

DeviaVir commented 3 years ago

@DeviaVir do you prefer me building a client in the else if c.ImpersonatedUserEmail != "" { instead of this approach?

I think we´d want to use the same background context, so it might make sense to create the client there and append it (or refactor it a bit)

natalysheinin commented 3 years ago

@DeviaVir do you prefer me building a client in the else if c.ImpersonatedUserEmail != "" { instead of this approach?

I think we´d want to use the same background context, so it might make sense to create the client there and append it (or refactor it a bit)

Actually it seems like the background context is completely unrelated to the client so it doesn't make sense to add it at all? For example, the context is passed to the services initialized regardless of whether it's used to setup the httpclient or not:

directorySvc, err := directory.NewService(context, clientOptions...)

DeviaVir commented 3 years ago

Makes sense

DeviaVir commented 3 years ago

https://github.com/DeviaVir/terraform-provider-gsuite/releases/tag/v0.1.61 let me know if this works better for you