add plugin runtime API #22469

thyton · 2023-08-21T16:33:50Z

Implement plugin runtime CRUD API

github-actions · 2023-08-21T16:51:30Z

Build Results:
All builds succeeded! ✅

github-actions · 2023-08-21T17:15:11Z

CI Results:
All Go tests succeeded! ✅

tomhjp

Looking good! LMK any thoughts on the API path comments. This is a good sized unit of work to land so definitely save the CLI changes for another PR. Feel free to ping me when the TODOs are tidied up (or earlier if you prefer) and I can take another more detailed pass.

vault/logical_system_paths.go

vault/logical_system.go

sdk/helper/pluginruntimeutil/runner.go

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

…/vault into VAULT-18181-plugin-runtime-api

tomhjp

Mostly nits and follow ups. The only substantive comments are to replace float with integer types throughout (unless I'm missing something) and deleting some APIs that relate to backwards compatibility baggage from the plugin catalog. Otherwise, LGTM!

vault/plugin_runtime_catalog.go

sdk/helper/pluginruntimeutil/config.go

vault/plugin_runtime_catalog.go

tomhjp · 2023-08-30T15:54:00Z

vault/plugin_runtime_catalog.go

+		return ErrPluginRuntimeNotFound
+	}
+
+	// TODO what happens to the running existing container with(out) running plugins. Need to shut it down before setting it?


The implementation doesn't belong in this PR, but I think we should handle this case higher up in the call stack - i.e. in the delete API handler function.

vault/logical_system_paths.go

vault/logical_system.go

api/sys_plugins_runtimes.go

…/vault into VAULT-18181-plugin-runtime-api

…context.Context parameter

… of mapstructure.Decode

thyton · 2023-08-31T01:55:50Z

api/sys_plugins_runtimes.go

+	runtimesJSON, err := json.Marshal(secret.Data["runtimes"])
+	if err != nil {
+		return nil, fmt.Errorf("unable to marshal runtimes")
+	}
+
+	var runtimes []PluginRuntimeDetails
+	if err = json.Unmarshal(runtimesJSON, &runtimes); err != nil {
+		return nil, fmt.Errorf("unable to parse runtimes")
+	}


I had a bit of struggle parsing the Data back to runtimes after changing to store the returned runtimes in Data as []map[string]any

vault/vault/logical_system.go

Lines 835 to 871 in 9e59d84

var data []map[string]any

for _, runtimeType := range consts.PluginRuntimeTypes {

if runtimeType == consts.PluginRuntimeTypeUnsupported {

continue

}

configs, err := b.Core.pluginRuntimeCatalog.List(ctx, runtimeType)

if err != nil {

return nil, err

}

if len(configs) > 0 {

sort.Slice(configs, func(i, j int) bool {

return strings.Compare(configs[i].Name, configs[j].Name) == -1

})

for _, conf := range configs {

data = append(data, map[string]any{

"name": conf.Name,

"type": conf.Type.String(),

"oci_runtime": conf.OCIRuntime,

"cgroup_parent": conf.CgroupParent,

"cpu": conf.CPU,

"memory": conf.Memory,

})

}

}

}

resp := &logical.Response{

Data: map[string]interface{}{},

}

if len(data) > 0 {

resp.Data["runtimes"] = data

}

return resp, nil

}

like in the example Response below.

{ "request_id": "e93d3f93-8e4f-8443-a803-f1c97c123456", "data": { "runtimes": [ { "name": "gvisor", "type": "container", "oci_runtime": "runsc", "cgroup_parent": "/cpulimit/", "cpu": 1, "memory": 10000 }, { "name": "foo", "type": "container", "oci_runtime": "otherociruntime", "cgroup_parent": "/memorylimit/", "cpu": 2, "memory": 20000 } ] }, "warnings": null, "auth": null }

I attempted use mapstructure.Decode different ways to parse Response.Data["runtimes"]
but TestList* sys_plugins_runtimes_test.go couldn't pass asOCIRuntime:"", CgroupParent:"" weren't parsed correctly.

=== RUN TestListPluginRuntimeTyped/container sys_plugins_runtimes_test.go:135: expected: &api.ListPluginRuntimesResponse{Runtimes:[]api.PluginRuntimeDetails{api.PluginRuntimeDetails{Type:"container", Name:"gvisor", OCIRuntime:"runsc", CgroupParent:"/cpulimit/", CPU:1, Memory:10000}}} got: &api.ListPluginRuntimesResponse{Runtimes:[]api.PluginRuntimeDetails{api.PluginRuntimeDetails{Type:"container", Name:"gvisor", OCIRuntime:"", CgroupParent:"", CPU:1, Memory:10000}}} === RUN TestListPluginRuntimeTyped/unsupported --- FAIL: TestListPluginRuntimeTyped (0.01s) === RUN TestListPluginRuntimeUntyped/#00 sys_plugins_runtimes_test.go:197: expected: &api.ListPluginRuntimesResponse{Runtimes:[]api.PluginRuntimeDetails{api.PluginRuntimeDetails{Type:"container", Name:"gvisor", OCIRuntime:"runsc", CgroupParent:"/cpulimit/", CPU:1, Memory:10000}, api.PluginRuntimeDetails{Type:"container", Name:"foo", OCIRuntime:"otherociruntime", CgroupParent:"/memorylimit/", CPU:2, Memory:20000}, api.PluginRuntimeDetails{Type:"container", Name:"bar", OCIRuntime:"otherociruntime", CgroupParent:"/cpulimit/", CPU:3, Memory:30000}}} got: &api.ListPluginRuntimesResponse{Runtimes:[]api.PluginRuntimeDetails{api.PluginRuntimeDetails{Type:"container", Name:"gvisor", OCIRuntime:"", CgroupParent:"", CPU:1, Memory:10000}, api.PluginRuntimeDetails{Type:"container", Name:"foo", OCIRuntime:"", CgroupParent:"", CPU:2, Memory:20000}, api.PluginRuntimeDetails{Type:"container", Name:"bar", OCIRuntime:"", CgroupParent:"", CPU:3, Memory:30000}}} --- FAIL: TestListPluginRuntimeUntyped (0.01s)

After mapstructure.Decode attemps, I resorted to use json.Marshal and json.Unmarshal to put Data["runtimes"] in JSON bytes and let the library do the work, and the tests pass now. Admittedly, I feel this is a bit weird and hacky way. I'm open to any thoughts and tips that you have.

The "bug" here was that mapstructure didn't correctly guess the translation between field name and JSON key. You can give it explicit instructions in 2 ways: either create a mapstructure.DecoderConfig and set the TagName to "json" so that it uses the json tags instead of its default "mapstructure". Or you can set the mapstructure keys in the struct like so:

type PluginRuntimeDetails struct { Type string `json:"type" mapstructure:"type"` Name string `json:"name" mapstructure:"name"` OCIRuntime string `json:"oci_runtime" mapstructure:"oci_runtime"` CgroupParent string `json:"cgroup_parent" mapstructure:"cgroup_parent"` CPU int64 `json:"cpu" mapstructure:"cpu"` Memory int64 `json:"memory" mapstructure:"memory"` }

If you go with the latter method, the tests pass with this as the decoding step, which is probably my personal preference because the "interesting" code is more straightforward:

if err = mapstructure.Decode(secret.Data["runtimes"], &runtimes); err != nil { return nil, fmt.Errorf("unable to parse runtimes") }

Setting DecoderConfig looks like this:

decoderConfig := mapstructure.DecoderConfig{ TagName: "json", Result: &runtimes, } decoder, err := mapstructure.NewDecoder(&decoderConfig) if err != nil { return nil, err } if err = decoder.Decode(secret.Data["runtimes"]); err != nil { return nil, fmt.Errorf("unable to parse runtimes") }

tomhjp

LGTM! One potential typo and just nits otherwise

api/sys_plugins_runtimes.go

tomhjp · 2023-08-31T12:51:45Z

