Refactor TenantAware - remove TenantRunner and replace with standard Runnable / Callable (#2755)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2025-10-14 16:36:42 +03:00
committed by GitHub
parent 0a2f18fbad
commit 04cd9fb30d
14 changed files with 88 additions and 118 deletions

View File

@@ -47,7 +47,7 @@ public interface SystemManagement {
/**
* Runs consumer for each tenant as
* {@link TenantAware#runAsTenant(String, org.eclipse.hawkbit.tenancy.TenantAware.TenantRunner)}
* {@link TenantAware#runAsTenant(String, java.util.concurrent.Callable)}
* silently (i.e. exceptions will be logged but operations will continue for further tenants).
*
* @param consumer to run as tenant

View File

@@ -15,46 +15,37 @@ import org.eclipse.hawkbit.im.authentication.SpPermission;
import org.eclipse.hawkbit.repository.RolloutApprovalStrategy;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.repository.model.Rollout;
import org.eclipse.hawkbit.repository.model.Rollout.RolloutStatus;
import org.eclipse.hawkbit.security.SystemSecurityContext;
import org.eclipse.hawkbit.tenancy.UserAuthoritiesResolver;
import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.User;
import org.springframework.util.ObjectUtils;
/**
* Default implementation of {@link RolloutApprovalStrategy}. Decides whether
* approval is needed based on configuration of the tenant as well as the roles
* of the user who created the Rollout. Provides a no-operation implementation
* of {@link RolloutApprovalStrategy#onApprovalRequired(Rollout)}.
* Default implementation of {@link RolloutApprovalStrategy}. Decides whether approval is needed based on configuration of the tenant as well
* as the roles of the user who created the Rollout. Provides a no-operation implementation of
* {@link RolloutApprovalStrategy#onApprovalRequired(Rollout)}.
*/
public class DefaultRolloutApprovalStrategy implements RolloutApprovalStrategy {
private final UserAuthoritiesResolver userAuthoritiesResolver;
private final TenantConfigurationManagement tenantConfigurationManagement;
private final SystemSecurityContext systemSecurityContext;
DefaultRolloutApprovalStrategy(
final UserAuthoritiesResolver userAuthoritiesResolver,
final TenantConfigurationManagement tenantConfigurationManagement,
final SystemSecurityContext systemSecurityContext) {
this.userAuthoritiesResolver = userAuthoritiesResolver;
this.tenantConfigurationManagement = tenantConfigurationManagement;
this.systemSecurityContext = systemSecurityContext;
}
/**
* Returns true, if rollout approval is enabled and rollout creator doesn't
* have approval role.
* Returns true, if rollout approval is enabled and rollout creator doesn't have approval role. It have to be called in the user context
*/
@Override
public boolean isApprovalNeeded(final Rollout rollout) {
return isApprovalEnabled() && hasNoApproveRolloutPermission(getActorAuthorities(rollout));
return isApprovalEnabled() && hasNoApproveRolloutPermission(
getCurrentAuthentication().getAuthorities().stream().map(GrantedAuthority::getAuthority).toList());
}
/***
@@ -85,19 +76,4 @@ public class DefaultRolloutApprovalStrategy implements RolloutApprovalStrategy {
return systemSecurityContext.runAsSystem(() -> tenantConfigurationManagement
.getConfigurationValue(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, Boolean.class).getValue());
}
private Collection<String> getActorAuthorities(final Rollout rollout) {
// rollout state transition from CREATING to CREATED is managed by
// scheduler under SYSTEM user context, thus we get the
// user based on the properties of initially created rollout entity
if (RolloutStatus.CREATING == rollout.getStatus()) {
final String actor = rollout.getLastModifiedBy() != null ? rollout.getLastModifiedBy() : rollout.getCreatedBy();
if (!ObjectUtils.isEmpty(actor)) {
return systemSecurityContext.runAsSystem(() -> userAuthoritiesResolver.getUserAuthorities(rollout.getTenant(), actor));
}
}
return ((User) getCurrentAuthentication().getPrincipal()).getAuthorities().stream()
.map(GrantedAuthority::getAuthority).toList();
}
}

View File

@@ -106,7 +106,6 @@ import org.eclipse.hawkbit.security.HawkbitSecurityProperties;
import org.eclipse.hawkbit.security.SecurityTokenGenerator;
import org.eclipse.hawkbit.security.SystemSecurityContext;
import org.eclipse.hawkbit.tenancy.TenantAware;
import org.eclipse.hawkbit.tenancy.UserAuthoritiesResolver;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
@@ -397,10 +396,9 @@ public class JpaRepositoryConfiguration {
*/
@Bean
@ConditionalOnMissingBean
RolloutApprovalStrategy rolloutApprovalStrategy(final UserAuthoritiesResolver userAuthoritiesResolver,
final TenantConfigurationManagement tenantConfigurationManagement,
final SystemSecurityContext systemSecurityContext) {
return new DefaultRolloutApprovalStrategy(userAuthoritiesResolver, tenantConfigurationManagement, systemSecurityContext);
RolloutApprovalStrategy rolloutApprovalStrategy(
final TenantConfigurationManagement tenantConfigurationManagement, final SystemSecurityContext systemSecurityContext) {
return new DefaultRolloutApprovalStrategy(tenantConfigurationManagement, systemSecurityContext);
}
/**

View File

@@ -155,10 +155,7 @@ public class JpaRolloutExecutor implements RolloutExecutor {
context -> // has stored context - executes it with it
contextAware.runInContext(context, () -> execute0(rollout)),
() -> // has no stored context - executes it in the tenant & user scope
contextAware.runAsTenantAsUser(contextAware.getCurrentTenant(), rollout.getCreatedBy(), () -> {
execute0(rollout);
return null;
}));
contextAware.runAsTenantAsUser(contextAware.getCurrentTenant(), rollout.getCreatedBy(), () -> execute0(rollout)));
}
private void execute0(final Rollout rollout) {

View File

@@ -142,10 +142,7 @@ public class AutoAssignChecker implements AutoAssignExecutor {
() -> // has no stored context - executes it in the tenant & user scope
contextAware.runAsTenantAsUser(
contextAware.getCurrentTenant(),
getAutoAssignmentInitiatedBy(filterQuery), () -> {
consumer.accept(filterQuery);
return null;
})
getAutoAssignmentInitiatedBy(filterQuery), () -> consumer.accept(filterQuery))
);
} catch (final RuntimeException ex) {
if (log.isDebugEnabled()) {

View File

@@ -9,6 +9,7 @@
*/
package org.eclipse.hawkbit.repository.jpa.management;
import static org.eclipse.hawkbit.im.authentication.SpPermission.READ_GATEWAY_SECURITY_TOKEN;
import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY;
import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.BATCH_ASSIGNMENTS_ENABLED;
import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.MULTI_ASSIGNMENTS_ENABLED;
@@ -28,7 +29,6 @@ import java.util.function.Function;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.im.authentication.SpPermission;
import org.eclipse.hawkbit.repository.TargetFields;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.repository.event.remote.TenantConfigurationDeletedEvent;
@@ -248,10 +248,9 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana
private void checkAccess(final String configurationKeyName) {
if (AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY.equalsIgnoreCase(configurationKeyName)) {
final SystemSecurityContext systemSecurityContext = SystemSecurityContextHolder.getInstance().getSystemSecurityContext();
if (!systemSecurityContext.isCurrentThreadSystemCode() &&
!systemSecurityContext.hasPermission(SpPermission.READ_GATEWAY_SECURITY_TOKEN)) {
if (!SystemSecurityContext.isCurrentThreadSystemCode() && !systemSecurityContext.hasPermission(READ_GATEWAY_SECURITY_TOKEN)) {
throw new InsufficientPermissionException(
"Can't read gateway security token! " + SpPermission.READ_GATEWAY_SECURITY_TOKEN + " is required!");
"Can't read gateway security token! " + READ_GATEWAY_SECURITY_TOKEN + " is required!");
}
}
}

View File

@@ -216,7 +216,7 @@ public class JpaTarget extends AbstractJpaNamedEntity implements Target, EventAw
@Override
public String getSecurityToken() {
final SystemSecurityContext systemSecurityContext = SystemSecurityContextHolder.getInstance().getSystemSecurityContext();
if (systemSecurityContext.isCurrentThreadSystemCode() || systemSecurityContext.hasPermission(SpPermission.READ_TARGET_SECURITY_TOKEN)) {
if (SystemSecurityContext.isCurrentThreadSystemCode() || systemSecurityContext.hasPermission(SpPermission.READ_TARGET_SECURITY_TOKEN)) {
return securityToken;
}
return null;

View File

@@ -11,6 +11,7 @@ package org.eclipse.hawkbit.repository.jpa.autoassign;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -27,8 +28,8 @@ import org.eclipse.hawkbit.repository.TargetFilterQueryManagement;
import org.eclipse.hawkbit.repository.TargetManagement;
import org.eclipse.hawkbit.repository.model.DeploymentRequest;
import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
import org.eclipse.hawkbit.tenancy.TenantAware;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -47,9 +48,9 @@ import org.springframework.transaction.PlatformTransactionManager;
class AutoAssignCheckerTest {
@Mock
private TargetFilterQueryManagement targetFilterQueryManagement;
private TargetFilterQueryManagement<? extends TargetFilterQuery> targetFilterQueryManagement;
@Mock
private TargetManagement targetManagement;
private TargetManagement<? extends Target> targetManagement;
@Mock
private DeploymentManagement deploymentManagement;
@Mock
@@ -57,12 +58,12 @@ class AutoAssignCheckerTest {
@Mock
private ContextAware contextAware;
private AutoAssignChecker sut;
private AutoAssignChecker autoAssignChecker;
@BeforeEach
void before() {
sut = new AutoAssignChecker(targetFilterQueryManagement, targetManagement, deploymentManagement,
transactionManager, contextAware);
autoAssignChecker = new AutoAssignChecker(
targetFilterQueryManagement, targetManagement, deploymentManagement, transactionManager, contextAware);
}
/**
@@ -75,18 +76,15 @@ class AutoAssignCheckerTest {
final long ds = getRandomLong();
final TargetFilterQuery matching = mockFilterQuery(ds);
final TargetFilterQuery notMatching = mockFilterQuery(ds);
when(targetFilterQueryManagement.findWithAutoAssignDS(any()))
.thenReturn(new SliceImpl<>(Arrays.asList(notMatching, matching)));
when(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(target, ds, matching.getQuery()))
.thenReturn(true);
when(targetFilterQueryManagement.findWithAutoAssignDS(any())).thenReturn(new SliceImpl<>(Arrays.asList(notMatching, matching)));
when(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(target, ds, matching.getQuery())).thenReturn(true);
when(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(target, ds, notMatching.getQuery()))
.thenReturn(false);
sut.checkSingleTarget(target);
autoAssignChecker.checkSingleTarget(target);
verify(deploymentManagement).assignDistributionSets(eq(matching.getAutoAssignInitiatedBy()),
Mockito.argThat(deployReqMatcher(target, ds)), any());
verify(deploymentManagement).assignDistributionSets(
eq(matching.getAutoAssignInitiatedBy()), Mockito.argThat(deployReqMatcher(target, ds)), any());
Mockito.verifyNoMoreInteractions(deploymentManagement);
}
@@ -118,8 +116,9 @@ class AutoAssignCheckerTest {
private void mockRunningAsNonSystem() {
when(contextAware.getCurrentTenant()).thenReturn(getRandomString());
when(contextAware.runAsTenantAsUser(any(String.class), any(String.class), any(TenantAware.TenantRunner.class)))
.thenAnswer(i -> ((TenantAware.TenantRunner) i.getArgument(2)).run());
doAnswer(i -> {
((Runnable) i.getArgument(2)).run();
return null;
}).when(contextAware).runAsTenantAsUser(any(String.class), any(String.class), any(Runnable.class));
}
}
}