Make sure vault.unseal runner is unauthenticated

[voyvodov]

What does this PR do?

vault.unseal runner will now use unauthenticated query to unseal Vault

What issues does this PR fix or reference?

Fixes: 85

Previous Behavior

vault.unseal runner was trying to authenticate with Vault before unseal it.

New Behavior

vault.unseal runner will not do any authentication with Vault.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes

[lkubb]

Thanks for submitting bug + fix and sorry for taking a while!

Have you verified this actually fixes the behavior? Afair is_unauthd only influences whether a token use is deducted (internal statistics).

I think in the code's current state you might need to create the unauthenticated client yourself (since it's a runner function we can just rely on the opts directly):

from saltext.vault.utils import factory
from saltext.vault.utils.client import VaultClient
# ...

def unseal():
    config = factory.parse_config(__opts__.get("vault", {}))
    client = VaultClient(**config["server"], **config["client"])
    for key in __opts__["vault"]["keys"]:
        ret = client.post("sys/unseal", payload={"key": key})

[voyvodov]

You're right :( .
I was expecting that unauth is really unauth. Probably that will be good to be fixed.

Using your solution for now (will prepare another one where unauth is really unauth)

[lkubb]

I don't think making vault.query with is_unauthd=True do an unauthenticated request is the correct layer to fix this behavior. The parameter is passed through to the authenticated client and is an ugly workaround intended to avoid unnecessary token reissuance when calling arbitrary endpoints - afair the token is still optionally used for auditing purposes.*

I would prefer introducing a separate vault.query_noauth helper function essentially doing the same as above (for locally sourced configuration).

* A more theoretical concern that's specific to the sealed state is that it would only work as expected when sourcing the parameters from local configuration.

[voyvodov]

Wait...what? I was thinking that I pushed the change (using the client directly).
Will do it in a few minutes 🤦🏻‍♂️

[voyvodov]

Updated. Now it uses directly the client (so no tokens, authentication are used). Pretty much raw call (as it should be)

[lkubb]

Thanks :)

We could consider extracting the unauthenticated query part into a dedicated helper function in the future, but for now it's fine like that.