-
Notifications
You must be signed in to change notification settings - Fork 61
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
New Env Var: GOOGLEWORKSPACE_OAUTH_SCOPES
#109
Conversation
…to look for comma separated list in var and return nil otherwise
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few quick comments, but great idea. Thank you for your contribution!
internal/provider/provider.go
Outdated
@@ -81,9 +81,23 @@ func New(version string) func() *schema.Provider { | |||
"oauth_scopes": { | |||
Description: "The list of the scopes required for your application (for a list of possible scopes, see " + | |||
"[Authorize requests](https://developers.google.com/admin-sdk/directory/v1/guides/authorizing))", | |||
Type: schema.TypeList, | |||
Type: schema.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason this would be TypeSet
? I think I'd prefer to keep it a list if possible, lists are much easier to work with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@megan07 I mainly based it on the docs description of Set being where you don't care about the order and List where you do. I think the former is more correct but TBH I don't know if it really matters for this type of schema anyways. I've reverted in my more recent commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tgoodsell-tempus! Thanks for the changes! I played around with this earlier today (but haven't had a lot of time). For some reason, I couldn't get it to set the oauth_scopes, it would always come up with the default. It seems to me a bug with DefaultFunc
maybe? I can't say for sure yet, I'll need more time, but I wanted to double-check to see if you ran this against tests that weren't paired with your defined scopes. Did it give you an error?
Thanks!
I've had a bit more time to play with this. It seems like this is an open issue in the SDK. I'll follow up with them and see if we can figure out a workaround. |
Closing to re-do in the |
This PR allows the provider to use a env var
GOOGLEWORKSPACE_OAUTH_SCOPES
for setting the OAuth Scopes used with the credentials call.Is necessary when your workspace user is not delegated all scopes/access, but you'd rather not set them in each individual provider block.