#155: Improve error handling for extension (#156)

* Increment version, upgrade dependencies * Fix sonar warning about regex * #155: Check if VS already exists before creating it * Adapt extension to shared integration tests * Make check for existing vs case-insensitive * Fix warning * Add changelog entry * Upgrade dependencies * Downgrade to Exasol 8.26.0 java.lang.IllegalStateException: E-ETAJ-12: Failed to ssh to the exasol database. This is required for redirecting a host port. Known mitigations: * Make sure the database is reachable, i.e. port 32773 open on host 'localhost'. * Make sure user 'root' can login to the database using your ssh-key. at com.exasol.exasoltestsetup.SshConnection.createSession(SshConnection.java:53) at com.exasol.exasoltestsetup.SshConnection.<init>(SshConnection.java:31) at com.exasol.exasoltestsetup.testcontainers.ExasolTestcontainerTestSetup.<init>(ExasolTestcontainerTestSetup.java:38) at com.exasol.exasoltestsetup.ExasolTestSetupFactory.getTestSetup(ExasolTestSetupFactory.java:123) at com.exasol.rls.extension.ExtensionIT.setup(ExtensionIT.java:47) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at java.base/java.util.ArrayList.forEach(ArrayList.java:1541) Suppressed: java.lang.NullPointerException at com.exasol.rls.extension.ExtensionIT.teardown(ExtensionIT.java:64) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at java.base/java.util.ArrayList.forEach(ArrayList.java:1541) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1085) ... 1 more Caused by: com.jcraft.jsch.JSchException: Session.connect: java.net.SocketException: Connection reset at com.jcraft.jsch.Session.connect(Session.java:569) at com.jcraft.jsch.Session.connect(Session.java:198) at com.exasol.exasoltestsetup.SshConnection.createSession(SshConnection.java:39) ... 6 more Caused by: java.net.SocketException: Connection reset at java.base/java.net.SocketInputStream.read(SocketInputStream.java:186) at java.base/java.net.SocketInputStream.read(SocketInputStream.java:140) at java.base/java.net.SocketInputStream.read(SocketInputStream.java:200) at com.jcraft.jsch.IO.getByte(IO.java:86) at com.jcraft.jsch.Session.connect(Session.java:275) ... 8 more * Cleanup test * Implement review findings
exasol · May 7, 2024 · 18829eb · 18829eb
1 parent 5ae6440
commit 18829eb
Show file tree

Hide file tree

Showing 20 changed files with 410 additions and 480 deletions.
diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml
diff --git a/.project-keeper.yml b/.project-keeper.yml
@@ -12,14 +12,12 @@ sources:
       - dist/row-level-security-extension.js
 version:
   fromSource: pom.xml
-linkReplacements:
-  - "LICENSE-exasol-jdbc.txt|https://docs.exasol.com/db/latest/connect_exasol/drivers/jdbc.htm"
 build:
   runnerOs: ubuntu-22.04
   freeDiskSpace: true
   exasolDbVersions:
-    - "8.24.0"
-    - "7.1.26"
+    - 8.26.0
+    - 7.1.26
   workflows:
     - name: ci-build.yml
       stepCustomizations:

diff --git a/dependencies.md b/dependencies.md
diff --git a/doc/changes/changelog.md b/doc/changes/changelog.md
diff --git a/doc/changes/changes_1.5.5.md b/doc/changes/changes_1.5.5.md
@@ -0,0 +1,32 @@
+# Row Level Security lua 1.5.5, released 2024-05-07
+
+Code name: Improve error handling for extension
+
+## Summary
+
+This release improves error handling when creating a new Virtual Schema using the extension: the extension now checks if a schema with the same name exists and returns a helpful error message. This check is case-insensitive to be consistent with other virtual schemas.
+
+## Bugfix
+
+* #155: Improved error handling for creating Virtual Schema using the extension
+
+## Dependency Updates
+
+### Exasol Row Level Security (Lua)
+
+#### Test Dependency Updates
+
+* Updated `com.exasol:exasol-testcontainers:7.0.1` to `7.1.0`
+* Updated `com.exasol:extension-manager-integration-test-java:0.5.9` to `0.5.11`
+* Updated `org.slf4j:slf4j-jdk14:2.0.12` to `2.0.13`
+
+### Extension
+
+#### Compile Dependency Updates
+
+* Updated `@exasol/extension-manager-interface:0.4.1` to `0.4.2`
+
+#### Development Dependency Updates
+
+* Updated `typescript-eslint:^7.6.0` to `^7.8.0`
+* Updated `typescript:^5.4.4` to `^5.4.5`
diff --git a/extension/package-lock.json b/extension/package-lock.json
diff --git a/extension/package.json b/extension/package.json
@@ -15,7 +15,7 @@
         "test-watch": "jest --watch"
     },
     "dependencies": {
-        "@exasol/extension-manager-interface": "0.4.1"
+        "@exasol/extension-manager-interface": "0.4.2"
     },
     "devDependencies": {
         "@types/jest": "^29.5.12",
@@ -24,7 +24,7 @@
         "jest": "29.7.0",
         "ts-jest": "^29.1.2",
         "ts-node": "^10.9.2",
-        "typescript": "^5.4.4",
-        "typescript-eslint": "^7.6.0"
+        "typescript": "^5.4.5",
+        "typescript-eslint": "^7.8.0"
     }
 }
