Make optimize_permission_filters an --experimental-features flag

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3589
GitOrigin-RevId: 5e102e5dd348e170ebe10f04ff9598fdc4167522

CHANGELOG.md

@@ -14,8 +14,9 @@ is expressly experimental, because of the security-sensitive nature of the
 transformation that it applies. You should scrutinize the optimized SQL
 generated by this feature before using it in production.
 
-The optimization can be enabled using the `--optimize-permission-filters` flag
-or the `HASURA_GRAPHQL_OPTIMIZE_PERMISSION_FILTERS` environment variable.
+The optimization can be enabled using the
+`--experimental-features=optimize_permission_filters` flag or the
+`HASURA_GRAPHQL_EXPERIMENTAL_FEATURES` environment variable.
 
 ### Bug fixes and improvements
 (Add entries below in the order of server, console, cli, docs, others)

server/src-lib/Hasura/App.hs

@@ -369,7 +369,8 @@ initialiseServeCtx env GlobalCtx {..} so@ServeOptions {..} = do
                   }
               sourceConnInfo = PostgresSourceConnInfo dbUrlConf (Just connSettings) (Q.cpAllowPrepare soConnParams) soTxIso Nothing
            in PostgresConnConfiguration sourceConnInfo Nothing
-      sqlGenCtx = SQLGenCtx soStringifyNum soDangerousBooleanCollapse soOptimizePermissionFilters
+      optimizePermissionFilters = EFOptimizePermissionFilters `elem` soExperimentalFeatures
+      sqlGenCtx = SQLGenCtx soStringifyNum soDangerousBooleanCollapse optimizePermissionFilters
 
   let serverConfigCtx =
         ServerConfigCtx
@@ -657,7 +658,8 @@ mkHGEServer setupHook env ServeOptions {..} ServeCtx {..} initTime postPollHook
   -- NOTE: be sure to compile WITHOUT code coverage, for this to work properly.
   liftIO disableAssertNF
 
-  let sqlGenCtx = SQLGenCtx soStringifyNum soDangerousBooleanCollapse soOptimizePermissionFilters
+  let optimizePermissionFilters = EFOptimizePermissionFilters `elem` soExperimentalFeatures
+      sqlGenCtx = SQLGenCtx soStringifyNum soDangerousBooleanCollapse optimizePermissionFilters
       Loggers loggerCtx logger _ = _scLoggers
   --SchemaSyncCtx{..} = _scSchemaSyncCtx
 

server/src-lib/Hasura/Server/Init.hs

@@ -266,9 +266,6 @@ mkServeOptions rso = do
     WSConnectionInitTimeout . fromIntegral . fromMaybe 3
       <$> withEnv (rsoWebSocketConnectionInitTimeout rso) (fst webSocketConnectionInitTimeoutEnv)
 
-  optimizePermissionFilters <-
-    fromMaybe False <$> withEnv (rsoOptimizePermissionFilters rso) (fst optimizePermissionFiltersEnv)
-
   pure $
     ServeOptions
       port
@@ -308,7 +305,6 @@ mkServeOptions rso = do
       webSocketConnectionInitTimeout
       EventingEnabled
       ReadOnlyModeDisabled
-      optimizePermissionFilters
   where
     defaultAsyncActionsFetchInterval = Interval 1000 -- 1000 Milliseconds or 1 Second
     defaultSchemaPollInterval = Interval 1000 -- 1000 Milliseconds or 1 Second
@@ -657,7 +653,7 @@ wsReadCookieEnv =
     "Read cookie on WebSocket initial handshake, even when CORS is disabled."
       ++ " This can be a potential security flaw! Please make sure you know "
       ++ "what you're doing."
-      ++ "This configuration is only applicable when CORS is disabled."
+      ++ " This configuration is only applicable when CORS is disabled."
   )
 
 stringifyNumEnv :: (String, String)
@@ -690,7 +686,10 @@ enabledAPIsEnv =
 experimentalFeaturesEnv :: (String, String)
 experimentalFeaturesEnv =
   ( "HASURA_GRAPHQL_EXPERIMENTAL_FEATURES",
-    "Comma separated list of experimental features. (all: inherited_roles)"
+    "Comma separated list of experimental features. (all: inherited_roles,optimize_permission_filters). "
+      <> "optimize_permission_filters: Use experimental SQL optimization"
+      <> "transformations for permission filters. "
+      <> "inherited_roles: ignored; inherited roles cannot be switched off"
   )
 
 gracefulShutdownEnv :: (String, String)
