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

Dynamically build the mixer binary and config proto based on adapters the operator selects #579

Closed
ZackButcher opened this issue Apr 19, 2017 · 3 comments
Assignees
Milestone

Comments

@ZackButcher
Copy link
Contributor

ZackButcher commented Apr 19, 2017

We need to devise a way to dynamically choose which adapters the mixer is compiled with based on something higher level than the operator editing a go file. Additionally, we'd like to generate the adapter config proto based on only the adapters in the deployment.

Copied over from #567:


@geeknoid
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

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
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
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
​I think we want to do the same thing with cfg.proto somehow. We should
list all the adapter configs in that file and select which ones are
compiled in and available for use in config based on the actual set of
adapters used in the deployment.​


@ZackButcher
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
Yeah, I call that too "Bazel" :-)

I was thinking we'd just have Bazel inject an env var listing the adapters
to use in the deployment. If the env var is empty, all available adapters
are used. Bazel would first trigger the generation of cfg.proto and would
control which adapter is pulled into the binary.

I think this could all be pretty slick...


@ZackButcher
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 ZackButcher added this to the mixer beta milestone Apr 19, 2017
@ZackButcher
Copy link
Contributor Author

There's also some overlap with #565

@ZackButcher ZackButcher changed the title Dynamically build the mixer and config proto based on adapters the operator selects Dynamically build the mixer binary and config proto based on adapters the operator selects Apr 19, 2017
@geeknoid geeknoid assigned douglas-reid and unassigned ayj, ZackButcher and geeknoid Jun 30, 2017
@douglas-reid
Copy link
Contributor

@ZackButcher right now, we have a way for specifying which adapters to include in a mixer build via the bazel file in //adapters/. I'm not sure that meets your goal of a higher-level tool or not.

What do you think is left to close this issue out for 0.2 ?

@ZackButcher
Copy link
Contributor Author

This is done for 0.2, we can open new issues in 0.3 to cover a nicer experience for it.

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

No branches or pull requests

4 participants