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

Resilience4jCircuitBreaker ignoring Context set by ContextPropagator #182

Closed
Kustosh opened this issue Dec 19, 2023 · 12 comments
Closed

Resilience4jCircuitBreaker ignoring Context set by ContextPropagator #182

Kustosh opened this issue Dec 19, 2023 · 12 comments

Comments

@Kustosh
Copy link

Kustosh commented Dec 19, 2023

Hello everyone,
recently we have upgraded our Spring Cloud version to 2023.0.0 that updated spring-cloud-starter-circuitbreaker-resilience4j from version 3.0.3 to version 3.1.0. After that migration our service was not able to propagate security context when calling external service through OpenFeign client. We were using ContextPropagator to pass the the SecurityContext to the circuitbreaker in order to retrieve custom authentication token and pass it in the next request.
It seems that after the upgrade to latest version the thread that is running the Feign request is not populated with context as it was before.

After some investigation we suspect that this change might be responsible. According to some issues from the resilience4j GitHub such as this spawning additional threads after ThreadPoolBulkhead already created one might result in issues that we are observing.

We have found a solution to circumvent this issue by customizing the Resilience4jCircuitBreakerFactory with DelegatingSecurityContextExecutorService, but this would force us to disable the groups as it is not yet supported (#180) and we need to have groups enabled.

For now we have reverted to version 3.0.3 of spring-cloud-starter-circuitbreaker-resilience4j which works with the ContextPropagator. Would it be possible to re-enable support for it?

Sample code below:

Feign client and configuration

@FeignClient(url = "localhost:8090",
    name = "FeignApiClient",
    configuration = {FeignClientConfiguration.class})
public interface FeignApiClient extends ApiDescription {

}
public class FeignClientConfiguration {

    @Bean
    public RequestInterceptor oauth2RequestInterceptor() {
        return new SecurityContextAwareRequestInterceptor();
    }

}

Our custom Feign request interceptor to retrieve the security context and set proper headers (it is just a dummy one, so it does not set the actual values):

public class SecurityContextAwareRequestInterceptor implements RequestInterceptor {

    @Override
    public void apply(final RequestTemplate template) {
        getAuthenticationOrNull().ifPresentOrElse(authentication -> template.header(HttpHeaders.AUTHORIZATION, "Bearer " + authentication), () -> System.out.println("Security context is empty"));
    }

    private Optional<UsernamePasswordAuthenticationToken> getAuthenticationOrNull() {
        return Optional.ofNullable(SecurityContextHolder.getContext()).map(SecurityContext::getAuthentication).map(authentication -> (UsernamePasswordAuthenticationToken) authentication);
    }
}

And our ContextPropagator to set SecurityContext in threads spawned by resilience4j:

public class SecurityContextPropagator implements ContextPropagator<SecurityContext> {

    @Override
    public Supplier<Optional<SecurityContext>> retrieve() {
        return () -> Optional.ofNullable(SecurityContextHolder.getContext());
    }

    @Override
    public Consumer<Optional<SecurityContext>> copy() {
        return context -> SecurityContextHolder.setContext(context.orElse(null));
    }

    @Override
    public Consumer<Optional<SecurityContext>> clear() {
        return context -> {

        };
    }
}
@ryanjbaxter
Copy link
Contributor

Group support should be there. In fact #180 should have been closed by #181

@spring-cloud-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@Kustosh
Copy link
Author

Kustosh commented Dec 28, 2023

Group support should be there. In fact #180 should have been closed by #181

Oh, that's great, I missed that the PR was already merged. Then we will switch to this approach when the change is released.

Still, I would appreciate if someone would look into support for ContextPropagator from resilience4j or at least mention in the docs that ContextPropagator is not compatible and other approach is required.

@ryanjbaxter
Copy link
Contributor

Have you tried to see if it works without using OpenFeign?

@Kustosh
Copy link
Author

Kustosh commented Jan 2, 2024

I tried adding something like this:

public String getValue() {
        return circuitBreakerFactory.create("no-feign-circuitbreaker", "no-feign").run(() -> {
            SecurityContext context = SecurityContextHolder.getContext();
            return Optional.ofNullable(context).map(SecurityContext::getAuthentication)
                .filter(authentication -> authentication instanceof UsernamePasswordAuthenticationToken)
                .map(UsernamePasswordAuthenticationToken.class::cast)
                .map(authentication -> {
                    System.out.println("Context is here");
                    return restClient.get().uri("http://localhost:8090").header("Authentication",
                        authentication.getName()).exchange((clientRequest, clientResponse) -> {
                        return new String(clientResponse.getBody().readAllBytes(), StandardCharsets.UTF_8);
                    });
                })
                .orElseGet(() -> {
                    System.out.println("No Context Available");
                    return "";
                });
        }, throwable -> fallback());
    }

    public String fallback() {
        return "Ups, something's broken";
    }

The result is the same, using version 3.0.3 of spring-cloud-circuitbreaker-resilience4j I can access the properly populated SecurityContext within the method wrapped by the circuitbreaker, but using version 3.1.0 results in an empty context. In both cases I can see that the ContextPropagator is called, but only version 3.0.3 actually allows me to access the context.

@ryanjbaxter
Copy link
Contributor

You mentioned...

We have found a solution to circumvent this issue by customizing the Resilience4jCircuitBreakerFactory with DelegatingSecurityContextExecutorService, but this would force us to disable the groups as it is not yet supported (#180) and we need to have groups enabled.

As mentioned above groups are now supported, so does this workaround work for your use case then?

Can you describe how you are customizing Resilience4jCircuitBreakerFactory?

@Kustosh
Copy link
Author

Kustosh commented Jan 5, 2024

Sorry, I think there was some miscommunication there. Yes, the latest code from the master branch works with this workaround when I added following configuration:

@Configuration
public class Resilience4JSecurityContextDelegatingConfig {

    @Bean
    public Customizer<Resilience4JCircuitBreakerFactory> groupExecutorServiceCustomizer() {
        return factory -> factory.configureGroupExecutorService(group -> new DelegatingSecurityContextExecutorService(Executors.newVirtualThreadPerTaskExecutor()));
    }

}

This, along with the ContextPropagator, solves the issue of the missing context, both for the raw spring-cloud-circuitbreaker as well as for the OpenFeign client. But the group support is not yet released if I'm not mistaken?

@ryanjbaxter
Copy link
Contributor

But the group support is not yet released if I'm not mistaken?

Correct, it should be in the 2202.0.5 release which is currently scheduled for the end of the month. Could you try 2202.0.5-SNAPSHOT to see if it addresses the concern?

We can add the Customizer configuration to the docs to document how to make the context propagation work.

@Kustosh
Copy link
Author

Kustosh commented Jan 10, 2024

Yes, the snapshot version together with the Customizer fixes the problem as far as I can tell. Thank you for help and information.

@luisalves00
Copy link

Adding:

@Bean public Customizer<Resilience4JCircuitBreakerFactory> groupExecutorServiceCustomizer() { return factory -> factory.configureGroupExecutorService(group -> new DelegatingSecurityContextExecutorService(Executors.newVirtualThreadPerTaskExecutor())); }

is enough or do we need the ContextPropagator? How to configure it?

@gls-luis-alves
Copy link

Tried:

@Bean
	public Customizer< Resilience4JCircuitBreakerFactory > executorServiceCustomizer()
	{
		return factory ->
		{
			SecurityContextPropagator securityContextPropagator = new SecurityContextPropagator();
			final ContextAwareScheduledThreadPoolExecutor.Builder executorBuilder = ContextAwareScheduledThreadPoolExecutor.newScheduledThreadPool();
			executorBuilder.corePoolSize( 6 );
			executorBuilder.contextPropagators( securityContextPropagator );
			factory.configureExecutorService( executorBuilder.build() );
			factory.configureGroupExecutorService( groupName -> executorBuilder.build() );
		};
	}

and would expect my thread to have in the name the following:

THREAD_PREFIX = "ContextAwareScheduledThreadPool"

But from the logs, it seems that the pool that was defined is not the used one:

2024-06-07 10:36:29.926 DEBUG [pool-3-thread-2] - (xxx.business.service.client.DummyClient:72) - [DummyClient#get] ---> GET http://dummy-service/v1/user/abc HTTP/1.1
2024-06-07 10:36:30.927 ERROR [http-nio-8080-exec-4] - (xxx.validation.ErrorHandlingControllerAdvice:66) - Unhandled Exception
org.springframework.cloud.client.circuitbreaker.NoFallbackAvailableException: No fallback available.

Request failed because it had no Bearer token.

@gls-luis-alves
Copy link

Finally I've figured out.
The Customizer was not applied, because of wrong import.

Make sure you import:

import org.springframework.cloud.client.circuitbreaker.Customizer

Then you can use configuration properties:

resilience4j:
  scheduled:
    executor:
      corePoolSize: 8
      contextPropagators:
        - xxx.config.SecurityContextPropagator

or you can do it programmatically:

	@Bean
	public ContextAwareScheduledThreadPoolExecutor contextAwareScheduledThreadPool(){
           return ContextAwareScheduledThreadPoolExecutor.newScheduledThreadPool()
	          .corePoolSize(8)
                  .contextPropagators(new SecurityContextPropagator()).build();
        }

Then like in the Spring Cloud docs, you can use the Customizer:

	@Bean
	public Customizer< Resilience4JCircuitBreakerFactory > executorServiceCustomizer( ContextAwareScheduledThreadPoolExecutor executor )
	{
		log.info( "Configuring Resilience4JCircuitBreaker executor service with context propagation!" );
		return factory ->
		{
			// https://docs.spring.io/spring-cloud-circuitbreaker/reference/spring-cloud-circuitbreaker-resilience4j/specific-circuit-breaker-configuration.html
			// https://docs.spring.io/spring-cloud-circuitbreaker/docs/current/reference/html/spring-cloud-circuitbreaker-resilience4j.html#_customizing_the_executorservice
			factory.configureExecutorService( executor );
			factory.configureGroupExecutorService( groupName -> executor );
		};
	}

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

5 participants