api/sys_plugins_runtimes.go

+	runtimesJSON, err := json.Marshal(secret.Data["runtimes"])
+	if err != nil {
+		return nil, fmt.Errorf("unable to marshal runtimes")
+	}
+
+	var runtimes []PluginRuntimeDetails
+	if err = json.Unmarshal(runtimesJSON, &runtimes); err != nil {
+		return nil, fmt.Errorf("unable to parse runtimes")
+	}


The "bug" here was that mapstructure didn't correctly guess the translation between field name and JSON key. You can give it explicit instructions in 2 ways: either create a mapstructure.DecoderConfig and set the TagName to "json" so that it uses the json tags instead of its default "mapstructure". Or you can set the mapstructure keys in the struct like so:

type PluginRuntimeDetails struct { Type string `json:"type" mapstructure:"type"` Name string `json:"name" mapstructure:"name"` OCIRuntime string `json:"oci_runtime" mapstructure:"oci_runtime"` CgroupParent string `json:"cgroup_parent" mapstructure:"cgroup_parent"` CPU int64 `json:"cpu" mapstructure:"cpu"` Memory int64 `json:"memory" mapstructure:"memory"` }

If you go with the latter method, the tests pass with this as the decoding step, which is probably my personal preference because the "interesting" code is more straightforward:

if err = mapstructure.Decode(secret.Data["runtimes"], &runtimes); err != nil { return nil, fmt.Errorf("unable to parse runtimes") }

Setting DecoderConfig looks like this:

decoderConfig := mapstructure.DecoderConfig{ TagName: "json", Result: &runtimes, } decoder, err := mapstructure.NewDecoder(&decoderConfig) if err != nil { return nil, err } if err = decoder.Decode(secret.Data["runtimes"]); err != nil { return nil, fmt.Errorf("unable to parse runtimes") }

vault/logical_system.go

vault/logical_system_paths.go

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

add plugin runtime API

a67d58c

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

thyton requested a review from tomhjp August 21, 2023 16:34

thyton marked this pull request as draft August 21, 2023 16:34

tomhjp reviewed Aug 21, 2023

View reviewed changes

vault/logical_system_paths.go Outdated Show resolved Hide resolved

vault/logical_system_paths.go Show resolved Hide resolved

vault/logical_system.go Show resolved Hide resolved

sdk/helper/pluginruntimeutil/runner.go Outdated Show resolved Hide resolved

thyton and others added 6 commits August 22, 2023 18:32

Update sdk/helper/pluginruntimeutil/runner.go

2cfb2a6

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

Update sdk/helper/pluginruntimeutil/runner.go

9c360b6

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

Merge branch 'VAULT-18181-plugin-runtime-api' of github.com:hashicorp…

d3891d1

…/vault into VAULT-18181-plugin-runtime-api

change runner.go to config.go

bb32254

wip plugin runtime CRUD test

5107640

Merge branch 'main' into VAULT-18181-plugin-runtime-api

b10e679

vercel bot deployed to Preview August 29, 2023 23:32 View deployment

fix plugin runtime CRUD test failure

d2b8903

tomhjp reviewed Aug 30, 2023

View reviewed changes

thyton added 4 commits August 30, 2023 10:14

Merge branch 'main' into VAULT-18181-plugin-runtime-api

10ea40d

Merge branch 'VAULT-18181-plugin-runtime-api' of github.com:hashicorp…

0769af5

…/vault into VAULT-18181-plugin-runtime-api

add copyright headers

8352ecd

change cpu to int64 type and refactor PluginRuntimeCatalog.Set()

49a26e0

thyton added this to the 1.15 milestone Aug 30, 2023

thyton added 4 commits August 30, 2023 11:27

faithfully represent the data passed to us in storage

66201a1

remove ListPluginRuntimesWithContext and use ListPluginRuntimes with …

d06ef06

…context.Context parameter

list API returns a slice of plugin runtimes in response data

8ebbdfb

using encoding/json to marshal and unmarshal runtimes in Data instead…

