test/sign defines a CreateOperation but no ExistenceCheck

[haraldh]

Describe the bug A clear and concise description of what the bug is.

changing the sdk from:

-       github.com/hashicorp/vault/sdk v0.9.2-0.20230704151349-7522ca248f90
+       github.com/hashicorp/vault/sdk v0.9.2

let's my plugin tests fail with:

2023-08-02T16:55:57.031+0200 [INFO]  core: http: panic serving 127.0.0.1:56846: Pattern managed-keys/(?P<type>\w(([\w-.]+)?\w)?)/(?P<name>\w(([\w-.]+)?\w)?)/test/sign defines a CreateOperation but no ExistenceCheck

introduced by commit 00e13abf1fe79a00d4564acd634902b7634dbd47 which adds the panic.

with https://github.com/hashicorp/vault/blob/64f92b40fc5858e1dee68a1be6eddfda859f5698/vault/logical_system_helpers.go#L253-L253

[maxb]

The conversation at https://github.com/hashicorp/vault/discussions/22080 may be useful to you.

[maxb]

The conversation at #22080 may be useful to you.

To expand upon this in slightly more detail now that I'm at a real keyboard...

You are combining a post-#18492 github.com/hashicorp/vault/sdk with a pre-#18492 github.com/hashicorp/vault. This is expected to fail.

Importing github.com/hashicorp/vault as a Go module at all is explicitly not supported but if you want to do it anyway, you need to be careful about managing its version in sync with the sdk module.

You may find this unofficial documentation I wrote helpful: https://github.com/maxb/vault-docs/blob/main/modules.md#what-about-using-githubcomhashicorpvault-itself-as-a-go-module

[haraldh]

@maxb Thank you for the explanation!

[jaredallard]

I follow the above explanation, but it seems like there may be an actual issue now?

If you attempt to import github.com/hashicorp/vault@1.14.2 with github.com/hashicorp/vault/sdk@v0.9.2 (which is what 1.14.2 is using), the test functions don't work with the above panic.

I went ahead and created a repro here, in case it's useful: https://github.com/jaredallard/vault-issue

Note: This happens without a CoreConfig as well, which leads me to believe some sort of regression occurred in one of the test functions.

[maxb]

github.com/hashicorp/vault@1.14.2 with github.com/hashicorp/vault/sdk@v0.9.2 (which is what 1.14.2 is using)

That's not actually the case. In fact you can see here: https://github.com/hashicorp/vault/blob/v1.14.2/go.mod#L23 that vault 1.14.2 is using the copy of sdk embedded within the same commit, which is substantially different to v0.9.2.

You may find the document I wrote about Vault's Go modules useful: https://github.com/maxb/vault-docs/blob/main/modules.md especially the final section which discusses how to select appropriate versions of vault and sdk that are compatible.

[jaredallard]

You may find the document I wrote about Vault's Go modules useful: maxb/vault-docs@main/modules.md especially the final section which discusses how to select appropriate versions of vault and sdk that are compatible.

Hmmmm, I even followed those steps exactly and it always ends up with go mod tidy resetting the /sdk dependency to v0.9.2. To be clear, it's not a blocking issue for me. I had followed those steps for v1.14.1 and it works as expected.

The go mod tidy resetting is what lead me to believe that v0.9.2 was in-use, but yeah, it doesn't appear to be the case. This may be isolated to some misunderstanding with go mod here.

Here's what I was doing in that same https://github.com/jaredallard/vault-issue repo.

$ cat go.mod | grep hashicorp/vault | grep -v //
        github.com/hashicorp/vault v1.14.2
        github.com/hashicorp/vault/sdk v0.9.2

$ git ls-remote https://github.com/hashicorp/vault v1.14.2
16a7033a0686eca50ee650880d5c55438d274489        refs/tags/v1.14.2

