fix: Enable atomic pushes in git using an environment configuration (#33367)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
Nidhi 2024-05-14 10:24:21 +05:30 committed by GitHub
parent 77193b6227
commit dd73e8b91f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
24 changed files with 212 additions and 74 deletions

View File

@ -6,6 +6,7 @@ export const tenantConfigConnection: string[] = [
"showRolesAndGroups",
"hideWatermark",
"userSessionTimeoutInMinutes",
"isAtomicPushAllowed",
];
export const RESTART_POLL_TIMEOUT = 2 * 150 * 1000;

View File

@ -121,6 +121,15 @@ export const APPSMITH_USER_SESSION_TIMEOUT_SETTING: Setting = {
isDisabled: () => true,
};
export const APPSMITH_IS_ATOMIC_PUSH_ALLOWED: Setting = {
id: "isAtomicPushAllowed",
name: "isAtomicPushAllowed",
category: SettingCategories.GENERAL,
controlType: SettingTypes.CHECKBOX,
label: "Allow atomic pushes",
text: "Git operations on this tenant should attempt to perform pushes atomically",
};
export const APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING: Setting = {
id: "APPSMITH_ALLOWED_FRAME_ANCESTORS",
name: "APPSMITH_ALLOWED_FRAME_ANCESTORS",
@ -201,6 +210,7 @@ export const config: AdminConfigType = {
APPSMITH_SHOW_ROLES_AND_GROUPS_SETTING,
APPSMITH_SINGLE_USER_PER_SESSION_SETTING,
APPSMITH_USER_SESSION_TIMEOUT_SETTING,
APPSMITH_IS_ATOMIC_PUSH_ALLOWED,
APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING,
],
} as AdminConfigType;

View File

@ -1,5 +1,6 @@
package com.appsmith.git.service;
import com.appsmith.external.configurations.git.GitConfig;
import com.appsmith.external.git.GitExecutor;
import com.appsmith.git.configurations.GitServiceConfig;
import com.appsmith.git.service.ce.GitExecutorCEImpl;
@ -12,8 +13,8 @@ import org.springframework.stereotype.Component;
@Primary
@Slf4j
public class GitExecutorImpl extends GitExecutorCEImpl implements GitExecutor {
public GitExecutorImpl(GitServiceConfig gitServiceConfig, ObservationRegistry observationRegistry) {
super(gitServiceConfig, observationRegistry);
public GitExecutorImpl(
GitServiceConfig gitServiceConfig, GitConfig gitConfig, ObservationRegistry observationRegistry) {
super(gitServiceConfig, gitConfig, observationRegistry);
}
}

View File

@ -1,5 +1,6 @@
package com.appsmith.git.service.ce;
import com.appsmith.external.configurations.git.GitConfig;
import com.appsmith.external.constants.AnalyticsEvents;
import com.appsmith.external.constants.ErrorReferenceDocUrl;
import com.appsmith.external.dtos.GitBranchDTO;
@ -80,6 +81,7 @@ public class GitExecutorCEImpl implements GitExecutor {
private final RepositoryHelper repositoryHelper = new RepositoryHelper();
private final GitServiceConfig gitServiceConfig;
private final GitConfig gitConfig;
protected final ObservationRegistry observationRegistry;
@ -222,43 +224,49 @@ public class GitExecutorCEImpl implements GitExecutor {
// open the repo
Path baseRepoPath = createRepoPath(repoSuffix);
return Mono.using(
() -> Git.open(baseRepoPath.toFile()),
git -> Mono.fromCallable(() -> {
log.debug(Thread.currentThread().getName() + ": pushing changes to remote "
+ remoteUrl);
// open the repo
Stopwatch processStopwatch = StopwatchHelpers.startStopwatch(
baseRepoPath, AnalyticsEvents.GIT_PUSH.getEventName());
TransportConfigCallback transportConfigCallback =
new SshTransportConfigCallback(privateKey, publicKey);
return gitConfig
.getIsAtomicPushAllowed()
.flatMap(isAtomicPushAllowed -> {
return Mono.using(
() -> Git.open(baseRepoPath.toFile()),
git -> Mono.fromCallable(() -> {
log.debug(Thread.currentThread().getName() + ": pushing changes to remote "
+ remoteUrl);
// open the repo
Stopwatch processStopwatch = StopwatchHelpers.startStopwatch(
baseRepoPath, AnalyticsEvents.GIT_PUSH.getEventName());
TransportConfigCallback transportConfigCallback =
new SshTransportConfigCallback(privateKey, publicKey);
StringBuilder result = new StringBuilder("Pushed successfully with status : ");
git.push()
.setTransportConfigCallback(transportConfigCallback)
.setRemote(remoteUrl)
.call()
.forEach(pushResult -> pushResult
.getRemoteUpdates()
.forEach(remoteRefUpdate -> {
result.append(remoteRefUpdate.getStatus())
.append(",");
if (!StringUtils.isEmptyOrNull(remoteRefUpdate.getMessage())) {
result.append(remoteRefUpdate.getMessage())
StringBuilder result = new StringBuilder("Pushed successfully with status : ");
git.push()
.setAtomic(isAtomicPushAllowed)
.setTransportConfigCallback(transportConfigCallback)
.setRemote(remoteUrl)
.call()
.forEach(pushResult -> pushResult
.getRemoteUpdates()
.forEach(remoteRefUpdate -> {
result.append(remoteRefUpdate.getStatus())
.append(",");
}
}));
// We can support username and password in future if needed
// pushCommand.setCredentialsProvider(new
// UsernamePasswordCredentialsProvider("username",
// "password"));
processStopwatch.stopAndLogTimeInMillis();
return result.substring(0, result.length() - 1);
})
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
.name(GitSpan.FS_PUSH)
.tap(Micrometer.observation(observationRegistry)),
Git::close)
if (!StringUtils.isEmptyOrNull(
remoteRefUpdate.getMessage())) {
result.append(remoteRefUpdate.getMessage())
.append(",");
}
}));
// We can support username and password in future if needed
// pushCommand.setCredentialsProvider(new
// UsernamePasswordCredentialsProvider("username",
// "password"));
processStopwatch.stopAndLogTimeInMillis();
return result.substring(0, result.length() - 1);
})
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
.name(GitSpan.FS_PUSH)
.tap(Micrometer.observation(observationRegistry)),
Git::close);
})
.subscribeOn(scheduler);
}

View File

@ -1,3 +1,3 @@
package com.appsmith.external.connectionpoolconfig.configurations;
package com.appsmith.external.configurations.connectionpool;
public interface ConnectionPoolConfig extends ConnectionPoolConfigCECompatible {}

View File

@ -1,4 +1,4 @@
package com.appsmith.external.connectionpoolconfig.configurations;
package com.appsmith.external.configurations.connectionpool;
import reactor.core.publisher.Mono;

View File

@ -1,3 +1,3 @@
package com.appsmith.external.connectionpoolconfig.configurations;
package com.appsmith.external.configurations.connectionpool;
public interface ConnectionPoolConfigCECompatible extends ConnectionPoolConfigCE {}

View File

@ -0,0 +1,3 @@
package com.appsmith.external.configurations.git;
public interface GitConfig extends GitConfigCECompatible {}

View File

@ -0,0 +1,7 @@
package com.appsmith.external.configurations.git;
import reactor.core.publisher.Mono;
public interface GitConfigCE {
Mono<Boolean> getIsAtomicPushAllowed();
}

View File

@ -0,0 +1,3 @@
package com.appsmith.external.configurations.git;
public interface GitConfigCECompatible extends GitConfigCE {}

View File

@ -6,10 +6,13 @@ import org.springframework.beans.BeanWrapperImpl;
import org.springframework.beans.PropertyAccessorFactory;
import java.beans.PropertyDescriptor;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
public final class AppsmithBeanUtils {
@ -124,4 +127,16 @@ public final class AppsmithBeanUtils {
return values;
}
public static Stream<Field> getAllFields(Class<?> currentType) {
Set<Class<?>> classes = new HashSet<>();
while (currentType != null) {
classes.add(currentType);
currentType = currentType.getSuperclass();
}
return classes.stream().flatMap(currentClass -> Arrays.stream(currentClass.getDeclaredFields()));
}
}

View File

@ -1,6 +1,6 @@
package com.external.plugins;
import com.appsmith.external.connectionpoolconfig.configurations.ConnectionPoolConfig;
import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfig;
import com.appsmith.external.constants.DataType;
import com.appsmith.external.datatypes.AppsmithType;
import com.appsmith.external.dtos.ExecuteActionDTO;
@ -89,7 +89,6 @@ import static com.appsmith.external.helpers.PluginUtils.getIdenticalColumns;
import static com.appsmith.external.helpers.PluginUtils.getPSParamLabel;
import static com.appsmith.external.helpers.Sizeof.sizeof;
import static com.appsmith.external.helpers.SmartSubstitutionHelper.replaceQuestionMarkWithDollarIndex;
import static com.appsmith.external.models.SSLDetails.AuthType.VERIFY_CA;
import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.BOOL;
import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.DATE;
import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.DECIMAL;

View File

@ -1,6 +1,6 @@
package com.external.plugins;
import com.appsmith.external.connectionpoolconfig.configurations.ConnectionPoolConfig;
import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfig;
import com.appsmith.external.datatypes.ClientDataType;
import com.appsmith.external.dtos.ExecuteActionDTO;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;

View File

@ -1,6 +1,7 @@
package com.appsmith.server.applications.git;
import com.appsmith.external.git.FileInterface;
import com.appsmith.external.helpers.AppsmithBeanUtils;
import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.ApplicationGitReference;
import com.appsmith.external.models.ArtifactGitReference;
@ -40,14 +41,12 @@ import reactor.core.publisher.Mono;
import java.lang.reflect.Field;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static com.appsmith.external.git.constants.GitConstants.NAME_SEPARATOR;
import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties;
@ -141,7 +140,7 @@ public class ApplicationGitFileUtilsCEImpl implements ArtifactGitFileUtilsCE<App
private void setApplicationMetadataInApplicationReference(
ApplicationJson applicationJson, ApplicationGitReference applicationReference) {
// Pass metadata
Iterable<String> keys = getAllFields(applicationJson)
Iterable<String> keys = AppsmithBeanUtils.getAllFields(applicationJson.getClass())
.map(Field::getName)
.filter(name -> !getBlockedMetadataFields().contains(name))
.collect(Collectors.toList());
@ -304,19 +303,6 @@ public class ApplicationGitFileUtilsCEImpl implements ArtifactGitFileUtilsCE<App
applicationReference.setActionBody(resourceMapBody);
}
protected Stream<Field> getAllFields(ApplicationJson applicationJson) {
Class<?> currentType = applicationJson.getClass();
Set<Class<?>> classes = new HashSet<>();
while (currentType != null) {
classes.add(currentType);
currentType = currentType.getSuperclass();
}
return classes.stream().flatMap(currentClass -> Arrays.stream(currentClass.getDeclaredFields()));
}
private void removeUnwantedFieldsFromApplication(Application application) {
// Don't commit application name as while importing we are using the repoName as application name
application.setName(null);

View File

@ -1,6 +1,6 @@
package com.appsmith.server.connectionpoolconfig.configurations;
package com.appsmith.server.configurations.connectionpool;
import com.appsmith.external.connectionpoolconfig.configurations.ConnectionPoolConfigCECompatible;
import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfigCECompatible;
import org.springframework.stereotype.Component;
@Component

View File

@ -1,6 +1,6 @@
package com.appsmith.server.connectionpoolconfig.configurations;
package com.appsmith.server.configurations.connectionpool;
import com.appsmith.external.connectionpoolconfig.configurations.ConnectionPoolConfigCE;
import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfigCE;
import reactor.core.publisher.Mono;
public class ConnectionPoolConfigCEImpl implements ConnectionPoolConfigCE {

View File

@ -1,6 +1,6 @@
package com.appsmith.server.connectionpoolconfig.configurations;
package com.appsmith.server.configurations.connectionpool;
import com.appsmith.external.connectionpoolconfig.configurations.ConnectionPoolConfig;
import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfig;
import org.springframework.stereotype.Component;
@Component

View File

@ -0,0 +1,12 @@
package com.appsmith.server.configurations.git;
import com.appsmith.external.configurations.git.GitConfigCECompatible;
import com.appsmith.server.services.TenantService;
import org.springframework.stereotype.Component;
@Component
public class GitConfigCECompatibleImpl extends GitConfigCEImpl implements GitConfigCECompatible {
public GitConfigCECompatibleImpl(TenantService tenantService) {
super(tenantService);
}
}

View File

@ -0,0 +1,20 @@
package com.appsmith.server.configurations.git;
import com.appsmith.external.configurations.git.GitConfigCE;
import com.appsmith.server.services.TenantService;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;
@RequiredArgsConstructor
@Component
public class GitConfigCEImpl implements GitConfigCE {
private final TenantService tenantService;
@Override
public Mono<Boolean> getIsAtomicPushAllowed() {
return tenantService.getTenantConfiguration().map(tenant -> tenant.getTenantConfiguration()
.getIsAtomicPushAllowed());
}
}

View File

@ -0,0 +1,12 @@
package com.appsmith.server.configurations.git;
import com.appsmith.external.configurations.git.GitConfig;
import com.appsmith.server.services.TenantService;
import org.springframework.stereotype.Component;
@Component
public class GitConfigImpl extends GitConfigCECompatibleImpl implements GitConfig {
public GitConfigImpl(TenantService tenantService) {
super(tenantService);
}
}

View File

@ -53,6 +53,8 @@ public class TenantConfigurationCE {
Boolean isStrongPasswordPolicyEnabled;
private Boolean isAtomicPushAllowed;
public void addThirdPartyAuth(String auth) {
if (thirdPartyAuths == null) {
thirdPartyAuths = new ArrayList<>();
@ -77,6 +79,7 @@ public class TenantConfigurationCE {
featuresWithPendingMigration = tenantConfiguration.getFeaturesWithPendingMigration();
migrationStatus = tenantConfiguration.getMigrationStatus();
isStrongPasswordPolicyEnabled = tenantConfiguration.getIsStrongPasswordPolicyEnabled();
isAtomicPushAllowed = tenantConfiguration.getIsAtomicPushAllowed();
}
public Boolean isEmailVerificationEnabled() {

View File

@ -0,0 +1,48 @@
package com.appsmith.server.migrations.db.ce;
import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.TenantConfiguration;
import io.mongock.api.annotations.ChangeUnit;
import io.mongock.api.annotations.Execution;
import io.mongock.api.annotations.RollbackExecution;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.mongodb.core.MongoTemplate;
import org.springframework.data.mongodb.core.query.Query;
import java.io.IOException;
import java.util.Objects;
import static org.springframework.data.mongodb.core.query.Criteria.where;
@Slf4j
@ChangeUnit(order = "055", id = "add-is-atomic-push-allowed-env-variable-tenant-configuration")
public class Migration055AddIsAtomicPushAllowedEnvVarToTenantConfiguration {
private final MongoTemplate mongoTemplate;
public Migration055AddIsAtomicPushAllowedEnvVarToTenantConfiguration(MongoTemplate mongoTemplate) {
this.mongoTemplate = mongoTemplate;
}
@RollbackExecution
public void executionRollback() {}
@Execution
public void executeMigration() throws IOException {
Query tenantQuery = new Query();
tenantQuery.addCriteria(where(Tenant.Fields.slug).is("default"));
Tenant defaultTenant = mongoTemplate.findOne(tenantQuery, Tenant.class);
boolean isAtomicPushAllowed = false;
TenantConfiguration defaultTenantConfiguration = new TenantConfiguration();
if (defaultTenant == null) {
throw new IllegalStateException("Default tenant not found");
}
if (Objects.nonNull(defaultTenant.getTenantConfiguration())) {
defaultTenantConfiguration = defaultTenant.getTenantConfiguration();
}
defaultTenantConfiguration.setIsAtomicPushAllowed(isAtomicPushAllowed);
defaultTenant.setTenantConfiguration(defaultTenantConfiguration);
mongoTemplate.save(defaultTenant);
}
}

View File

@ -1,6 +1,7 @@
package com.appsmith.server.solutions.ce;
import com.appsmith.external.constants.AnalyticsEvents;
import com.appsmith.external.helpers.AppsmithBeanUtils;
import com.appsmith.server.configurations.CommonConfig;
import com.appsmith.server.configurations.EmailConfig;
import com.appsmith.server.configurations.GoogleRecaptchaConfig;
@ -32,6 +33,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.mail.MessagingException;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.beanutils.ConvertUtils;
import org.jetbrains.annotations.NotNull;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.buffer.DataBufferUtils;
@ -63,7 +65,6 @@ import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@ -296,8 +297,7 @@ public class EnvManagerCEImpl implements EnvManagerCE {
* @return
*/
private Set<String> allowedTenantConfiguration() {
Field[] fields = TenantConfiguration.class.getDeclaredFields();
return Arrays.stream(fields)
return AppsmithBeanUtils.getAllFields(TenantConfiguration.class)
.map(field -> {
JsonProperty jsonProperty = field.getDeclaredAnnotation(JsonProperty.class);
return jsonProperty == null ? field.getName() : jsonProperty.value();
@ -314,20 +314,30 @@ public class EnvManagerCEImpl implements EnvManagerCE {
* @param value
*/
private void setConfigurationByKey(TenantConfiguration tenantConfiguration, String key, String value) {
Field[] fields = tenantConfiguration.getClass().getDeclaredFields();
for (Field field : fields) {
Stream<Field> fieldStream = AppsmithBeanUtils.getAllFields(TenantConfiguration.class);
fieldStream.forEach(field -> {
JsonProperty jsonProperty = field.getDeclaredAnnotation(JsonProperty.class);
if (jsonProperty != null && jsonProperty.value().equals(key)) {
try {
field.setAccessible(true);
field.set(tenantConfiguration, value);
Object typedValue = ConvertUtils.convert(value, field.getType());
field.set(tenantConfiguration, typedValue);
} catch (IllegalAccessException e) {
// Catch the error, log it and then do nothing.
log.error(
"Got error while parsing the JSON annotations from TenantConfiguration class. Cause: ", e);
}
} else if (field.getName().equals(key)) {
try {
field.setAccessible(true);
Object typedValue = ConvertUtils.convert(value, field.getType());
field.set(tenantConfiguration, typedValue);
} catch (IllegalAccessException e) {
// Catch the error, log it and then do nothing.
log.error("Got error while attempting to save property to TenantConfiguration class. Cause: ", e);
}
}
}
});
}
private Mono<Tenant> updateTenantConfiguration(String tenantId, Map<String, String> changes) {

View File

@ -1,6 +1,6 @@
package com.appsmith.server.connectionpoolconfig.configurations;
import com.appsmith.external.connectionpoolconfig.configurations.ConnectionPoolConfig;
import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfig;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;