search

stephenplusplus / google-auto-auth

Making it as easy as possible to authenticate a Google API request
MIT License
34 stars 9 forks source link

use local env test for kubernetes #25

Closed ofrobots closed 6 years ago

ofrobots commented 7 years ago

Looking for a metadata attribute can be slow specially if the key doesn't exist. Use local environment to detect kubernetes.

See: https://github.com/GoogleCloudPlatform/google-cloud-node/issues/2544

Motivated by the Java client libraries: https://github.com/GoogleCloudPlatform/google-cloud-java/blob/master/google-cloud-logging/src/main/java/com/google/cloud/logging/MonitoredResourceUtil.java#L174

stephenplusplus commented 7 years ago

I'm all for this if we can count on this env var to work as expected (any reason to believe it will go out of date some day?)

If we wanted to keep the same check in place, using the metadata, we could upgrade retry-request in gcp-metadata so that those 404s aren't retried-- that's likely why the large delays are occurring.

stephenplusplus commented 7 years ago

PR for updating retry-request: https://github.com/stephenplusplus/gcp-metadata/pull/4

ofrobots commented 7 years ago

:+1: on upgrading retry-request. You're right that it explains more concretely where the large delay is coming from.

There is still a flaw in the logic here. !err was not sufficient to check if we are on kubernetes. In the 404 case err is undefined (a bug I introduced 😢 ). But this can be independently be fixed.

Let's hold off on this PR for now. It is not on the critical path anymore, but there might be still some merit in it.

ofrobots commented 6 years ago

I think this can be closed now. It is not clear if this worth making now that the underlying problem with retry-request is fixed.