Add controller.Setup

[stephen-soltesz]

This change adds controller.Setup to provide consistent setup of the http access controller handlers for ndt-server, the envelope service, and other services integrating this functionality.

In order to simplify the logic in controller.Setup the interfaces of several existing controller types are changed in this PR. Since the m-lab/access package has no active users yet, no changes are breaking.

• NewTxController accepts a context in order to start the Watch goroutine immediately.
• NewTokenController uses the machine name provided by a flag to validate access tokens.
• Unit tests are updated accordingly

---

This change is 

[coveralls]

Pull Request Test Coverage Report for Build 30

• 37 of 37 (100.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.1%) to 99.219%

---

Totals
Change from base Build 18:	0.1%
Covered Lines:	254
Relevant Lines:	256

---

💛 - Coveralls

[yachang]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @stephen-soltesz and @yachang)

---

controller/control.go, line 10 at r1 (raw file):

	"net/http"

	"github.com/justinas/alice"

Add some doc about this third party dependency?

[yachang]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @stephen-soltesz and @yachang)

---

controller/control.go, line 52 at r1 (raw file):

// Setup creates sequence of access control http.Handlers. If the
// verifier is nil then it will be excluded. If the tx controller is

It is hard to read IMHO.

the first "it will be excluded", what is "it"?

The second "it will be excluded", tx controller is returned value, right? Why do you want to return an invalid value, instead of just sending an err?

[yachang]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @stephen-soltesz)

---

controller/control.go, line 56 at r1 (raw file):

func Setup(ctx context.Context, v Verifier) (alice.Chain, *TxController) {
	// Setup sequence of access control http.Handlers.
	// Controllers must be applied in specific order:

Same confusion here.

As I read the code, there are token controller and tx controller. alice chain try to append both of them.

If the doc here means both kinds of controller, could it explain how the txcontroller access token?

[stephen-soltesz]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @yachang)

---

controller/control.go, line 10 at r1 (raw file):

Previously, yachang wrote…

Add some doc about this third party dependency?

I've added a comment, but this is not typical. The package is very light weight (and very helpful!).

---

controller/control.go, line 52 at r1 (raw file):

Previously, yachang wrote…

It is hard to read IMHO.

the first "it will be excluded", what is "it"?

The second "it will be excluded", tx controller is returned value, right? Why do you want to return an invalid value, instead of just sending an err?

I've added additional text. How does it look?

---

controller/control.go, line 56 at r1 (raw file):

Previously, yachang wrote…

Same confusion here.

As I read the code, there are token controller and tx controller. alice chain try to append both of them.

If the doc here means both kinds of controller, could it explain how the txcontroller access token?

I've rephrased the comments here. PTAL?

[yachang]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @stephen-soltesz and @yachang)

---

controller/control.go, line 75 at r2 (raw file):

	// If the tx controller is successful, include the tx limit.
	tx, err := NewTxController(ctx)

Is the searching for "claims in the context" done in

---

controller/tx.go, line 50 at r2 (raw file):

// NewTxController creates a new instance initialized to run every second.
// Caller should run Watch in a goroutine to regularly update the current rate.

update the doc here.

[yachang]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @stephen-soltesz)

[yachang]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @stephen-soltesz)

[stephen-soltesz]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @yachang)

---

controller/control.go, line 75 at r2 (raw file):

Previously, yachang wrote…

Is the searching for "claims in the context" done in

Looks like your comment was incomplete. It looks like you're asking where does the tx controller lookup the access token claims?

On each request, the token controller will look for and if it finds a valid access token add the claim to the HTTP request context here:

access/controller/token.go

Line 95 in 88ce58b

return true, SetClaim(ctx, cl)

When the tx controller checks whether to limit the connection, it retrieves the claim from the request context here:

access/controller/tx.go

Line 119 in 88ce58b

monitoring := IsMonitoring(GetClaim(r.Context()))

I'll add a diagram of this process to the design doc and the access control slides.

---

controller/tx.go, line 50 at r2 (raw file):

Previously, yachang wrote…

update the doc here.

Good call. Done.

[stephen-soltesz]

(I forgot to publish these comments before merging - they were drafts in my browser)