Interface for event subscription plugins; SQS plugin #24352

swenson · 2023-12-04T19:28:04Z

Looking to get feedback on how this is looking so far.

I've sketched out a plugin interface for how event subscription plugins would work, that is, a plugin that would grab events from Vault's event bus and publish them to an external place.

(Perhaps calling them event publish plugins would be better?)

As a POC, I've started sketching what such a plugin would look like for AWS SQS.

github-actions · 2023-12-04T19:42:23Z

CI Results:
All Go tests succeeded! ✅

sdk/event/plugin_server.go

plugins/event/sqs/cmd/vault-plugin-events-sqs/main.go

tomcf-hcp · 2023-12-04T19:44:29Z

sdk/eventplugin/eventplugin.proto

+	string eventJson = 2;
+}
+
+message SubscribeRequest {


Is there a way to subscribe to a grouping of events, or is each one a separate thing?

It's flexible -- this is a low-level interface that is more about plumbing events to SQS (e.g.), but it will depend on how we want to configure the endpoint that matches up event topics / bexpr to the subscription.

My current inclination is to support globs in the event topic and bexpr, similar to how we do websockets, so that the interfaces are similar.

How will we reference a particular events plugin mount in that matching up endpoint, and where in the API are we hosting this? Will it all be in the sys/ mount? In particular, secrets engines give us a bit of a headache because they can occupy basically any path that isn't already reserved for e.g. auth/ or sys/

tomhjp

Sorry for the delay reviewing, left some thoughts and questions. I'd be interested to understand the plans for plumbing events into this interface in a bit more detail, I don't have a great feel for the whole flow yet.

sdk/event/event_subscription_plugin.go

plugins/event/sqs/sqs.go

tomhjp · 2024-01-05T15:40:31Z

sdk/eventplugin/eventplugin.proto

+	string eventJson = 2;
+}
+
+message SubscribeRequest {


How will we reference a particular events plugin mount in that matching up endpoint, and where in the API are we hosting this? Will it all be in the sys/ mount? In particular, secrets engines give us a bit of a headache because they can occupy basically any path that isn't already reserved for e.g. auth/ or sys/

plugins/event/sqs/sqs.go

tomhjp · 2024-01-05T15:50:49Z

sdk/eventplugin/eventplugin.proto

+	rpc Initialize(InitializeRequest) returns (InitializeResponse);
+	// Start a new subscription.
+	rpc Subscribe(SubscribeRequest) returns (SubscribeResponse);
+	// Used by Vault to send events to a subscription. Only one stream is allowed per subscription.


Trying to get my head around the model here - do we envisage mounting events plugins like other plugins, or is there only ever going to be a single instance of them and they'll handle multiple subscriptions?

Then there's a 1-1 mapping between subscription and stream - is there any case where a stream will restart or is closing a stream tantamount to calling Unsubscribe?

And there's going to be a many:many mapping from event filters to subscriptions that's not defined in this PR right?

For now, there's a 1-1 mapping between subscriptions and plugin instances, but I wanted to leave the interface flexible about that, so that it could potentially support multiplexing natively without having to be retrofitted later.

I think closing the stream and unsubscribing should be synonymous.

And yes, a subscription will allow (potentially many) event type globs + event filters mapped to a subscription.

swenson · 2024-01-05T19:02:01Z

How will we reference a particular events plugin mount in that matching up endpoint, and where in the API are we hosting this? Will it all be in the sys/ mount? In particular, secrets engines give us a bit of a headache because they can occupy basically any path that isn't already reserved for e.g. auth/ or sys/

@tomhjp My current thinking is that this isn't a mount, but more like how the database plugin works: you configure a new subscription, which specifies the event sub-plugin you want (like SQS) along with the event type glob and event filter, and then the event system/backend will manage the plugin instance and route everything.

This will be the next PR, I think.

github-actions · 2024-01-05T20:20:03Z

Build Results:
All builds succeeded! ✅

sdk/helper/backoff/backoff.go

sdk/event/plugin_server.go

sdk/event/grpc_client.go

tomhjp · 2024-01-29T15:18:45Z

sdk/event/event_subscription_plugin.go

+	"github.com/hashicorp/vault/sdk/logical"
+)
+
+type EventSubscriptionPlugin interface {


I think the plugin interface and proto definition should probably live in plugins/event until we enable external plugins, otherwise it becomes part of our external facing API unnecessarily early.

plugins/event/sqs/sqs.go

sdk/event/plugin_client.go

austingebauer · 2024-01-30T21:33:06Z

plugins/event/sqs/sqs.go

+		options = append(options, awsutil.WithRegion(sconfig.Region))
+	}
+	options = append(options, awsutil.WithEnvironmentCredentials(true))
+	credConfig, err := awsutil.NewCredentialsConfig(options...)


