googleapis / nodejs-grafeas

This repository is deprecated. All of its content and history has been moved to googleapis/google-cloud-node.
https://grafeas.io/
Apache License 2.0
8 stars 9 forks source link

move the default endpoint and any Google auth defaulting #2

Closed bcoe closed 5 years ago

bcoe commented 5 years ago

We need the @google-cloud/grafeas to not default to the default Google endpoint, we should allow authentication and endpoint to be passed in after the client is created:

see:

https://github.com/jskeet/google-cloud-dotnet/blob/grafeas-2/apis/Grafeas.V1/Grafeas.V1.SmokeTests/GrafeasSmokeTest.cs

https://github.com/jskeet/google-cloud-dotnet/commit/34befe1b517371418498c0de9cfdfb752d22c794

I'm not quite sure what this implementation would look like in Node.js; but I think it might be able to dovetail with some of the work you're doing with options @JustinBeckwith? (maybe this repo would be a good candidate for testing).

JustinBeckwith commented 5 years ago

So endpoint override - good news, that's possible today! The user would just have to set the servicePath property in the options for the client constructor:

const client = new GrafeasClient({
  servicePath: 'some.other.endpoint',
});

Now authentication ... that's a bit of a different story. We could add code to allow disabling authentication, but I can't think of what a generic auth story would look like for this. @jskeet would we just accept a user implementation of grpc call credentials as part of the constructor options?

jskeet commented 5 years ago

For C# at least, we accept the gRPC channel, which already knows the endpoint and auth.

But to accept an endpoint, you'd want to use some implementation of gRPC cress, yes. I don't have much to say about that other than to ensure that the Grafeas library doesn't try to load default credentials.

bcoe commented 5 years ago

@JustinBeckwith in my testing last week, I was able to pass in a stubbed auth parameter to the library initialization:

const client = new GrafeasClient({
  auth: {...}
})

I was, however, unable to in practice connect to a testing server:

https://github.com/grafeas/grafeas/tree/master/samples/server/go-server/api/server

The client seems to never negotiate a connection, eventually timing out.

Any thoughts @alexander-fenster? I know you've experimented a bit with connecting a testing GRPC service.

JustinBeckwith commented 5 years ago

I went spelunking through gax, and I think @jskeet's approach here with C# is starting to make a lot of sense. We could accept an optional GrpcCallCredential object (or something like that...) which gets passed directly down into gRPC instead of the combined credentials we create today. That way the user could create whatever kind of auth thingy they wanted to. Of course, this is something we would have to figure out over in go/extensible-options-in-nodejs.

@bcoe wanna find 30 with me and @alexander-fenster today?

alexander-fenster commented 5 years ago

We can pass sslCreds object right now.

const client = new GrafeasClient({
  sslCreds: grpc.credentials.createInsecure(), // or any other credentials object
  servicePath: 'localhost', // overriding the service path
  port: 1234 // and override port if needed 
});
JustinBeckwith commented 5 years ago

I will admit to having no idea what that does 😆

stephenplusplus commented 5 years ago

That is the same thing we would do in other libraries when a user was using an emulator. It just disables the authentication / requirement for credentials.

JustinBeckwith commented 5 years ago

Ok - after further discussion with @alexander-fenster, it turns out the sslCreds object can accept any sort of grpc credential, including the ssl cert credentials that the grafeas APIs use right now. So in reality... we're actually good to go here! No changes needed.

@bcoe since overriding the endpoint and providing your own creds are both supported.... can we just close this?

bcoe commented 5 years ago

@JustinBeckwith 👍 I think we can just close this; unfortunately I've been unable to actually test things because the testing server uses the v1beta