diff --git a/extension/src/addInstance.ts b/extension/src/addInstance.ts
@@ -1,6 +1,7 @@
 import { BadRequestError, Instance, NotFoundError, Parameter, ParameterValues } from "@exasol/extension-manager-interface";
 import { ADAPTER_SCRIPT_NAME, EXTENSION_NAME, convertSchemaNameToInstanceId } from "./common";
 import { ExtendedContext } from "./extension";
+import { findInstances } from "./findInstances";
 import { getAllParameterDefinitions } from "./parameterDefinitions";
 
 interface VirtualSchemaConfig {
@@ -17,6 +18,7 @@ export function addInstance(context: ExtendedContext, version: string, paramValu
         throw new NotFoundError(`Version '${version}' not supported, can only use '${context.version}'.`)
     }
     const config = buildVirtualSchemaConfig(paramValues)
+    verifySchemaDoesNotExist(context, config.virtualSchemaName)
     const createVirtualSchemaStmt = createVirtualSchemaStatement(context.extensionSchemaName, config);
     context.sqlClient.execute(createVirtualSchemaStmt);
     const comment = `Created by Extension Manager for ${EXTENSION_NAME} version ${context.version}`;
@@ -69,3 +71,18 @@ function createVirtualSchemaStatement(adapterSchema: string, config: VirtualSche
     }
     return stmt;
 }