That's right that NewCredentialsConfig doesn't support WithEnvironmentCredentials from what I'm seeing. I'm not exactly sure what the motivation for that was. The only place the option is applied is within GenerateCredentialChain. I think you can also pass it through via GetSession.

I wish awsutil was more opinionated to help with consistency. A good start that's consistent with most AWS integrations would be to make sure the following credentials providers are in the chain (in order):

1.credentials.StaticProvider
2. credentials.EnvProvider
3. credentials.SharedCredentialsProvider

I do think it'd be worth checking out awsutil/v2 to get ahead of a potential future migration.

Looking to get feedback on how this is looking so far. I've sketched out a plugin interface for how event subscription plugins would work, that is, a plugin that would grab events from Vault's event bus and publish them to an external place. (Perhaps calling them event publish plugins would be better?) As a POC, I've started sketching what such a plugin would look like for AWS SQS.

And rename Type() to PluginName() and PluginVersion()

Co-authored-by: Tom Proctor <[email protected]>

swenson · 2024-01-31T22:06:01Z

I realized that I probably could use awsutil/v2, so I'm going to rework this to pull that in if I can.

swenson · 2024-01-31T23:04:35Z

Okay, this is ready for review again.

I've decided not to go with awsutil/v2 for now, as it doesn't seem to support environment credentials, and in general I'm having a lot harder of a time getting it to be useful. (I kept finding myself not able to use the options it provides, or bypassing it since it does not have what we need.)
Even though the awsutil.NewCredentialsConfig says it does not support awsutil.WithEnvironmentCredentials, the credentials are correctly getting picked up from the environment. (Otherwise my tests would not pass when I run them.)

tomhjp

Looking good 👍 I think we should spin out a separate ticket to look at updates we need to make to the awsutil library based on the use-case here, as it would be nice to get there eventually but it shouldn't block the rest of the PR.

sdk/event/event_subscription_plugin.go

vault/logical_system.go

tomhjp · 2024-02-02T13:02:48Z

command/commands.go

@@ -185,6 +187,10 @@ var (
 		"plugin": plugin.Factory,
 	}

+	eventBackends = map[string]event.Factory{
+		"sqs": sqs.New,


It doesn't really matter right now, but just FYI we'll want to move this to helper/builtinplugins/registry.go instead when we enable external plugins. It's one of the most confusing parts of how plugins are configured, but these plugin maps just cover the plugins that can't be overridden or registered externally - the mounting code completely skips checking the plugin registry if it finds an entry here.

Ack. It is definitely confusing :)

plugins/event/sqs/sqs_test.go

plugins/event/sqs/sqs.go

tomhjp · 2024-02-02T13:54:39Z

plugins/event/sqs/sqs.go

+	if !ok {
+		return
+	}
+	conn.ctxCancelFunc()


I don't see how this will ever have an effect on network calls, because we only ever set the ctx in the connection struct, and never use it. I think there are two contexts we care about for cancelling in-flight requests; this one for the whole subscription ID and the per-request ctx that gets passed with the Send method call. However, neither ctx is passed into the retry loop's logic or API call currently AFAICT.

Oh, you're right, we never use the context. I think in a previous iteration we did, but I simplified the logic and left the context in still.

I'll go ahead and remove this.

It would be really nice to be able to cancel/timeout in-flight requests, but I'm ok with deferring that for now as it seems a little tied up in the v1/v2 package concerns. Please could we also track that as part of the ticket for updating to v2 awsutil and sdk?

