Allow TLS certificate to authenticate against the named certificate role

[ielatif]

Fixes #874

[spencergibb]

Thank you for you contribution! Please add a test.

[mp911de]

Looking at the repetitions, would you mind introducing an abstraction that removes duplications?

A possible design could be a context runner utility that accepts reactive, bootstrap and test class details (basically everything that is changing across individual test methods while hiding everything that remains the same) and then invokes app.run(…)?

[ielatif]

Thanks @mp911de for your review.

A possible design could be a context runner utility that accepts reactive, bootstrap and test class details (basically everything that is changing across individual test methods while hiding everything that remains the same) and then invokes app.run(…)?

Are you considering a generic solution for all Vault tests or a specific abstraction for TLS authentication tests?

However, i can remove the duplications by configuring the SpringApplication app with shared properties

SpringApplication app = new SpringApplicationBuilder().sources(TestConfig.class)
	.web(WebApplicationType.NONE)
	.properties("spring.cloud.vault.authentication=cert",
		"spring.cloud.vault.ssl.key-store=file:../work/client-cert.jks",
		"spring.cloud.vault.ssl.key-store-password=changeit",
		"spring.cloud.vault.application-name=VaultConfigTlsCertAuthenticationTests",
		"spring.cloud.vault.reactive.enabled=false", "spring.cloud.bootstrap.enabled=true")
	.build();

Then add specific properties when running tests :

ConfigurableApplicationContext context = this.app.run("--spring.cloud.vault.ssl.role=my-role");

[mp911de]

Are you considering a generic solution for all Vault tests or a specific abstraction for TLS authentication tests?

Going forward, I'd like to have a consistent approach for all tests, however in the scope of this ticket, let's only focus on this test class to keep the scope rather focussed. In any case, appreciate your view outside of the box :-)

I think that SpringApplicationBuilder is a fine abstraction for the low-level part. By putting the aspect in front what is relevant for the actual test turns the entire property handling (names, values) rather into noise. I'm thinking about a functional design approach along the lines of:

VaultTestContextRuner.of(this.app).testClass(getClass()).sources(…).property("spring.cloud.vault.ssl.role", "my-role").run()

We would pick it then up for other tests and extend it to:

VaultTestContextRuner.of(this.app).sources(…).config(c -> c.reactive(true/false).boostrap(true/false)).run()

This is mostly an idea for now, when we introduce the broader notion of VaultTestContextRuner, we might refine the API as needed and based on how it makes the most sense.

[ielatif]

I like the idea. I can explore it further in a separate PR

[ielatif]

@mp911de I had some time this weekend, so I gave your idea a try.

[mp911de]

Thanks a lot. Let us take this PR from here.

[ielatif]

@mp911de while working on #751 I discovred that spring-vault have rich testing utilities than spring-cloud-vault like VaultExtension and VaultInitializer. I think it will be very helpfull if we extract these utilities in a seperate module spring-vault-test and use them in spring-cloud-vault tests to avoid boilerplate code for example in @BeforeAll. It will also facilitate maintenance as code will be put in one place.

What do you think?

[mp911de]

VaultExtension works well for Spring Vault expecting Vault to run on a specific port assuming an initial key and SSL configuration being provided through a bundle. These are rather specific assumptions that don't hold necessarily true for application out there. I don't have much reason to introduce another module and maintain test support, especially that nowadays testcontainers-driven integration testing is much more common than it has been when creating the project.

[ielatif]

I completely agree with this assessment about Testcontainers.
I haven't proposed using Testcontainers primarily because I assumed that option was off the table for this project's testing approach. Thanks for clarifying it.

[mp911de]

Thank you for your contribution. That's merged and polished now. VaultTestContextRunner is a decent improvement to keep Vault integration testing settings in a common place. In some areas, we're using deliberately ApplicationContextRunner as it uses isolated auto-configuration and some places still make use of @SpringBootTest. We don't have a test module and we don't want to introduce one just for the sake of simplifying some additional tests right now. In any case, this has been a good exercise.

[ielatif]

In any case, this has been a good exercise.

Indeed!
I enjoyed contributing to this and greatly appreciate your valuable feedback.
Thanks.

[mp911de]

Likewise, you might want to have a look at the polishing commits that follow your contribution commit to get an idea how the context runner has evolved.

[ielatif]

I Love it! More robust more fluent a master touch! :)