75d6d48

… of mapstructure.Decode

thyton marked this pull request as ready for review August 31, 2023 01:09

change data from type []map[string]interface{} to []map[string]any

9e59d84

thyton commented Aug 31, 2023

View reviewed changes

thyton requested a review from tomhjp August 31, 2023 02:02

tomhjp approved these changes Aug 31, 2023

View reviewed changes

thyton and others added 4 commits August 31, 2023 08:31

Update api/sys_plugins_runtimes.go

48bfbc0

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

Update vault/logical_system.go

b030803

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

add mapstructure tags to PluginRuntimeDetails

5a18988

rename fields cpu and memory to cpu_nanos and memory_bytes

257d6fe

thyton removed this from the 1.15 milestone Aug 31, 2023

thyton added pr/no-changelog pr/no-milestone labels Aug 31, 2023

thyton added 3 commits August 31, 2023 10:47

add /sys/plugins/runtimes/catalog/{type}{name} to sudo paths

47353fc

add /sys/plugins/runtimes/catalog to sudo paths

bbb7c88

revert the regex change in /sys/plugins/runtimes/catalog/{type}/{name}

c5185d3

thyton merged commit 0857450 into main Aug 31, 2023
102 checks passed

thyton deleted the VAULT-18181-plugin-runtime-api branch August 31, 2023 20:37

tomhjp mentioned this pull request Aug 31, 2023

Support running plugins in isolated containers #22712

Merged

thyton mentioned this pull request Sep 1, 2023

add copyright header #22739

Merged

thyton removed the pr/no-milestone label Sep 7, 2023

thyton added this to the 1.15 milestone Sep 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add plugin runtime API #22469

add plugin runtime API #22469

thyton commented Aug 21, 2023 •

edited

Loading

github-actions bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023 •

edited

Loading

tomhjp left a comment

tomhjp left a comment

tomhjp Aug 30, 2023

thyton Aug 31, 2023 •

edited

Loading

thyton Aug 31, 2023

tomhjp Aug 31, 2023

tomhjp left a comment

tomhjp Aug 31, 2023

	var data []map[string]any
	for _, runtimeType := range consts.PluginRuntimeTypes {
	if runtimeType == consts.PluginRuntimeTypeUnsupported {
	continue
	}
	configs, err := b.Core.pluginRuntimeCatalog.List(ctx, runtimeType)
	if err != nil {
	return nil, err
	}

	if len(configs) > 0 {
	sort.Slice(configs, func(i, j int) bool {
	return strings.Compare(configs[i].Name, configs[j].Name) == -1
	})
	for _, conf := range configs {
	data = append(data, map[string]any{
	"name": conf.Name,
	"type": conf.Type.String(),
	"oci_runtime": conf.OCIRuntime,
	"cgroup_parent": conf.CgroupParent,
	"cpu": conf.CPU,
	"memory": conf.Memory,
	})
	}
	}
	}

	resp := &logical.Response{
	Data: map[string]interface{}{},
	}

	if len(data) > 0 {
	resp.Data["runtimes"] = data
	}

	return resp, nil
	}

add plugin runtime API #22469

add plugin runtime API #22469

Conversation

thyton commented Aug 21, 2023 • edited Loading

github-actions bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023 • edited Loading

tomhjp left a comment

Choose a reason for hiding this comment

tomhjp left a comment

Choose a reason for hiding this comment

tomhjp Aug 30, 2023

Choose a reason for hiding this comment

thyton Aug 31, 2023 • edited Loading

Choose a reason for hiding this comment

thyton Aug 31, 2023

Choose a reason for hiding this comment

tomhjp Aug 31, 2023

Choose a reason for hiding this comment

tomhjp left a comment

Choose a reason for hiding this comment

tomhjp Aug 31, 2023

Choose a reason for hiding this comment

thyton commented Aug 21, 2023 •

edited

Loading

github-actions bot commented Aug 21, 2023 •

edited

Loading

thyton Aug 31, 2023 •

edited

Loading