$ sed -i.bak 's_github.com/hashicorp/vault/sdk v0.9.2_github.com/hashicorp/vault/sdk 16a7033a0686eca50ee650880d5c55438d274489_' go.mod
$ sed -i.bak 's_github.com/hashicorp/vault v1.14.2_github.com/hashicorp/vault 16a7033a0686eca50ee650880d5c55438d274489_' go.mod

$ cat go.mod | grep hashicorp/vault | grep -v //
        github.com/hashicorp/vault 16a7033a0686eca50ee650880d5c55438d274489
        github.com/hashicorp/vault/sdk 16a7033a0686eca50ee650880d5c55438d274489

$ go mod tidy

$ cat go.mod | grep hashicorp/vault | grep -v //
        github.com/hashicorp/vault v1.14.2
        github.com/hashicorp/vault/sdk v0.9.2

If you try to do only the go get steps in the above doc, you get:

$ git ls-remote https://github.com/hashicorp/vault v1.14.2
16a7033a0686eca50ee650880d5c55438d274489        refs/tags/v1.14.2

$ go get github.com/hashicorp/vault@16a7033a0686eca50ee650880d5c55438d274489
go: warning: github.com/aerospike/aerospike-client-go/v5@v5.6.0: retracted by module author: Scan/Query/Other streaming commands could put a faulty connection back to the pool after a cluster event where in certain conditions its contents would end up in another scan and mix the results.
go: warning: github.com/hashicorp/go-msgpack@v1.1.5: retracted by module author: v1.1.5 merged upstream ugorji/go which breaks compatibility with previous versions.
go: to switch to the latest unretracted version, run:
        go get <module>@latest

$ go get github.com/hashicorp/vault/sdk@16a7033a0686eca50ee650880d5c55438d274489
go: downgraded github.com/hashicorp/vault v1.14.2 => v1.14.1
go: downgraded github.com/hashicorp/vault-plugin-auth-azure v0.16.0 => v0.15.1
go: downgraded github.com/hashicorp/vault-plugin-database-snowflake v0.9.0 => v0.8.0
go: downgraded github.com/hashicorp/vault/sdk v0.9.2 => v0.9.2-0.20230824131912-16a7033a0686

$ cat go.mod | grep hashicorp/vault | grep -v //
        github.com/hashicorp/vault v1.14.1
        github.com/hashicorp/vault/sdk v0.9.2-0.20230824131912-16a7033a0686

[maxb]

Oh.... I see what's going on here :-(

The version number of sdk listed in the Vault go.mod file is a lie (because it is overridden by the replace directive), but usually it is a lie in the direction of being too old.

When it is too old, there's no problem with asking go get or go mod tidy to actually bring in a newer version.

But, (for the first time ever?) in Vault v1.14.2, the version number of sdk listed there is now too new.

This causes go get or go mod tidy to incorrectly force the version of sdk too high, if you ever try to combine Vault and sdk in your own project.

There is no way to counteract this without using a replace directive of your own.

HashiCorp have been saying for a long time that depending on Vault itself as a Go module is not supported, but it looks like in v1.14.2 it has become newly more difficult to make use of it that way.

Why are you trying to use v1.14.2 anyway? If you have a Vault plugin that you are building use sdk v0.9.2, I'd just use whatever commit of Vault that matches sdk v0.9.2:

$ go get github.com/hashicorp/vault{,/sdk}@fd20c99c4a8ae3412308961b813b8dc43f399543
...
$ grep -E '/vault(/sdk)? ' go.mod 
    github.com/hashicorp/vault v1.2.1-0.20230725160220-fd20c99c4a8a
    github.com/hashicorp/vault/sdk v0.9.2
$ go run .
... it works ...

[jaredallard]

Why are you trying to use v1.14.2 anyway? If you have a Vault plugin that you are building use sdk v0.9.2, I'd just use whatever commit of Vault that matches sdk v0.9.2:

I'm using it for an integration test of a vault client that we use at my company that's an alternative to the current Go SDK, so using a specific version of Vault is useful for testing compatibility. We use vault.TestCoreUnsealedWithConfig to spin up an in-process version of Vault.

Of course, one could say it's much safer to just spin up a docker container of Vault instead. They would be not wrong 😄

Anyways, 1.14.1 works fine for us now, and if this continues to not function in newer versions of Vault we can just move to using a Docker container with setup/teardown before each test. NBD! Thanks for helping!

[maxb]

Yeah... You really should be running Vault as a separate process for that, in order to stick to supported interfaces. Either via Docker, or just run the Vault server binary directly - it's a single file executable with no dependencies, after all.

[heliobmartins]

Hey guys, apologies if this re-opens the issue, but I'm kinda facing the same problem here. And I was just expecting to clarify some of the questions I have regarding this change if you don't mind.

We are currently running Vault 1.15.0, with the following dependencies:

github.com/hashicorp/vault/api v1.10.0
github.com/hashicorp/vault/sdk v0.9.1

We are not importing github.com/hashicorp/vault in our go.mod.

I looked through the documentation you wrote @maxb , which is very nice by the way, but I'm slightly confused regarding the change, more specifically around the ExistenceCheck function. Based on the documentation for the function, it seems that the implementation of this function is optional, right?

    // ExistenceCheck, if implemented, is used to query whether a given
    // resource exists or not. This is used for ACL purposes: if an Update
    // action is specified, and the existence check returns false, the action
    // is not allowed since the resource must first be created. The reverse is
    // also true. If not specified, the Update action is forced and the user
    // must have UpdateCapability on the path.

Once we attempt to upgrade Vault to a version >= v0.9.2, we encountered the exact same issue for the custom paths we have implemented within our custom plugin and that implements the operation logical.CreateOperation. For example:

        {
            Pattern: "secret/" + framework.MatchAllRegex("path"),
            Fields: map[string]*framework.FieldSchema{
                "path": {
                    Type:        framework.TypeString,
                    Description: "Path of secret",
                },
                "data": {
                    Type:        framework.TypeMap,
                    Description: "The contents of the data map will be stored and returned on read.",
                },
            },
            Operations: map[logical.Operation]framework.OperationHandler{
                logical.CreateOperation: &framework.PathOperation{
                    Callback: b.withMetadataModeCheck(b.pathSecretCreate),
                },
                logical.UpdateOperation: &framework.PathOperation{
                    Callback: b.withMetadataModeCheck(b.pathSecretCreate),
                },
            },
            ExistenceCheck: func(ctx context.Context, request *logical.Request, data *framework.FieldData) (bool, error) {
                //TODO: I noticed that this might now be required to be implemented?
                //return (false/true), nil
            },
        },

Do we have any documentation on how we should be handling this change for the new sdk versions, or do we intend to update https://github.com/hashicorp/vault-auth-plugin-example, to reflect the changes with the new sdk / api?

Thanks in advance for the help here.

[maxb]

@heliobmartins This is a different issue to what was originally reported - the original issue was about people trying to import Vault itself as a Go module.

The purpose of ExistenceCheck is to allow Vault to decide whether an HTTP PUT/POST is processed as a CreateOperation or an UpdateOperation. When no ExistenceCheck is defined, Vault treats everything as an UpdateOperation. Some people don't know this, and end up implementing a CreateOperation handler even though they have not implemented ExistenceCheck. When this happens, the CreateOperation handler is redundant unreachable code, and Vault's OpenAPI generator used to end up incorrectly claiming x-vault-createSupported for such APIs even though they don't actually support create properly.

So I implemented an extra check that detects this incorrect code, and forces it to be corrected.

If you have existing code that is triggering it, you can simply delete your CreateOperation implementation to resolve the problem.

https://github.com/hashicorp/vault-auth-plugin-example requires no update for this, it is already correct.

[heliobmartins]

Oh! I see what it means now @maxb , thanks so much for the help. I really appreciate it. The explanation now is very very clear.