+
+/**
+ * Check if a virtual schema with the given name already exists. This is case-insensitive
+ * because other virtual schemas that use `CONNECTION`s are case-insensitive and this
+ * schema should behave the same way, even if it does not use a connection.
+ * @param context extension context
+ * @param virtualSchemaName name of the virtual schema to check
+ */
+function verifySchemaDoesNotExist(context: ExtendedContext, virtualSchemaName: string) {
+    const existingSchema = findInstances(context)
+        .filter(instance => instance.id.toUpperCase() === virtualSchemaName.toUpperCase());
+    if (existingSchema.length > 0) {
+        throw new BadRequestError(`Virtual Schema '${existingSchema[0].name}' already exists`)
+    }
+}
diff --git a/extension/src/common.ts b/extension/src/common.ts
@@ -11,7 +11,7 @@ function identity(arg: string): string {
 export const convertInstanceIdToSchemaName = identity
 export const convertSchemaNameToInstanceId = identity
 
-const versionCommentRegexp = new RegExp("^\\s*-- RLS Lua version (.*?)\\s*$", "m")
+const versionCommentRegexp = /^\s*-- RLS Lua version (.*?)\s*$/m
 export function extractVersion(scriptText: string): Result<string> {
     const match = versionCommentRegexp.exec(scriptText)
     if (!match) {
@@ -22,4 +22,4 @@ export function extractVersion(scriptText: string): Result<string> {
 
 export function getScriptVersionComment(): string {
     return `-- RLS Lua version ${EXTENSION_DESCRIPTION.version}`
-}
+}
diff --git a/extension/src/extension.test.ts b/extension/src/extension.test.ts
@@ -1,12 +1,12 @@
-import { BadRequestError, ExaMetadata, Installation, Instance, NotFoundError, ParameterValue, PreconditionFailedError, Row } from '@exasol/extension-manager-interface';
+import { BadRequestError, ExaMetadata, Installation, Instance, NotFoundError, ParameterValue, ParameterValues, PreconditionFailedError, Row } from '@exasol/extension-manager-interface';
 import { failureResult, successResult } from '@exasol/extension-manager-interface/dist/base/common';
 import { ExaScriptsRow } from '@exasol/extension-manager-interface/dist/exasolSchema';
 import { describe, expect, it } from '@jest/globals';
 import { ADAPTER_SCRIPT_NAME, EXTENSION_NAME, extractVersion } from './common';
 import { createExtension } from "./extension";
 import { EXTENSION_DESCRIPTION } from './extension-description';
 import { buildCreateScriptCommand } from './install';
-import { createMockContext, getInstalledExtension, script } from './test-utils';
+import { ContextMock, createMockContext, getInstalledExtension, script } from './test-utils';
 
 const currentVersion = EXTENSION_DESCRIPTION.version
 
@@ -81,12 +81,12 @@ describe("Row Level Security Lua", () => {
 
         it("returns single item when script is available", () => {
             const scripts: ExaScriptsRow[] = [script({ name: "RLS_ADAPTER", text: "-- RLS Lua version version" })]
-            expect(findInstallations(scripts)).toStrictEqual([{ name: "schema.RLS_ADAPTER", version: "version" }])
+            expect(findInstallations(scripts)).toStrictEqual([{ name: "Row Level Security Lua", version: "version" }])
         })
 
         it("returns unknown version for invalid script", () => {
             const scripts: ExaScriptsRow[] = [script({ name: "RLS_ADAPTER", text: "invalid" })]
-            expect(findInstallations(scripts)).toStrictEqual([{ name: "schema.RLS_ADAPTER", version: "(unknown)" }])
+            expect(findInstallations(scripts)).toStrictEqual([{ name: "Row Level Security Lua", version: "(unknown)" }])
         })
     })
 
@@ -150,7 +150,11 @@ table.insert(package.searchers,`)
             const actual = createExtension().getInstanceParameters(createMockContext(), currentVersion)
             expect(actual).toHaveLength(6)
             expect(actual[0]).toStrictEqual({
-                id: "virtualSchemaName", name: "Name of the new virtual schema", required: true, type: "string"
+                id: "virtualSchemaName",
+                name: "Virtual Schema name",
+                placeholder: "MY_VIRTUAL_SCHEMA",
+                regex: "[a-zA-Z_]+", required: true, type: "string",
+                "description": "Name for the new virtual schema",
             })
             expect(actual[1]).toStrictEqual({
                 id: "SCHEMA_NAME", name: "Name of the schema for which to apply row-level security", required: true, type: "string"
@@ -159,21 +163,51 @@ table.insert(package.searchers,`)
     })
 
     describe("addInstance()", () => {
+        let contextMock: ContextMock
+        function addInstance(version: string, params: ParameterValues): Instance {
+            return addInstanceSimulateExistingVs(version, params, [])
+        }
+
+        function addInstanceSimulateExistingVs(version: string, params: ParameterValues, sqlQueryRows: Row[]): Instance {
+            contextMock = createMockContext();
+            contextMock.mocks.sqlQuery.mockReturnValue({ columns: [], rows: sqlQueryRows });
+            return createExtension().addInstance(contextMock, version, params)
+        }
+
         it("fails for missing schema name", () => {
-            expect(() => createExtension().addInstance(createMockContext(), currentVersion, { values: [] }))
+            expect(() => addInstance(currentVersion, { values: [] }))
                 .toThrowError(new BadRequestError(`Missing parameter "virtualSchemaName"`))
         })
+
         it("fails for missing base schema name", () => {
-            expect(() => createExtension().addInstance(createMockContext(), currentVersion, { values: [{ name: "virtualSchemaName", value: "new_vs" }] }))
+            expect(() => addInstance(currentVersion, { values: [{ name: "virtualSchemaName", value: "new_vs" }] }))
                 .toThrowError(new BadRequestError(`Missing parameter "SCHEMA_NAME"`))
         })
 
+        describe("checks for existing virtual schema", () => {
+            it("throws error when existing schema has the same name", () => {
+                expect(() => addInstanceSimulateExistingVs(currentVersion, { values: [{ name: "virtualSchemaName", value: "new_vs" }, { name: "SCHEMA_NAME", value: "baseSchema" }] }, [["new_vs"]]))
+                    .toThrowError(new BadRequestError(`Virtual Schema 'new_vs' already exists`))
+            })
+            it("throws error when existing schema has the same case-insensitive name", () => {
+                expect(() => addInstanceSimulateExistingVs(currentVersion, { values: [{ name: "virtualSchemaName", value: "new_vs" }, { name: "SCHEMA_NAME", value: "baseSchema" }] }, [["NEW_vs"]]))
+                    .toThrowError(new BadRequestError(`Virtual Schema 'NEW_vs' already exists`))
+            })
+            it("does not throw error when no schema exists", () => {
+                expect(() => addInstanceSimulateExistingVs(currentVersion, { values: [{ name: "virtualSchemaName", value: "new_vs" }, { name: "SCHEMA_NAME", value: "baseSchema" }] }, []))
+                    .not.toThrow()
+            })
+            it("does not throw error when existing schema has a different name", () => {
+                expect(() => addInstanceSimulateExistingVs(currentVersion, { values: [{ name: "virtualSchemaName", value: "new_vs" }, { name: "SCHEMA_NAME", value: "baseSchema" }] }, [["other_vs"]]))
+                    .not.toThrow()
+            })
+        })
+
         it("executes expected statements", () => {
-            const context = createMockContext();
             const parameters = [{ name: "virtualSchemaName", value: "NEW_RLS_VS" }, { name: "SCHEMA_NAME", value: "baseSchema" }]
-            const instance = createExtension().addInstance(context, currentVersion, { values: parameters });
+            const instance = addInstance(currentVersion, { values: parameters });
             expect(instance.name).toBe("NEW_RLS_VS")
-            const calls = context.mocks.sqlExecute.mock.calls
+            const calls = contextMock.mocks.sqlExecute.mock.calls
             expect(calls.length).toBe(2)
 
             expect(calls[0]).toEqual([`CREATE VIRTUAL SCHEMA "NEW_RLS_VS" USING "ext-schema"."RLS_ADAPTER" WITH SCHEMA_NAME = 'baseSchema'`])
@@ -196,26 +230,24 @@ table.insert(package.searchers,`)
                 },
             ]
             tests.forEach(test => it(test.name, () => {
-                const context = createMockContext();
                 const parameters = [{ name: "virtualSchemaName", value: "NEW_RLS_VS" }, { name: "SCHEMA_NAME", value: "baseSchema" }]
                 test.params.forEach(p => parameters.push(p))
-                const instance = createExtension().addInstance(context, currentVersion, { values: parameters });
+                const instance = addInstance(currentVersion, { values: parameters });
                 expect(instance.name).toBe("NEW_RLS_VS")
-                const calls = context.mocks.sqlExecute.mock.calls
+                const calls = contextMock.mocks.sqlExecute.mock.calls
                 expect(calls.length).toBe(2)
                 expect(calls[0]).toEqual([`CREATE VIRTUAL SCHEMA "NEW_RLS_VS" USING "ext-schema"."RLS_ADAPTER" WITH SCHEMA_NAME = 'baseSchema'` + test.expectedScript])
             }))
         })
 
         it("returns id and name", () => {
-            const context = createMockContext();
             const parameters = [{ name: "virtualSchemaName", value: "NEW_RLS_VS" }, { name: "SCHEMA_NAME", value: "baseSchema" }]
-            const instance = createExtension().addInstance(context, currentVersion, { values: parameters });
+            const instance = addInstance(currentVersion, { values: parameters });
             expect(instance).toStrictEqual({ id: "NEW_RLS_VS", name: "NEW_RLS_VS" })
         })
 
         it("fails for wrong version", () => {
-            expect(() => { createExtension().addInstance(createMockContext(), "wrongVersion", { values: [] }) })
+            expect(() => { addInstance("wrongVersion", { values: [] }) })
                 .toThrow(`Version 'wrongVersion' not supported, can only use '${currentVersion}'.`)
         })
     })
@@ -289,7 +321,7 @@ table.insert(package.searchers,`)
         })
         describe("failure", () => {
             const tests: { name: string; scripts: ExaScriptsRow[], expectedMessage: string }[] = [
-                { name: "no script", scripts: [], expectedMessage: "Adapter script 'RLS_ADAPTER' is not installed" },
+                { name: "no script", scripts: [], expectedMessage: "Not all required scripts are installed: Validation failed: Script 'RLS_ADAPTER' is missing" },
                 {
                     name: "invalid version", scripts: [script({ name: "RLS_ADAPTER", text: "invalid script" })],
                     expectedMessage: `Failed to extract version from adapter script schema.RLS_ADAPTER: version not found in script text 'invalid script'`
@@ -309,4 +341,3 @@ table.insert(package.searchers,`)
         })
     })
 })
-
diff --git a/extension/src/findInstallations.ts b/extension/src/findInstallations.ts
@@ -7,11 +7,10 @@ export function findInstallations(context: ExtendedContext, scripts: ExaScriptsR
     if (!adapterScript) {
         return []
     }
-    const name = `${adapterScript.schema}.${adapterScript.name}`
     const versionResult = extractVersion(adapterScript.text)
     if (versionResult.type === "failure") {
         console.warn(versionResult.message)
     }
     const version = versionResult.type === "success" ? versionResult.result : "(unknown)"
-    return [{ name, version }];
+    return [{ name: "Row Level Security Lua", version }];
 }
diff --git a/extension/src/parameterDefinitions.ts b/extension/src/parameterDefinitions.ts
@@ -16,7 +16,7 @@ const REMOTELOG_LUA_LOG_LEVELS: string[] = ["NONE", "FATAL", "ERROR", "WARN", "I
 const LOG_LEVEL_OPTIONS: SelectOption[] = REMOTELOG_LUA_LOG_LEVELS.map(level => { return { id: level, name: level } })
 
 const allParams: AllParameters = {
-    virtualSchemaName: { id: "virtualSchemaName", name: "Name of the new virtual schema", type: "string", required: true },
+    virtualSchemaName: { id: "virtualSchemaName", name: "Virtual Schema name", type: "string", required: true, regex: "[a-zA-Z_]+", description: "Name for the new virtual schema", placeholder: "MY_VIRTUAL_SCHEMA" },
 
     // Virtual Schema parameters
     schemaName: { id: "SCHEMA_NAME", name: "Name of the schema for which to apply row-level security", type: "string", required: true },
@@ -40,4 +40,4 @@ export function createInstanceParameters(): Parameter[] {
         allParams.debugAddress,
         allParams.logLevel,
     ];
-}
+}
diff --git a/extension/src/upgrade.ts b/extension/src/upgrade.ts
@@ -13,7 +13,7 @@ export function upgrade(context: ExtendedContext): UpgradeResult {
 function getAdapterVersion(context: ExtendedContext): string {
     const adapterScript = context.metadata.getScriptByName(ADAPTER_SCRIPT_NAME)
     if (!adapterScript) {
-        throw new PreconditionFailedError(`Adapter script '${ADAPTER_SCRIPT_NAME}' is not installed`)
+        throw new PreconditionFailedError(`Not all required scripts are installed: Validation failed: Script '${ADAPTER_SCRIPT_NAME}' is missing`)
     }
     const version = extractVersion(adapterScript.text)
     if (version.type === "failure") {

diff --git a/pk_generated_parent.pom b/pk_generated_parent.pom
diff --git a/pom.xml b/pom.xml
@@ -2,7 +2,7 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
     <artifactId>row-level-security-lua</artifactId>
-    <version>1.5.4</version>
+    <version>1.5.5</version>
     <name>Exasol Row Level Security (Lua)</name>
     <description>This projects provides a plug-in to the Exasol database that adds per-row access control.</description>
     <url>https://github.com/exasol/row-level-security-lua/</url>
@@ -25,7 +25,7 @@
         <dependency>
             <groupId>com.exasol</groupId>
             <artifactId>exasol-testcontainers</artifactId>
-            <version>7.0.1</version>
+            <version>7.1.0</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -61,7 +61,7 @@
         <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-jdk14</artifactId>
-            <version>2.0.12</version>
+            <version>2.0.13</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -79,7 +79,7 @@
         <dependency>
             <groupId>com.exasol</groupId>
             <artifactId>extension-manager-integration-test-java</artifactId>
-            <version>0.5.9</version>
+            <version>0.5.11</version>
             <scope>test</scope>
         </dependency>
     </dependencies>
@@ -340,7 +340,7 @@
     <parent>
         <artifactId>row-level-security-lua-generated-parent</artifactId>
         <groupId>com.exasol</groupId>
-        <version>1.5.4</version>
+        <version>1.5.5</version>
         <relativePath>pk_generated_parent.pom</relativePath>
     </parent>
 </project>