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

When using gcloud preview app run appengineTokenFunc is nil, causing a panic #142

Open
m4tty opened this issue Aug 13, 2015 · 17 comments
Open

Comments

@m4tty
Copy link

m4tty commented Aug 13, 2015

When I use gcloud preview app run instead of goapp ... I am unable to access appengine apis due to AppEngineTokenSource(context, scope) error. AppEngineTokenSource seems to fail because appengineTokenFunc is nil, which causes a panic.

I suspect this is because the latest gcloud tool's gcloud preview app run isn't actually using docker (and so isn't using the go-wrapper which would set the -tags to appenginevm), it just launches via dev_appserver.py. And the gcloud tool (not leveraging docker, go-wrapper -tags) results in no build tag, which would result in appengine_hook not being compiled in and a nil appEngineTokenFunc. As an aside, -when I'm coding on the java side of things- I believe that mvn gcloud:run does result in running dockers).

Error:
CRITICAL: panic: google: AppEngineTokenSource can only be used on App Engine.

I'm thinking that this is a side-effect of an issue the gcloud tool, but I figured I'd post here in case someone had some valuable insight. I appreciate it.

@rakyll
Copy link
Contributor

rakyll commented Aug 13, 2015

cc/ @okdave

@m4tty
Copy link
Author

m4tty commented Aug 13, 2015

Also, I forgot to mention that I verified that appengine.AccessToken is not nil inside of my app... just before calling google.AppEngineTokenSource which panics. Another data point which perhaps points to the missing build flag as causing trouble.

@cnbuff410
Copy link
Contributor

We are having the same issue here. @m4tty did you find a workaround for this?

@okdave
Copy link
Contributor

okdave commented Sep 14, 2015

@broady you were looking at something similar about build tags in the oauth2 packages

@cnbuff410
Copy link
Contributor

Probably something to do with commit ad01282

@broady
Copy link
Contributor

broady commented Sep 14, 2015

Yes, ad01282 is related, but not the cause of this particular bug.

@m4tty - can you explain why you're using AppEngineTokenSource? Could you try DefaultTokenSource/DefaultClient instead?

If you really need control over the token source, then you should likely be using ComputeTokenSource for App Engine Managed VM's.

@m4tty
Copy link
Author

m4tty commented Sep 14, 2015

In order to leverage appengine storage api, we do the following:

    defaultClient := &http.Client{
        Transport: &oauth2.Transport{
            Source: google.AppEngineTokenSource(c, storage.ScopeFullControl),
            Base:   &urlfetch.Transport{Context: c},
        },
    }
    cloudContext = cloud.NewContext(appengine.AppID(c), defaultClient)

Similar to this example:
https://cloud.google.com/appengine/docs/go/googlecloudstorageclient/getstarted?hl=en

In any case, the appenginevm/appengine build flag isn't passed in when using gcloud preview app run, and so appEngineTokenFunc is nil, which causes trouble. (presumably because it does not leverage docker and go-wrapper). This problem does not show up for gcloud preview app deploy as docker / go-wrapper is used.

@cnbuff410 I'm getting around the problem right now by using my own fork of oauth2 that has removed the build flag in appengine_hook.go. This allows things to work fine in my environment (which is appengine for the most part so the dependency doesn't hurt me any). i.e. i'm not leveraging oauth2 in any other contexts.

@m4tty
Copy link
Author

m4tty commented Sep 14, 2015

@broady I'll try using DefaultTokenSource, but a quick glance looks like I might hit a similar issue as it passes by the appEngineTokenFunc check - https://github.com/golang/oauth2/blob/master/google/default.go#L87 and returns an error.
I'll let you know.

@broady
Copy link
Contributor

broady commented Sep 14, 2015

yes, using DefaultClient should work for you. Just make sure you have performed gcloud auth login from your command-line before running the app.

Also, is there a reason you are using gcloud app run instead of just go run?

@m4tty
Copy link
Author

m4tty commented Sep 15, 2015

DefaultTokenSource worked great. And thanks for reminding me to gcloud auth login.

We are using gcloud because we are running multiple modules and a dispatch.yaml. And in an effort to be as close to our deployed environment as possible before releasing. I'd love to run gcloud preview app run ... --remote or something to run in a dockerized local environment, but I'm not aware of a way to do so. No matter, this works.

Anyway, thanks @broady I really appreciate the help. This is a solid work around to the issue.

@rakyll
Copy link
Contributor

rakyll commented Sep 15, 2015

Was the original bug related to AppEngineTokenSource? DefaultTokenSource is handy, but we must still support AppEngineTokenSource both on dev and prod on AppEngine and AppEngine managed VMs.

@broady
Copy link
Contributor

broady commented Sep 15, 2015

Original bug is that gcloud app run does not compile Go application with the appenginevm build tag.

Why do we need AppEngineTokenSource to work on Managed VMs? The google.golang.org/appengine package does not use the oauth2 package.

@rakyll
Copy link
Contributor

rakyll commented Sep 15, 2015

Why do we need AppEngineTokenSource to work on Managed VMs?

To minimize the API fragmentation between the classic App Engine and the Managed VMs.

The google.golang.org/appengine package does not use the oauth2 package.

We originally considered providing a TokenSource from the appengine package but the oauth2 package was not mature enough for being such a core dependency.

The user should never care about the internals and switching back and forth between different TokenSource implementations if they are on AppEngine. That's what AppEngineTokenSource promises. If we can't maintain the complexity of the appengine TokenSources in the scope of oauth2, let's move them elsewhere. If we provide a half broken library and continuously suggest workarounds, it doesn't help the users.

@broady
Copy link
Contributor

broady commented Sep 15, 2015

I think the correct thing to do is just use DefaultTokenSource/DefaultClient. I do not consider it to be a workaround.

It's possible AppEngineTokenSource should be deprecated.

@rakyll
Copy link
Contributor

rakyll commented Sep 16, 2015

When we deprecated the old OAuth package (goauth2), we made a compatibility promise not to break any users, e.g. we made several sacrifices along the way to keep the APIs at the cost of not breaking the users.

The same policy still applies. We don't break our users randomly or favoring the DefaultClient suddenly by breaking the existing implementations. If the default credentials flow is what is recommended by the App Engine team, we can document it accordingly and tweak the current TokenSource implementation to delegate the work to the DefaultTokenSource. Panicing is agreessive.

@broady
Copy link
Contributor

broady commented Sep 16, 2015

I agree. This (submitted) CL fixes that:
https://go-review.googlesource.com/#/c/14622/

I'll file an internal bug about gcloud app run.

@rakyll
Copy link
Contributor

rakyll commented Sep 16, 2015

@broady, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants