Move version out of SDK. #14229
Move version out of SDK. #14229
Conversation
…rt not addressed by this change is sdk/helper/useragent.String, which we'll want to deprecate in favour of PluginString.
|
Seems about right (aside from needing a run of gofumpt) |
|
It looks like |
# Conflicts: # command/agent.go # command/debug.go # command/operator_diagnose.go # command/server.go # sdk/framework/backend.go # sdk/framework/openapi_test.go # sdk/framework/path.go
…p updating the old sdk version and start updating the new non-sdk equivalent.
| @@ -31,6 +31,15 @@ var ( | |||
| // Given comments will be appended to the semicolon-delimited comment section. | |||
| // | |||
| // e.g. Vault/0.10.4 (+https://www.vaultproject.io/; go1.10.1; comment-0; comment-1) | |||
| // | |||
| // Deprecated: use PluginString instead. | |||
There was a problem hiding this comment.
I vote for we rip the band-aid off and delete this function, the variables above, and the reference above to sdk/version, and bump the major version of sdk.
There was a problem hiding this comment.
Or just replace it with PluginString() below, and then we don't have to bump the sdk version.
There was a problem hiding this comment.
+1 to replacing with PluginString
There was a problem hiding this comment.
How do you mean replace it? They have different arguments.
There was a problem hiding this comment.
Ah, I missed that. It looks like it will be non-trivial to replace some usages (like in agent). When I first looked I thought this function was mostly unused in our code base. Nevermind.
There was a problem hiding this comment.
What difficulty do you see with modifying agent?
There was a problem hiding this comment.
At a glance, I don't see a logical.PluginEnvironment in the agent to pass to PluginString().
There was a problem hiding this comment.
We spoke on slack, Chris's agent comment was re the idea of removing String(). But Since agent lives within the vault package, it now uses the internal useragent helper rather than sdk's, so that's not actually an impediment.
I'm starting to come around to the idea of removing useragent.String from the sdk. I don't think we actually need to bump the major since we're pre-1.0. Or do you mean going from v0.6 to v0.7?
There was a problem hiding this comment.
Bumping from 0.6 to 0.7 is what I meant.
# Conflicts: # Makefile # command/agent.go
# Conflicts: # http/sys_seal_test.go
ncabatoff
force-pushed
the
move-version-out-of-sdk
branch
from
019638b
to
312c257
Compare
Move version out of SDK. For now it's a copy rather than move: the part not addressed by this change is sdk/helper/useragent.String, which we'll want to remove in favour of PluginString. That will have to wait until we've removed uses of useragent.String from all builtins.
Move version out of SDK. For now it's a copy rather than move: the part not addressed by this change is sdk/helper/useragent.String, which we'll want to remove in favour of PluginString. That will have to wait until we've removed uses of useragent.String from all builtins.
The goal of this work is to simplify our release process so that we don't have to update the version in sdk, then update go.mod to pull in the new sdk version. Instead we put the version into the vault package itself.
The reason it was in the sdk was I think to allow plugins to have access to it. But that was wrongheaded: external plugins shouldn't be reporting the vault version they were built with as the vault version of the cluster, and builtin plugins are built with vault and thus don't need to go through the sdk to get the vault version. I've tried to make it so that regardless of the plugin type, they can get the correct vault version of the cluster to put into their user-agent string. This requires that they use the useragent.PluginString method instead of useragent.String. Followup work to modify plugins we own to do that is still needed.