Make the plugin catalog endpoint roundtrip so we can use terraform to manage them. #3778
Conversation
|
Failures seem LDAP related? |
|
Yep, seems like the LDAP test server is down. |
vault/logical_system.go
Outdated
| args := d.Get("args").([]string) | ||
| // For backwards compatibility, also accept args as part of command. | ||
| parts := strings.Split(command, " ") | ||
| args = append(parts[1:], args...) |
There was a problem hiding this comment.
Please do a length check before this line to make sure the command actually has more than one value...otherwise it'll panic.
There was a problem hiding this comment.
Length >0 is assured by the check for the empty string above (as with the previous iteration of the code). But I'll add it anyway :-)
vault/logical_system.go
Outdated
| if len(parts) <= 0 { | ||
| return logical.ErrorResponse("missing command value"), nil | ||
| } | ||
| args = append(parts[1:], args...) |
There was a problem hiding this comment.
If they specify both args and put args in the command they'll end up with doubled args here. I think a better check would be to ensure that len(parts) == 1 and if args is set and the length of parts is not 1, throw an error asking them to pick one or the other.
| command := fmt.Sprintf("%s --test", filepath.Base(file.Name())) | ||
| err = core.pluginCatalog.Set("mysql-database-plugin", command, []byte{'1'}) | ||
| command := fmt.Sprintf("%s", filepath.Base(file.Name())) | ||
| err = core.pluginCatalog.Set("mysql-database-plugin", command, []string{"--test"}, []byte{'1'}) |
There was a problem hiding this comment.
Please also add a check to verify correct behavior if both args and command-with-args are given (after fixing above)
There was a problem hiding this comment.
I've added the test to logical_system_test.go, as the logic to ensure only command or args is specified is in logical_system.go - hope thats okay!
|
Looks good after the requested changes are made. |
|
@jefferai sorry for the delay, have addressed your comments. Thanks! |
|
Thanks! |
Fixes #3776
You can now do:
Which should be enough to allow terraform to manage plugins.