@@ -1430,7 +1429,6 @@ serveOptionsParser =
     <*> parseEventsFetchBatchSize
     <*> parseGracefulShutdownTimeout
     <*> parseWebSocketConnectionInitTimeout
-    <*> parseOptimizePermissionFilters
 
 -- | This implements the mapping between application versions
 -- and catalog schema versions.
@@ -1513,18 +1511,3 @@ parseWebSocketConnectionInitTimeout =
       ( long "websocket-connection-init-timeout"
           <> help (snd webSocketConnectionInitTimeoutEnv)
       )
-
-optimizePermissionFiltersEnv :: (String, String)
-optimizePermissionFiltersEnv =
-  ( "HASURA_GRAPHQL_OPTIMIZE_PERMISSION_FILTERS",
-    "Use experimental SQL optimization transformations for permission filters"
-  )
-
-parseOptimizePermissionFilters :: Parser (Maybe Bool)
-parseOptimizePermissionFilters =
-  optional $
-    option
-      (eitherReader parseStrAsBool)
-      ( long "optimize-permission-filters"
-          <> help (snd optimizePermissionFiltersEnv)
-      )

server/src-lib/Hasura/Server/Init/Config.hs

@@ -151,8 +151,7 @@ data RawServeOptions impl = RawServeOptions
     rsoExperimentalFeatures :: Maybe [ExperimentalFeature],
     rsoEventsFetchBatchSize :: Maybe NonNegativeInt,
     rsoGracefulShutdownTimeout :: Maybe Seconds,
-    rsoWebSocketConnectionInitTimeout :: Maybe Int,
-    rsoOptimizePermissionFilters :: Maybe Bool
+    rsoWebSocketConnectionInitTimeout :: Maybe Int
   }
 
 -- | @'ResponseInternalErrorsConfig' represents the encoding of the internal
@@ -223,8 +222,7 @@ data ServeOptions impl = ServeOptions
     soGracefulShutdownTimeout :: Seconds,
     soWebsocketConnectionInitTimeout :: WSConnectionInitTimeout,
     soEventingMode :: EventingMode,
-    soReadOnlyMode :: ReadOnlyMode,
-    soOptimizePermissionFilters :: Bool
+    soReadOnlyMode :: ReadOnlyMode
   }
 
 data DowngradeOptions = DowngradeOptions

server/src-lib/Hasura/Server/Types.hs

@@ -46,22 +46,21 @@ newtype InstanceId = InstanceId {getInstanceId :: Text}
 
 data ExperimentalFeature
   = EFInheritedRoles
+  | EFOptimizePermissionFilters
   deriving (Show, Eq, Generic)
 
 instance Hashable ExperimentalFeature
 
--- TODO: when there are more than one constuctors in `ExperimentalFeature`, we should
--- auto derive the JSON instances. Doing it with a single data constructor messes
--- up the JSON instances which is why it's manually implemented here
-
 instance FromJSON ExperimentalFeature where
   parseJSON = withText "ExperimentalFeature" $ \case
     "inherited_roles" -> pure EFInheritedRoles
-    _ -> fail "ExperimentalFeature can only be one of these value: inherited_roles "
+    "optimize_permission_filters" -> pure EFOptimizePermissionFilters
+    _ -> fail "ExperimentalFeature can only be one of these value: inherited_roles, optimize_permission_filters"
 
 instance ToJSON ExperimentalFeature where
   toJSON = \case
     EFInheritedRoles -> "inherited_roles"
+    EFOptimizePermissionFilters -> "optimize_permission_filters"
 
 data MaintenanceMode = MaintenanceModeEnabled | MaintenanceModeDisabled
   deriving (Show, Eq)

server/tests-hspec/Harness/Constants.hs

@@ -225,8 +225,7 @@ serveOptions =
       soGracefulShutdownTimeout = 0, -- Don't wait to shutdown.
       soWebsocketConnectionInitTimeout = defaultWSConnectionInitTimeout,
       soEventingMode = EventingEnabled,
-      soReadOnlyMode = ReadOnlyModeDisabled,
-      soOptimizePermissionFilters = False
+      soReadOnlyMode = ReadOnlyModeDisabled
     }
 
 -- | Use the below to show messages.

server/tests/integration/Hasura/Test/App.hs

@@ -347,6 +347,5 @@ defaultRawServeOptions =
       rsoExperimentalFeatures = Nothing,
       rsoEventsFetchBatchSize = Nothing,
       rsoGracefulShutdownTimeout = Nothing,
-      rsoWebSocketConnectionInitTimeout = Nothing,
-      rsoOptimizePermissionFilters = Nothing
+      rsoWebSocketConnectionInitTimeout = Nothing
     }