Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement]: Run init commands before storing secrets to Vault #7532

Closed
ThomasKasene opened this issue Sep 15, 2023 · 3 comments
Closed

[Enhancement]: Run init commands before storing secrets to Vault #7532

ThomasKasene opened this issue Sep 15, 2023 · 3 comments

Comments

@ThomasKasene
Copy link

ThomasKasene commented Sep 15, 2023

Module

Vault

Proposal

In the VaultContainer there's this little method:

@Override
protected void containerIsStarted(InspectContainerResponse containerInfo) {
    addSecrets();
    runInitCommands();
}

This works fine for the most part, but I recently had a situation where I needed to run the kv (v1) engine on the postgresql secret path, so I added this to my config:

PostgreSQLContainer<?> VAULT_CONTAINER = new VaultContainer<>("hashicorp/vault:1.11.2")
    .withVaultToken("supersecret-vault-token")
    .withInitCommand(
        "secrets enable transit",
        "write -f transit/keys/my-key",
        "secrets enable -path=postgresql kv")
    .withSecretInVault("postgresql/test", "username=test", "password=test")
    .start();

It doesn't look as though existing secrets are affected by the secrets enable -path=postgresql kv, however. It does work if I comment out the .withSecretsInVault(...) line and add this monstrosity after the block above, which is more or less the same logic you'll find inside the addSecrets()-method:

try {
    Container.ExecResult execResult = VAULT_CONTAINER.execInContainer(
            "/bin/sh", "-c",
            "vault kv put postgresql/test username=test password=test");
    String result = execResult.getStdout();
    if (!result.contains("Success!")) {
        throw new IllegalStateException(execResult.getStderr());
    }
} catch (Exception exception) {
    throw new IllegalStateException("Failed to store secret in Vault", exception);
}

So my suggestion is to adjust the implementation of containerIsStarted(InspectContainerResponse) to this:

@Override
protected void containerIsStarted(InspectContainerResponse containerInfo) {
    runInitCommands();
    addSecrets();
}
@eddumelendez
Copy link
Member

Hi @ThomasKasene, yes, I think it's a bit confused but I would recommend to use just withInitiCommand as it just uses vault. So, kv put should be added to postgresql/test username=test password=test

.withInitCommand(
        "secrets enable transit",
        "write -f transit/keys/my-key",
        "secrets enable -path=postgresql kv",
        "kv put postgresql/test username=test password=test");

Short term, I see the deprecation of withSecretInVault in favor of withInitCommand. And, in the long term, as we are discussing this internally will probably not even provide utility methods for vault command as it couples the container implementation to the image. But, we will look for also the best experience so it is easy to run commands.

@ThomasKasene
Copy link
Author

ThomasKasene commented Sep 18, 2023

Thanks for your reply and suggested workaround! It worked, of course 😄

I'd contest any decision to not provide helper-methods, however:

Personally, I have no interest in knowing the APIs or command lines involved in talking to Vault. Vault is configured out-of-band of my application. Nevertheless, I needed to waste time reading up on them in order to get the testcontainer to work (and the API engine versioning stuff is no laughing matter, holy smokes!) It would've been a lot simpler for me if this had worked out of the box.

I appreciate what you say about coupling, but the helper methods could be overridable, could they not? And optionally, accept a DTO parameter that is also overridable for more flexibility.

@eddumelendez
Copy link
Member

I have no interest in knowing the APIs or command lines involved in talking to Vault

I can understand this as you are fine by running vault commands with execInContainer methods as you're familiar with Vault. If so, then we agree. As far as the user know there is execInContainer to execute commands in a live containers then if familiar with the cli, the user should be able to run those commands. There is also some users who expect some of those utility methods but I think it causes more damage than benefit. As mentioned before, we are planning to remove those but for now in VaultContainer, withSecretInVault has been deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants