Skip to content
This repository has been archived by the owner on Feb 9, 2022. It is now read-only.

Implement a nil adapter #567

Closed
geeknoid opened this issue Apr 18, 2017 · 11 comments
Closed

Implement a nil adapter #567

geeknoid opened this issue Apr 18, 2017 · 11 comments
Assignees
Milestone

Comments

@geeknoid
Copy link
Contributor

We need to implement a do-nothing nil adapter that can be used as part of any aspect. This adapter will be used for perf testing.

@geeknoid geeknoid added this to the mixer alpha milestone Apr 18, 2017
@douglas-reid
Copy link
Contributor

@geeknoid do we want this adapter to live in //adapter or are we establishing some other package for test framework artifacts?

@geeknoid
Copy link
Contributor Author

I think it's fine to leave it with the other adapters. It would probably cause us endless pain if that were kept elsewhere... It'll be small and innocuous, so I think it's fine to have it in the real code.

At some point, we'll need to figure out a simple approach to dynamically compile stuff in/out of the mixer, or support dynamically loaded stuff. In general, the best way to avoid security flaws in code i to delete the code and not have it there in the first place. As such, it's important to strip unused code in released bits so there are nothing dead sitting there waiting to be exploited. But we should do that in a general way, not specifically around the nil adapter.

@ayj
Copy link
Contributor

ayj commented Apr 18, 2017

simple approach to dynamically compile stuff in/out of the mixer

Some combination of https://golang.org/pkg/go/build/#hdr-Build_Constraints and having the adapters themselves register with the inventory might work, e.g.

// adapter/inventory.go
var inventory []adapter.RegisterFn
func Add(fn adapter.RegisterFn) {
    inventory = append(inventory, fn)
}
func Inventory() []adapter.RegisterFn {
    return inventory
}

// adapter/xxx/xxx.go guarded by file-level build constraint
func init() {
    inventory.Add(Register)
}

@ZackButcher
Copy link
Contributor

We need to investigate how build constraints interact with bazel (I suspect they don't interact well, but maybe that's just me being overly pessimistic), but it'd be nice if we could use that.

@ZackButcher
Copy link
Contributor

FWIW you can define custom tags and provide them on the CLI to the go tool, golang/go#8772, that was not obvious in the docs on the build package that Jason linked.

We could do something as simple as mandate that adapter authors add a //+build <my adapter name>, use init to register the adapter as Jason proposed, then to build a mixer binary you go build -tags "prometheus,myspecialadapter,kubernetes,..." where the tags list the adapter names you want in the deployment. We could even write a tool that looks at the global config, extracts the adapters you named, and executes a go build with those tags.

@geeknoid
Copy link
Contributor Author

geeknoid commented Apr 18, 2017 via email

@ZackButcher
Copy link
Contributor

So rather than read the global config, we could have a tool accept a set of adapter names then generate the binary and correct protos for configuring that binary.

@geeknoid
Copy link
Contributor Author

geeknoid commented Apr 18, 2017 via email

@ZackButcher
Copy link
Contributor

To capture a conversation @geeknoid and I had: another possible solution would be adding a istioctl generate-mixer --adapter=a,b,c,... command. IMO this is more idiomatic in the go ecosystem than e.g. using a go-C hybrid file with C preprocessor directives that we "compile" to produce a final go file. The concern is that this feels a little out of place in the istioctl tool, which is runtime focused, though it might be similar enough to the istioctl kube-inject command that we could argue it fits in.

@ZackButcher
Copy link
Contributor

Lets move talk about dynamic adapter compilation over to #579 and move this one back to the no-op adapters. I replicated our conversation in that thread.

@douglas-reid
Copy link
Contributor

Can we close this now?

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

No branches or pull requests

4 participants