gravitational / teleport

The easiest, and most secure way to access and protect all of your infrastructure.
https://goteleport.com
GNU Affero General Public License v3.0
17.44k stars 1.74k forks source link

Unable to SetPluginData with compare-and-swap semantics when data is not yet exist #6953

Open marshall-lee opened 3 years ago

marshall-lee commented 3 years ago

Description

What happened:

I started to use SetPluginData extensively and realized that it's impossible to call SetPluginData atomically in a "create-if-empty" manner.

The reason is that I cannot pass empty Expect parameter. When serializing, protobuf doesn't make a difference between empty map and nil map. Given that Teleport treats nil expectation as an ignore of compare-and-swap semantics, I don't get a desired result — compare-with-empty-and-swap.

What you expected to happen:

I expect this code not to be prone to race conditions:

// setNewPluginData sets plugin data but only if didn't exist before.
func (a *App) setNewPluginData(ctx context.Context, reqID string, data PluginData) error {
    expect := make(map[string]string)
    return a.apiClient.UpdatePluginData(ctx, types.PluginDataUpdateParams{
        Kind:     types.KindAccessRequest,
        Resource: reqID,
        Plugin:   pluginName,
        Set:      EncodePluginData(data),
        Expect:   expect,
    })
}

Wonder what a solution here would be. Maybe add some new parameter like IfEmpty: true or do some magic with serialization of Expect.

marshall-lee commented 3 years ago

It's worth to mention @fspmarshall

fspmarshall commented 3 years ago

While something like this could definitely be added, there is a fairly simple workaround that should achieve this behavior:

So long as there is at least one key that always exists when the data is set, you can "expect" that key as the empty string (which we treat as expecting the key to not exist). If there is not one key that always exists, then a sentinel key can be used for this purpose.