tomhjp

LGTM 👍

swenson · 2024-02-05T21:15:48Z

Thanks!

Initial version of an internal plugin interface for event subscription plugins, and an AWS SQS plugin as an example. Co-authored-by: Tom Proctor <[email protected]>

swenson requested a review from a team December 4, 2023 19:28

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Dec 4, 2023

github-advanced-security bot found potential problems Dec 4, 2023

View reviewed changes

sdk/event/plugin_server.go Fixed Show fixed Hide fixed

tomcf-hcp reviewed Dec 4, 2023

View reviewed changes

swenson force-pushed the vault-16977/event-plugin-interface branch from da162db to bad8e6d Compare January 2, 2024 18:28

tomhjp reviewed Jan 5, 2024

View reviewed changes

swenson force-pushed the vault-16977/event-plugin-interface branch from bad8e6d to 5f87693 Compare January 5, 2024 18:51

swenson marked this pull request as ready for review January 5, 2024 20:04

swenson requested a review from a team as a code owner January 5, 2024 20:04

swenson added this to the 1.16.0-rc1 milestone Jan 5, 2024

swenson changed the title ~~Draft interface for event subscription plugins~~ Interface for event subscription plugins; SQS plugin Jan 9, 2024

tomhjp reviewed Jan 29, 2024

View reviewed changes

austingebauer reviewed Jan 30, 2024

View reviewed changes

Christopher Swenson and others added 11 commits January 31, 2024 14:02

Reformat proto and clean up code

cf1ce90

Add basic SQS sub tests

b138210

Use awsutil and aws-sdk-go v1

44f4c6f

Add copywrite headers to sqs_test.go

e98c0d0

Remove Initialize() from event proto

1e8c6ee

And rename Type() to PluginName() and PluginVersion()

Add changelog

a3a8848

Delete gRPC stuff for now

5372b54

Use simplified, single-method interface for subscribe/send

8739518

Address most review comments

2d2f1df

Update sdk/helper/backoff/backoff.go

6954046

Co-authored-by: Tom Proctor <[email protected]>

swenson force-pushed the vault-16977/event-plugin-interface branch from 7c16b13 to 6954046 Compare January 31, 2024 22:05

Christopher Swenson added 3 commits January 31, 2024 15:05

revert buf.yaml changes

cf6d64e

Wire up sqs.New into the core map of plugins

3d4bf76

Add copywrite headers

436aba3

tomhjp reviewed Feb 2, 2024

View reviewed changes

Address PR feedback

74b6e98

tomhjp approved these changes Feb 5, 2024

View reviewed changes

swenson merged commit 8f6dfaa into main Feb 5, 2024
113 checks passed

swenson deleted the vault-16977/event-plugin-interface branch February 5, 2024 21:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interface for event subscription plugins; SQS plugin #24352

Interface for event subscription plugins; SQS plugin #24352

swenson commented Dec 4, 2023

github-actions bot commented Dec 4, 2023 •

edited

Loading

tomcf-hcp Dec 4, 2023

swenson Jan 2, 2024

tomhjp Jan 5, 2024

tomhjp left a comment

tomhjp Jan 5, 2024

tomhjp Jan 5, 2024

swenson Jan 5, 2024

swenson commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

tomhjp Jan 29, 2024

austingebauer Jan 30, 2024

swenson commented Jan 31, 2024

swenson commented Jan 31, 2024

tomhjp left a comment

tomhjp Feb 2, 2024

swenson Feb 2, 2024

tomhjp Feb 2, 2024

swenson Feb 2, 2024

tomhjp Feb 5, 2024

tomhjp left a comment

swenson commented Feb 5, 2024

Interface for event subscription plugins; SQS plugin #24352

Interface for event subscription plugins; SQS plugin #24352

Conversation

swenson commented Dec 4, 2023

github-actions bot commented Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomhjp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swenson commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swenson commented Jan 31, 2024

swenson commented Jan 31, 2024

tomhjp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomhjp left a comment

Choose a reason for hiding this comment

swenson commented Feb 5, 2024

github-actions bot commented Dec 4, 2023 •

edited

Loading