Skip to content

feat: Adapt the client and the integration tests to the KinD based CAPI - #1383

Open
ZPascal wants to merge 2 commits into
cloudfoundry:5.x.xfrom
ZPascal:integration-test-adaptions
Open

ZPascal wants to merge 2 commits into
cloudfoundry:5.x.xfrom
ZPascal:integration-test-adaptions

Conversation

@ZPascal

@ZPascal ZPascal commented Oct 7, 2026 •

Copy link
Copy Markdown

The integration tests fail against the current CAPI deployed on KinD (see #1381). This PR fixes what broke for 5.x.x.

Client

  • Bits uploads now always send a resources field. CAPI otherwise rejects them (CF-AppBitsUploadInvalid / missing :resources).
    • v3 packages: the resources part is always added, as JSON, and is [] when there are no matched resources.
    • v2 apps: the nginx upload module can drop the form field, so when there are no matched resources I also add resources=[] as a query parameter. Cloud Controller accepts it there, but only an empty list fits into a URL, so a non-empty list stays a form field only.
  • LdapConfiguration and SamlConfiguration got a caCertificates list for the CA certificates of the identity provider.

Integration tests

  • At startup, the tests now reserve the TCP ports that NameFactory hands out (1025 to the maximum) on the default-tcp router group. The TCP route tests no longer depend on the test order or on the default port range of the deployment. Setting SKIP_TCP_ROUTING_TESTS skips this. If there is no default-tcp group, it only logs a warning.
  • The routing client for this uses its own token provider, because the shared one depends on the cleaner, which now depends on this bean.

Testing

  • Unit tests of cloudfoundry-client, cloudfoundry-client-reactor and cloudfoundry-operations pass on JDK 11
  • I added tests for the v2 upload with and without async, and extended the v3 upload test to check the resources part.
  • The integration tests were made functional as part of this PR using a current CAPI release and tested here (main branch)

Connected PR

#1381

@ZPascal
ZPascal marked this pull request as draft October 7, 2026 20:25
@ZPascal
ZPascal force-pushed the integration-test-adaptions branch 3 times, most recently from 3b9b621 to 483635d Compare October 8, 2026 05:46
@ZPascal
ZPascal changed the base branch from main to 5.x.x October 8, 2026 05:59
@ZPascal ZPascal closed this Oct 10, 2026
@ZPascal
ZPascal force-pushed the integration-test-adaptions branch from 483635d to a4ad3db Compare October 10, 2026 04:15
@ZPascal ZPascal reopened this Oct 10, 2026
Make the integration tests pass against a CAPI deployed on KinD.

Client:
- Send the "resources" field with every bits upload (v3 packages) and
  also as query parameter for v2 uploads without matched resources, as
  CAPI otherwise rejects them (CF-AppBitsUploadInvalid / missing
  :resources).
- Add the CA certificates of the LDAP and SAML identity provider
  configurations.

Integration tests:
- Reserve the TCP ports handed out by the NameFactory on the default-tcp
  router group at startup, so the TCP route tests don't depend on the
  test order or the deployment's default range. Skipped together with
  the TCP routing tests (SKIP_TCP_ROUTING_TESTS).
@ZPascal
ZPascal force-pushed the integration-test-adaptions branch from 57252f5 to 2321e47 Compare October 10, 2026 07:17
@ZPascal
ZPascal marked this pull request as ready for review October 10, 2026 09:04
@ZPascal ZPascal changed the title Adapt the client and the integration tests to the KinD based CAPI feat: Adapt the client and the integration tests to the KinD based CAPI Oct 10, 2026

@Kehrlann Kehrlann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not make the routerGroupPorts a bean.

Comment on lines +461 to +462
// Tests allocate TCP ports from NameFactory, so the default router group has to reserve them
// regardless of the order in which the tests run (or of the deployment's default range).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a bit unsure what we're doing with this bean. Are we making sure that ports can be reserved on the default group in the first place, before we do anything else? And this is only so that the reservable range from the CF config is overridden by the config we have in our tests, correct?

This follows the established pattern, which, unfortunately, is hacky and convoluted (e.g. @Bean(initMethod = "block") Mono<...> is bad). Let's try and make it cleaner.

I don't see why the CC Cleaner depends on this though - is it only to avoid doing an initial cleanup if the routing is not possible?

If that's just it, I'm ok paying the price of the initial cleanup just for clarity:

  1. We can re-use the existing routing client
  2. We can use ApplicationContextRunner and make stuff less crazy

e.g.

@Bean
@ConditionalOnExpression("!'true'.equalsIgnoreCase(systemEnvironment['SKIP_TCP_ROUTING_TESTS'])")
ApplicationRunner setReservablePort(RoutingClient routingClient) {
    // Ensure that the default-router-group reservable port range matches whatever our tests use.
    return args -> {
        var defaultGroup = routingClient
                .routerGroups()
                .list(ListRouterGroupsRequest.builder().build())
                .flatMapIterable(ListRouterGroupsResponse::getRouterGroups)
                .filter(group -> "default-tcp".equals(group.getName())).blockFirst();

        if (defaultGroup == null) {
            throw new RuntimeException("No default-tcp router group, TCP route tests will fail. " +
                                   "Set SKIP_TCP_ROUTING_TESTS=true to skip TCP routing tests entirely.");
        }

        routingClient
                .routerGroups()
                .update(
                        UpdateRouterGroupRequest.builder()
                                .reservablePorts(
                                        RandomNameFactory.PORT_RANGE)
                                .routerGroupId(defaultGroup.getRouterGroupId())
                                .build()).block();
    };
}

In the longer run, it'd be much, much better to move the CF cleaner to a similar pattern (but not as part of this PR)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants