Added nginx -t test and revert if the config is invalid. Fixed https://github.com/caprover/caprover/issues/660

src/api/ApiStatusCodes.ts

@@ -48,6 +48,7 @@ class ApiStatusCodes {
     static readonly STATUS_PASSWORD_BACK_OFF = 1113
     static readonly STATUS_ERROR_OTP_REQUIRED = 1114
     static readonly STATUS_ERROR_PRO_API_KEY_INVALIDATED = 1115
+    static readonly STATUS_ERROR_NGINX_VALIDATION_FAILED = 1116
 }
 
 export default ApiStatusCodes

src/user/ServiceManager.ts

@@ -613,6 +613,8 @@ class ServiceManager {
 
         let serviceName: string
 
+        let existingAppDefinition: IAppDef
+
         const checkIfNodeIdExists = function (nodeIdToCheck: string) {
             return dockerApi.getNodesInfo().then(function (nodeInfo) {
                 for (let i = 0; i < nodeInfo.length; i++) {
@@ -704,6 +706,11 @@ class ServiceManager {
                 }
             })
             .then(function () {
+                return dataStore.getAppsDataStore().getAppDefinition(appName)
+            })
+            .then(function (appDef) {
+                existingAppDefinition = appDef
+
                 return dataStore
                     .getAppsDataStore()
                     .updateAppDefinitionInDb(
@@ -733,8 +740,61 @@ class ServiceManager {
             .then(function () {
                 return self.ensureServiceInitedAndUpdated(appName)
             })
-            .then(function () {
-                return self.reloadLoadBalancer()
+            .catch(function (error) {
+                if (
+                    error &&
+                    error.captainErrorType ===
+                        ApiStatusCodes.STATUS_ERROR_NGINX_VALIDATION_FAILED
+                ) {
+                    // Revert back to the old definition because the nginx config is invalid
+                    if (existingAppDefinition) {
+                        Logger.d(
+                            `nginx validation failed, reverting configs for: ${appName}`
+                        )
+                        return dataStore
+                            .getAppsDataStore()
+                            .updateAppDefinitionInDb(
+                                appName,
+                                existingAppDefinition.description,
+                                existingAppDefinition.instanceCount,
+                                existingAppDefinition.captainDefinitionRelativeFilePath,
+                                existingAppDefinition.envVars,
+                                existingAppDefinition.volumes,
+                                existingAppDefinition.tags || [],
+                                existingAppDefinition.nodeId || '',
+                                existingAppDefinition.notExposeAsWebApp,
+                                existingAppDefinition.containerHttpPort || 80,
+                                existingAppDefinition.httpAuth,
+                                existingAppDefinition.forceSsl,
+                                existingAppDefinition.ports,
+                                existingAppDefinition.appPushWebhook
+                                    ?.repoInfo || {
+                                    repo: '',
+                                    branch: '',
+                                    user: '',
+                                    password: '',
+                                },
+                                self.authenticator,
+                                existingAppDefinition.customNginxConfig || '',
+                                existingAppDefinition.redirectDomain || '',
+                                existingAppDefinition.preDeployFunction || '',
+                                existingAppDefinition.serviceUpdateOverride ||
+                                    '',
+                                existingAppDefinition.websocketSupport,
+                                existingAppDefinition.appDeployTokenConfig || {
+                                    enabled: false,
+                                }
+                            )
+                            .then(function () {
+                                self.reloadLoadBalancer()
+                            })
+                            .then(function () {
+                                throw error
+                            })
+                    }
+                }
+
+                throw error
             })
     }
 

src/user/system/CaptainManager.ts

@@ -780,12 +780,47 @@ class CaptainManager {
 
     setNginxConfig(baseConfig: string, captainConfig: string) {
         const self = this
+        let existingConfigs: {
+            baseConfig: {
+                byDefault: string
+                customValue: any
+            }
+            captainConfig: {
+                byDefault: string
+                customValue: any
+            }
+        }
         return Promise.resolve()
             .then(function () {
+                return self.dataStore.getNginxConfig()
+            })
+            .then(function (configs) {
+                existingConfigs = configs
                 return self.dataStore.setNginxConfig(baseConfig, captainConfig)
             })
             .then(function () {
-                self.resetSelf()
+                return self.loadBalancerManager.rePopulateNginxConfigFile()
+            })
+            .catch(function (error) {
+                if (
+                    error &&
+                    error.captainErrorType ===
+                        ApiStatusCodes.STATUS_ERROR_NGINX_VALIDATION_FAILED
+                ) {
+                    Logger.d(
+                        "Nginx validation failed. Reverting changes in system's nginx configs..."
+                    )
+                    self.dataStore
+                        .setNginxConfig(
+                            existingConfigs.baseConfig.customValue,
+                            existingConfigs.captainConfig.customValue
+                        )
+
+                        .then(function () {
+                            return self.loadBalancerManager.rePopulateNginxConfigFile()
+                        })
+                }
+                throw error
             })
     }
 

src/user/system/LoadBalancerManager.ts

@@ -183,6 +183,34 @@ class LoadBalancerManager {
             .then(function () {
                 return self.createRootConfFile()
             })
+            .then(function () {
+                return self.dockerApi
+                    .executeCommand(CaptainConstants.nginxServiceName, [
+                        'nginx',
+                        '-t',
+                    ])
+                    .then(function (result) {
+                        // nginx:1.24
+                        // Failed example:
+                        //
+                        // 2024/08/11 22:32:59 [emerg] 28#28: unknown directive "eventsxxx" in /etc/nginx/nginx.conf:8
+                        // nginx: [emerg] unknown directive "eventsxxx" in /etc/nginx/nginx.conf:8
+                        // nginx: configuration file /etc/nginx/nginx.conf test failed
+
+                        // Successful example:
+                        //
+                        // nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
+                        // nginx: configuration file /etc/nginx/nginx.conf test is successful
+
+                        if (result.indexOf('test is successful') < 0) {
+                            throw ApiStatusCodes.createError(
+                                ApiStatusCodes.STATUS_ERROR_NGINX_VALIDATION_FAILED,
+                                result
+                            )
+                        }
+                    })
+            })
+
             .then(function () {
                 Logger.d('sendReloadSignal...')
                 return self.dockerApi.sendSingleContainerKillHUP(