Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document how plugin can be used with vendored and GOPATH OPA dependency. #94

Conversation

ashutosh-narkar
Copy link
Member

Signed-off-by: Ashutosh Narkar anarkar4387@gmail.com

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
@ashutosh-narkar ashutosh-narkar requested a review from tsandall June 21, 2019 22:54
Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on which options to expose. Other than that, LGTM.

If the go modules stuff doesn't pan out I'm wondering if we could do the .so file build inside of a container that has the dependencies setup in GOPATH. /cc @patrick-east for thoughts.

@@ -101,6 +101,20 @@ IP/port of the Istio Ingress gateway.
curl --user bob:password -i http://$GATEWAY_URL/api/v1/products
```

## How to use the OPA-Istio plugin

### Option 1: Using `vendored` OPA dependency
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a scenario where this option makes sense? I'm wondering if we should only recommend (1) running the opa_istio binary/container produced by the makefile or (2) building the .so file with the GOPATH option and then running with either (i) upstream OPA (of the correct version) or (ii) a custom OPA executable.

@patrick-east
Copy link
Contributor

Maybe a dumb question, but why do we need a separate opa_istio binary? Why not always build the .so and expect either the official opa or someone elses custom one?

If the go modules stuff doesn't pan out I'm wondering if we could do the .so file build inside of a container that has the dependencies setup in GOPATH

Yea, that would work. It might even be a good opportunity to show a "best practice" or something for building opa plugins. Some scripts/tooling to have a container with golang tools+opa source at some release version and churn out plugin .so's seems useful.

@tsandall
Copy link
Member

@patrick-east that's a good point. As part of the build process in this repo though we can still produce the openpolicyagent/opa:<version>-istio flavor that contains the .so file (i.e., the Docker image should be based on openpolicyagent/opa:<version>

@patrick-east
Copy link
Contributor

patrick-east commented Jun 25, 2019

As part of the build process in this repo though we can still produce the openpolicyagent/opa:-istio flavor that contains the .so file (i.e., the Docker image should be based on openpolicyagent/opa:

Yea, I would definitely want to see that happen. We can turn this into only a library and remove the vendor directory (keeping the dependency yaml/toml/etc). Our build scripts for the .so file can do the container based build which does the right thing import path-wise and anyone wanting a custom binary can much more easily use the library.

@tsandall
Copy link
Member

+1 this seems like the way to go. @ashutosh-narkar WDYT?

@ashutosh-narkar
Copy link
Member Author

ashutosh-narkar commented Jun 26, 2019

The suggestion sounds fine but the only concern I have is that we are now telling users that if they want to use the plugin they need to have OPA cloned in their GOPATH. Today, users cannot simply use the released OPA binary to run this plugin file. So if they want to use a released OPA version or a custom OPA version, they would need to build from source. If we are sure (or want to enforce a best practice) that a OPA plugin is only a library then this suggestion makes sense.

On the other hand, if someone wants to use the plugin as a standalone and does not prefer cloning OPA or is not doing any OPA development or simply wants to try it out, the current approach of generating the opa_istio is beneficial. Again I don't have enough info if users will use the plugin in this manner. If it helps we can think of making the tooling better to facilitate both usage patterns.

Update on the go modules transition: Was able to migrate from glide and build the plugin. In the first scenario, built the plugin by specifying github.com/open-policy-agent/opa v0.12.0 as a requirement. Then ran the plugin with a OPA v0.12.0 binary (built from source), but got the error:

plugin.Open("opa_istio_plugin"): plugin was built with a different version of package github.com/open-policy-agent/opa/util

In the second scenario, used the replace directive to use local OPA v0.12.0 source. But got the same above error when running the plugin with OPA v0.12.0 binary (built from local source).

The only way to run the plugin was by creating a opa_istio binary from the plugin directory either by depending on the opa in module cache (scenario 1) or local directory(scenario 2).

So it looks like the same OPA source code in the module cache is treated differently from the one in the local filesystem path (Investigating why this is the case). At this point I do not see any added benefit of transitioning to go modules imo.

Found this related issue: golang/go#31354

@tsandall
Copy link
Member

Update on the go modules transition: Was able to migrate from glide and build the plugin. In the first scenario, built the plugin by specifying github.com/open-policy-agent/opa v0.12.0 as a requirement. Then ran the plugin with a OPA v0.12.0 binary (built from source), but got the error:

:/ that's too bad. Let's ignore Go modules for now then.

I think the path forward is clear then:

  1. Remove the vendor directory from this repo
  2. Refactor the build process to build the .so file inside of a container that has the OPA source checked out in the GOPATH at a specific tag
  3. Refactor the Dockerfile to inherit from openpolicyagent/opa instead of ubuntu:16.04 adding the .so file to provide users wanting a standalone opa-istio image
  4. Document the two options for using this repository
    i. Build from source with GOPATH
    ii. Use standalone opa-istio image (i.e., standard OPA image with added .so file)

@ashutosh-narkar
Copy link
Member Author

Removed support for Go Plugin: #139

Customize OPA: https://www.openpolicyagent.org/docs/latest/extensions/#custom-plugins-for-opa-daemon

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

Successfully merging this pull request may close these issues.

3 participants