WIP: Client API implementation #1119

endophage · 2017-03-20T20:49:54Z

Huge feature list here and apologies for the PR bomb.

New Interface type defined for client interaction with a repository (client/interface.go)
GRPC implementation of the new interface (client_api/api/*)
CLI integration with GRPC API if configured (cmd/notary/repo_factory.go and other necessary knock on changes in the cmd/notary package)
JWT token auth for the GRPC API consistent with Notary Server and Docker Registry (auth/*). Some of this is to be upstreamed into docker/distribution when I get the chance.
- If enabled "push" and "pull" actions will always be requested to guarantee the Client API can communicate with the upstream Notary Server to read repos and publish updates. The server configuration will also accept a map of GRPC Endpoint Name : [permissions] (documentation to be written) that allows users to arbitrarily configure in more permissions per GRPC API. Users would need their token server to issue those permissions as appropriate to their users.

Still needed:

more tests
documentation

Shout out to @n4ss for all the hard work on this. Go get some sleep!

cc @ecordell @lewiada

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

Signed-off-by: David Lawrence <dclwrnc@gmail.com> (github: endophage)

…ld be working Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

…d code Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Signed-off-by: David Lawrence <david.lawrence@docker.com> Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> (github: endophage)

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

…e. Wrapping up configuration of auth Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

…r everyone Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

…exercise some of the code while I had a debugger running Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

ecordell

This mostly is looking really good! I left some comments from my first pass, I plan to do a more in-depth review soon.

ecordell · 2017-03-27T18:33:39Z

client_api/api/api.proto

+// Notary Interface
+service Notary {
+    // AddTarget adds a target to the TUF repository and re-signs.
+    rpc AddTarget(TargetAction) returns (BasicResponse) {}


Should these take an optional list of roles to sign with? I know the default is to sign with all available, but that might not be the desired behavior when we get to thresholding

ecordell · 2017-03-27T18:42:10Z

client_api/api/server.go

 		gun,
 		srv.upstream,
-		rt,
+		remoteStore, // remote store


don't know that this comment is adding much :)

ecordell · 2017-03-27T18:45:09Z

cmd/notary/repo_factory.go

+
+type repoFactory func(gun data.GUN) (client.Repository, error)
+
+func ConfigureRepo(v *viper.Viper, retriever notary.PassRetriever, onlineOperation bool) repoFactory {


👍 this really cleans things up

ecordell · 2017-03-27T18:58:10Z

utils/token/accesscontroller.go

@@ -0,0 +1,215 @@
+package token


👍 for a dependency removed

I'm actually going to try and upstream these auth pieces back into distribution. I don't particularly want a forked version I have to keep up to date if somebody changes something. The version in docker/distribution is basically the canonical version for Docker but it was too tightly coupled with net/http and I needed to make progress.

ecordell · 2017-03-27T19:06:29Z

cmd/notary/repo_factory.go

+
+const remoteConfigField = "api"
+
+type repoFactory func(gun data.GUN) (client.Repository, error)


I think the naming here could be confusing, because RepoBuilder already exists. Maybe more explicitly clientRepoFactory? (same for filename as well)

I have some stuff to discuss at our maintainers meeting at DockerCon that may obsolete RepoBuilder :-)

ecordell · 2017-03-27T19:28:33Z

auth/client/session.go

+		offlineAccess: false,
+		forceOAuth:    false,
+		clientID:      clientID,
+		//scopes: []Scope{


these comments should be cleaned up

ecordell · 2017-03-27T19:29:14Z

auth/client/session.go

+	return fmt.Sprintf("%s:%s:%s", repoType, rs.Repository, strings.Join(rs.Actions, ","))
+}
+
+//// RegistryScope represents a token scope for access


if this is unused it should be removed

ecordell · 2017-03-27T19:30:04Z

auth/client/session.go

+}
+
+func (th *TokenHandler) AuthorizeRequest(params map[string]string, scopes ...string) (string, error) {
+	//th.tokenLock.Lock()


remove these? can't think of a reason there would need to be a lock here.

There was a token cache (map[something]something) I removed that this was locking.

ecordell · 2017-03-27T19:33:23Z

client_api/api/server.go

+	); err != nil {
+		return nil, err
+	}
+	if err := publishRepo(r); err != nil {


Should the responsibility to publish be pushed back to the "clientapi client" instead of in the "clientapi server"?

IIRC this mirrors the current cli behavior, just wondering if there's a good reason to keep them coupled together.

In the case where somebody horizontally scales Client API's (I'm just using that as the name for this new service), there's no guarantee you'll hit the same instance again. I think it's a lot cleaner and easier to implement if every operation is a one shot that publishes (where appropriate, we intend to support things like letting clients add keys, which would still require some kind of synchronization between instances).

ecordell · 2017-03-27T19:40:52Z

client_api/api/server.go

+		return nil, err
+	}
+
+	resTargets := make([]*TargetWithRole, len(targets))


Can these be factored out? Maybe something like NewTargetWithRole(Target) and NewTargetWithRoles([]Target)

They only really need to be serialized here so I understand not separating it out, but if they were separate it would be very clear that all this server does is proxy commands to a repo object and then serialize the results.

(Same comment for all of the other serialization happening in this file)

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

endophage · 2017-04-25T18:08:50Z

Superseded by #1139

GordonTheTurtle added the dco/no label Mar 20, 2017

David Lawrence added 4 commits March 20, 2017 13:59

add initial client api code

fa13191

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

adding client interface back in

90660e7

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

basic factory for local/remote client

3fac89b

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

clientapi works with cli

b024930

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

endophage force-pushed the client_api branch from 16d03f5 to d9f14e3 Compare March 20, 2017 21:00

GordonTheTurtle added the dco/no label Mar 20, 2017

endophage and others added 23 commits March 20, 2017 14:07

grpc interceptors are working, need to get proper token auth in place

f2f3118

Signed-off-by: David Lawrence <dclwrnc@gmail.com> (github: endophage)

regenerated certs for all the services and server side of tokens shou…

7760a77

…ld be working Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

grpc client needed GetGUN method to meet Repository interface

44656cc

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

fixing some interfaces

bc29fa6

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

server side of producing a token is working

2386ae0

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

working token auth end to end

cc73b50

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

Add Target operations to client API protobuf definitions and generate…

02415db

…d code Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add ChangeList operations to client API protobuf definitions

a0095e8

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add generated code for ChangeList grpc interface

904cf35

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add Init, Publish and returned Errors to client API gRPC interface

7bba7ec

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add Role operations to client API gRPC interface with generated code

9066aa3

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add remaining rpc definitions to client gRPC API interface

55ad281

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement client api ListTargets

0da0e82

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more client api calls

8cd2776

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more client api calls - part 2

4d8d088

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more client api calls - part 3

6c0fd1e

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more client api calls - part 4

3c4ca95

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more client api calls - part 5

c5208b6

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Finish implementing main part of client API endpoint

822b5e4

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Switch Empty type to google protobuf's Empty type

1af5845

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add cryptoservice interface to client api endpoint

dd7c918

Signed-off-by: David Lawrence <david.lawrence@docker.com> Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> (github: endophage)

Add missing response type and update generated files

1ad6d9d

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add client api prototypes to server interface

adca046

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

n4ss and others added 9 commits March 20, 2017 14:08

Fix cherry-pick conflicts

c63fb30

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Add GUN parameters to client side of client api

270b9f3

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement cient api server-side Init and Publish

158c945

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more cient api server-side - part 2

8e3faf6

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Implement more cient api server-side - part 3

ac1b131

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Finish cient api server-side implementation

bb89cff

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

Fix cherry-pick conflicts

c82dade

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>

moving around some of the auth stuff to collect it better in one plac…

e21831c

…e. Wrapping up configuration of auth Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

use relative path for API CA in cmd/notary/config.json so it works fo…

e6e7e88

…r everyone Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

endophage force-pushed the client_api branch from d9f14e3 to e6e7e88 Compare March 20, 2017 21:08

GordonTheTurtle removed the dco/no label Mar 20, 2017

endophage changed the title ~~Client API implementation~~ WIP: Client API implementation Mar 20, 2017

removing test that was intentionally failing. I was just using it to …

cdfd8df

…exercise some of the code while I had a debugger running Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

ecordell suggested changes Mar 27, 2017

View reviewed changes

David Lawrence added 2 commits March 30, 2017 10:39

forward tokens from ClientAPI to notary server

bcc3abe

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

configure key storage for clientapi

297656c

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

endophage mentioned this pull request Apr 25, 2017

WIP: All the Client API work and dependencies rolled in to one monster PR #1139

Closed

endophage closed this Apr 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Client API implementation #1119

WIP: Client API implementation #1119

endophage commented Mar 20, 2017

ecordell left a comment

ecordell Mar 27, 2017

ecordell Mar 27, 2017

ecordell Mar 27, 2017

ecordell Mar 27, 2017

endophage Mar 28, 2017

ecordell Mar 27, 2017

endophage Mar 28, 2017

ecordell Mar 27, 2017

ecordell Mar 27, 2017

ecordell Mar 27, 2017

endophage Mar 28, 2017

ecordell Mar 27, 2017

endophage Mar 28, 2017

ecordell Mar 27, 2017

endophage commented Apr 25, 2017


		type repoFactory func(gun data.GUN) (client.Repository, error)

		func ConfigureRepo(v *viper.Viper, retriever notary.PassRetriever, onlineOperation bool) repoFactory {


		const remoteConfigField = "api"

		type repoFactory func(gun data.GUN) (client.Repository, error)

WIP: Client API implementation #1119

WIP: Client API implementation #1119

Conversation

endophage commented Mar 20, 2017

ecordell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endophage commented Apr 25, 2017