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

Proposal: Add visibility to OPA server initialization in Plugin Manager #3701

Closed
gshively11 opened this issue Aug 3, 2021 · 5 comments · Fixed by #3706
Closed

Proposal: Add visibility to OPA server initialization in Plugin Manager #3701

gshively11 opened this issue Aug 3, 2021 · 5 comments · Fixed by #3706

Comments

@gshively11
Copy link
Contributor

gshively11 commented Aug 3, 2021

I'm working on a plugin to make OPA play nice when running in AWS Lambda. I'm using the existing plugin architecture in OPA (i.e. like status, decision logs, etc.). The Lambda plugin is responsible for contacting the Lambda service to request an event when OPA is fully initialized (among other things). However, right now, there is no way to verify if/when the OPA server is fully initialized within a plugin. My proposal is to add a channel to the plugin Manager struct that plugins can optionally receive on, and modify the OPA runtime to send on that channel once the server is fully initialized. This should be a small, non-breaking change. Thoughts? I'll contribute once the idea receives a 👍 .

@srenatus
Copy link
Contributor

srenatus commented Aug 4, 2021

My proposal is to add a channel to the plugin Manager struct that plugins can optionally receive on, and modify the OPA runtime to send on that channel once the server is fully initialized.

I think the same problem is solved in the sdk package, like this:

	manager.RegisterPluginStatusListener("sdk", func(status map[string]*plugins.Status) {

		select {
		case <-ready:
			return
		default:
		}
		// NOTE(tsandall): we do not include a special case for the discovery
		// plugin. If the discovery plugin is the only plugin and it goes ready,
		// then OPA will be considered ready. The discovery plugin only goes ready
		// _after_ it has successfully processed a discovery bundle. During
		// discovery bundle processing, other plugins will register so their states
		// will be accounted for. If a discovery bundle did not enable any other
		// plugins (bundles, etc.) the OPA will still be operational.
		for _, s := range status {
			if s.State != plugins.StateOK {
				return
			}
		}

		close(ready)
	})

	// ... other stuff

	if block {
		select {
		case <-ctx.Done():
			return ctx.Err()
		case <-ready:
		}
	}

Am I wrong? Would the same approach solve your issue? 🤔

@srenatus
Copy link
Contributor

srenatus commented Aug 4, 2021

Oh wait, you'd like to do that from a plugin... is that what complicates things?

@gshively11
Copy link
Contributor Author

Yea, from within a plugin, I need to know when the OPA server's listeners are fully initialized and able to receive traffic. Right now, I'm stuck with a race condition where sometimes when I tell Lambda I'm ready for an event, the listener still isn't up.

Your example looks like a listener for Plugin state, which isn't sufficient, because all plugins must be in an OK state before the listener even starts its initialization.

I'll have a PR up pretty soon for clarity.

@srenatus
Copy link
Contributor

srenatus commented Aug 4, 2021

Thanks for clarifying. One thing that seems a little at odds here (to me) is that the server's handlers should be ready when its plugins are not. I suppose this isn't a problem with careful wording, though.

@gshively11
Copy link
Contributor Author

Yea it's a little odd, but makes sense in the case of our plugin. The plugin is "ready" to start asking for events once the server is initialized. So we set it to OK state right before that point, then wait on the channel until the server is ready.

tsandall pushed a commit that referenced this issue Aug 12, 2021
- Plugins can now access a channel which receives a message when the OPA
  server is fully initialized and ready to receive traffic.
- Added ServerInitialized() and ServerInitializedChannel() to plugin
  manager.
- Runtime now calls ServerInitialized() when server listeners are
  initialized.

Fixes #3701

Signed-off-by: Grant Shively <gshively@godaddy.com>
jgchn pushed a commit to jgchn/opa that referenced this issue Aug 20, 2021
- Plugins can now access a channel which receives a message when the OPA
  server is fully initialized and ready to receive traffic.
- Added ServerInitialized() and ServerInitializedChannel() to plugin
  manager.
- Runtime now calls ServerInitialized() when server listeners are
  initialized.

Fixes open-policy-agent#3701

Signed-off-by: Grant Shively <gshively@godaddy.com>
dolevf pushed a commit to dolevf/opa that referenced this issue Nov 4, 2021
- Plugins can now access a channel which receives a message when the OPA
  server is fully initialized and ready to receive traffic.
- Added ServerInitialized() and ServerInitializedChannel() to plugin
  manager.
- Runtime now calls ServerInitialized() when server listeners are
  initialized.

Fixes open-policy-agent#3701

Signed-off-by: Grant Shively <gshively@godaddy.com>
Signed-off-by: Dolev Farhi <farhi.dolev@gmail.com>
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 a pull request may close this issue.

2 participants