Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support of new identity: code, tests and CI #252

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Ecosystem engineering are default owners for everything in the repo.
* @firebolt-db/ecosystem-engineering
47 changes: 34 additions & 13 deletions .github/workflows/integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ on:
description: 'Database - a new one will be created if not provided'
required: false
default: ''
engine:
description: 'Engine - a new one will be created if not provided'
required: false
account:
description: 'Account'
required: true
default: 'developer'
environment:
description: 'Environment to run the tests against'
type: choice
Expand All @@ -21,6 +28,13 @@ jobs:
runs-on: ubuntu-latest

steps:
- name: Validate database and engine
if: ${{ (github.event.inputs.database == '') != (github.event.inputs.engine == '') }}
uses: actions/github-script@v3
with:
script: |
core.setFailed("Database and Engine parameters should be provided simultaneously")

- name: "Foresight: Collect Workflow Telemetry"
uses: runforesight/foresight-workflow-kit-action@v1
if: ${{ always() }}
Expand All @@ -35,26 +49,24 @@ jobs:
- name: Determine env variables
run: |
if [ "${{ github.event.inputs.environment }}" == 'staging' ]; then
echo "USERNAME=${{ secrets.FIREBOLT_USERNAME_STAGING }}" >> "$GITHUB_ENV"
echo "PASSWORD=${{ secrets.FIREBOLT_PASSWORD_STAGING }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_ID=${{ secrets.SERVICE_ACCOUNT_ID_STAGING }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_SECRET=${{ secrets.SERVICE_ACCOUNT_SECRET_STAGING }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_ID=${{ secrets.FIREBOLT_CLIENT_ID_STG_NEW_IDN }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_SECRET=${{ secrets.FIREBOLT_CLIENT_SECRET_STG_NEW_IDN }}" >> "$GITHUB_ENV"
else
echo "USERNAME=${{ secrets.FIREBOLT_USERNAME_DEV }}" >> "$GITHUB_ENV"
echo "PASSWORD=${{ secrets.FIREBOLT_PASSWORD_DEV }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_ID=${{ secrets.SERVICE_ACCOUNT_ID_DEV }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_SECRET=${{ secrets.SERVICE_ACCOUNT_SECRET_DEV }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_ID=${{ secrets.FIREBOLT_CLIENT_ID_NEW_IDN }}" >> "$GITHUB_ENV"
echo "SERVICE_ACCOUNT_SECRET=${{ secrets.FIREBOLT_CLIENT_SECRET_NEW_IDN }}" >> "$GITHUB_ENV"
fi

- name: Setup database and engine
id: setup
if: ${{ github.event.inputs.database == '' }}
uses: firebolt-db/integration-testing-setup@v1
uses: firebolt-db/integration-testing-setup@v2
with:
firebolt-username: ${{ env.USERNAME }}
firebolt-password: ${{ env.PASSWORD }}
firebolt-client-id: ${{ env.SERVICE_ACCOUNT_ID }}
firebolt-client-secret: ${{ env.SERVICE_ACCOUNT_SECRET }}
account: ${{ github.event.inputs.account }}
api-endpoint: "api.${{ github.event.inputs.environment }}.firebolt.io"
region: "us-east-1"
instance-type: "B2"
engine-scale: "1"

- name: Determine database name
id: find-database-name
Expand All @@ -65,8 +77,17 @@ jobs:
echo "database_name=${{ steps.setup.outputs.database_name }}" >> $GITHUB_OUTPUT
fi

- name: Determine engine name
id: find-engine-name
run: |
if ! [[ -z "${{ github.event.inputs.engine }}" ]]; then
echo "engine_name=${{ github.event.inputs.engine }}" >> $GITHUB_OUTPUT
else
echo "engine_name=${{ steps.setup.outputs.engine_name }}" >> $GITHUB_OUTPUT
fi

- name: Run integration tests
run: ./gradlew integrationTest -Ddb=${{ steps.find-database-name.outputs.database_name }} -Dapi=api.${{ github.event.inputs.environment }}.firebolt.io -Dpassword="${{ env.SERVICE_ACCOUNT_SECRET }}" -Duser="${{ env.SERVICE_ACCOUNT_ID }}"
run: ./gradlew integrationTest -Ddb=${{ steps.find-database-name.outputs.database_name }} -Denv=${{ github.event.inputs.environment }} -Dclient_secret="${{ env.SERVICE_ACCOUNT_SECRET }}" -Dclient_id="${{ env.SERVICE_ACCOUNT_ID }}" -Daccount="${{ github.event.inputs.account }}" -Dengine="${{ steps.find-engine-name.outputs.engine_name }}"

- name: "Foresight: Analyze Test Results"
uses: runforesight/foresight-test-kit-action@v1
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
version=2.4.6-SNAPSHOT
version=3.0.0
jdbcVersion=4.3
89 changes: 76 additions & 13 deletions src/integrationTest/java/integration/ConnectionInfo.java
Original file line number Diff line number Diff line change
@@ -1,29 +1,92 @@
package integration;

import lombok.Value;

import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;

import static java.lang.String.format;
import static java.lang.System.getProperty;
import static java.util.stream.Collectors.joining;

@Value
public class ConnectionInfo {
private static ConnectionInfo INSTANCE;
String password;
String user;
String api;
String database;
private static final String JDBC_URL_PREFIX = "jdbc:firebolt:";
private static volatile ConnectionInfo INSTANCE;
// principal and secret are used here instead of client_id and client_secret respectively as more common term also used in java security API.
private final String principal;
private final String secret;
private final String env;
private final String database;
private final String account;
private final String engine;

private ConnectionInfo() {
password = Optional.ofNullable(System.getProperty("password")).map(p -> p.replace("\"", "")).orElse(null);
user = Optional.ofNullable(System.getProperty("user")).map(u -> u.replace("\"", "")).orElse(null);
api = System.getProperty("api");
database = System.getProperty("db");
this(
getTrimmedProperty("client_id", "user"),
getTrimmedProperty("client_secret", "password"),
getProperty("env"),
getProperty("db"),
getProperty("account"),
getProperty("engine")
);
}

public ConnectionInfo(String principal, String secret, String env, String database, String account, String engine) {
this.principal = principal;
this.secret = secret;
this.env = env;
this.database = database;
this.account = account;
this.engine = engine;
}

public static ConnectionInfo getInstance() {
if (INSTANCE == null) {
INSTANCE = new ConnectionInfo();
synchronized (ConnectionInfo.class) {
if (INSTANCE == null) {
INSTANCE = new ConnectionInfo();
}
}
}
return INSTANCE;
}

private static String getTrimmedProperty(String name, String alias) {
return Optional.ofNullable(getProperty(name, getProperty(alias))).map(u -> u.replace("\"", "")).orElse(null);
}

public String getPrincipal() {
return principal;
}

public String getSecret() {
return secret;
}

public String getEnv() {
return env;
}

public String getDatabase() {
return database;
}

public String getAccount() {
return account;
}

public String getEngine() {
return engine;
}

public String toJdbcUrl() {
String params = Stream.of(param("env", env), param("engine", engine), param("account", account)).filter(Objects::nonNull).collect(joining("&"));
if (!params.isEmpty()) {
params = "?" + params;
}
return JDBC_URL_PREFIX + database + params;
}

private String param(String name, String value) {
return value == null ? null : format("%s=%s", name, value);
}
}
36 changes: 19 additions & 17 deletions src/integrationTest/java/integration/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,36 @@
import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Optional;

import static org.junit.jupiter.api.Assertions.assertNotNull;

@CustomLog
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public abstract class IntegrationTest {

private static final String JDBC_URL_PREFIX = "jdbc:firebolt:";

protected Connection createLocalConnection(String queryParams) throws SQLException {
return DriverManager.getConnection(
"jdbc:firebolt://localhost" + "/" + integration.ConnectionInfo.getInstance().getDatabase()
+ queryParams,
integration.ConnectionInfo.getInstance().getUser(),
integration.ConnectionInfo.getInstance().getPassword());
JDBC_URL_PREFIX + integration.ConnectionInfo.getInstance().getDatabase()
+ queryParams + "&host=localhost" + getAccountParam(),
integration.ConnectionInfo.getInstance().getPrincipal(),
integration.ConnectionInfo.getInstance().getSecret());
}

protected Connection createConnection() throws SQLException {
return DriverManager.getConnection(
"jdbc:firebolt://" + integration.ConnectionInfo.getInstance().getApi() + "/"
+ integration.ConnectionInfo.getInstance().getDatabase(),
integration.ConnectionInfo.getInstance().getUser(),
integration.ConnectionInfo.getInstance().getPassword());
return DriverManager.getConnection(integration.ConnectionInfo.getInstance().toJdbcUrl(),
integration.ConnectionInfo.getInstance().getPrincipal(),
integration.ConnectionInfo.getInstance().getSecret());
}

protected Connection createConnection(String engine) throws SQLException {
return DriverManager.getConnection(
"jdbc:firebolt://" + integration.ConnectionInfo.getInstance().getApi() + "/"
+ integration.ConnectionInfo.getInstance().getDatabase()
+ Optional.ofNullable(engine).map(e -> "?engine=" + e).orElse(""),
integration.ConnectionInfo.getInstance().getUser(),
integration.ConnectionInfo.getInstance().getPassword());
ConnectionInfo current = integration.ConnectionInfo.getInstance();
ConnectionInfo updated = new ConnectionInfo(current.getPrincipal(), current.getSecret(),
current.getEnv(), current.getDatabase(), current.getAccount(), engine);
return DriverManager.getConnection(updated.toJdbcUrl(),
integration.ConnectionInfo.getInstance().getPrincipal(),
integration.ConnectionInfo.getInstance().getSecret());
}

protected void setParam(Connection connection, String name, String value) throws SQLException {
Expand All @@ -53,7 +52,7 @@ protected void setParam(Connection connection, String name, String value) throws

@SneakyThrows
protected void executeStatementFromFile(String path) {
executeStatementFromFile(path, null);
executeStatementFromFile(path, integration.ConnectionInfo.getInstance().getEngine());
}

@SneakyThrows
Expand All @@ -71,4 +70,7 @@ protected void removeExistingClient() throws NoSuchFieldException, IllegalAccess
field.set(null, null);
}

private String getAccountParam() {
return "&account=" + integration.ConnectionInfo.getInstance().getAccount();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package integration;

import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;

import java.io.IOException;

import static java.lang.String.format;
import static org.junit.jupiter.api.Assertions.assertEquals;

public abstract class MockWebServerAwareIntegrationTest extends IntegrationTest {
protected MockWebServer mockBackEnd;
private final int INIT_STATEMENTS_COUNT = 1; // The statement that validates that DB exists.

@BeforeEach
void setUp() throws IOException {
mockBackEnd = new MockWebServer();
mockBackEnd.start();
mockBackEnd.enqueue(new MockResponse().setResponseCode(200).setBody(format("database_name%nstring%n%s", System.getProperty("db"))));
}

@AfterEach
void tearDown() throws IOException {
mockBackEnd.close();
}


protected void assertMockBackendRequestsCount(int expected) {
assertEquals(INIT_STATEMENTS_COUNT + expected, mockBackEnd.getRequestCount());
}
}
31 changes: 31 additions & 0 deletions src/integrationTest/java/integration/tests/ConnectionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package integration.tests;

import integration.ConnectionInfo;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;

import static java.lang.String.format;
import static org.junit.jupiter.api.Assertions.assertNotNull;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public class ConnectionTest {
/**
* This test connects to specific engine with additional property {@code use_standard_sql} supported by user engine but
* not supported by system engine used here to retrieve the data of user engine.
* The test is needed because there were create connection to system engine by copying all given connection properties
* while additional (custom) parameters should be ignored.
* @throws SQLException if something went wrong
*/
@Test
void connectionWithAdditionalProperties() throws SQLException {
ConnectionInfo params = integration.ConnectionInfo.getInstance();
String url = format("jdbc:firebolt:%s?env=%s&engine=%s&account=%s&use_standard_sql=1", params.getDatabase(), params.getEnv(), params.getEngine(), params.getAccount());
try(Connection connection = DriverManager.getConnection(url, params.getPrincipal(), params.getSecret())) {
assertNotNull(connection);
}
}
}
28 changes: 28 additions & 0 deletions src/integrationTest/java/integration/tests/MissingDataTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package integration.tests;

import integration.ConnectionInfo;
import integration.IntegrationTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public class MissingDataTest extends IntegrationTest {
@Test
void missingAccount() throws SQLException {
ConnectionInfo current = integration.ConnectionInfo.getInstance();
try (Connection good = DriverManager.getConnection(current.toJdbcUrl(), current.getPrincipal(), current.getSecret())) {
assertNotNull(good);
}

ConnectionInfo noAccount = new ConnectionInfo(current.getPrincipal(), current.getSecret(),
current.getEnv(), current.getDatabase(), null, current.getEngine());
assertThrows(SQLException.class, () -> DriverManager.getConnection(noAccount.toJdbcUrl(), noAccount.getPrincipal(), noAccount.getSecret()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package integration.tests;

import integration.IntegrationTest;
import org.junit.jupiter.api.Test;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class NumericTypesTest extends IntegrationTest {
@Test
void shouldHaveCorrectInfo() throws SQLException {
try (Connection connection = this.createConnection(null);
Statement statement = connection.createStatement();
ResultSet resultSet = statement.executeQuery("SELECT 3::decimal")) {
resultSet.next();
assertEquals(9, resultSet.getMetaData().getScale(1));
assertEquals(38, resultSet.getMetaData().getPrecision(1));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ void shouldFailSQLInjectionAttempt() throws SQLException {
private QueryResult createExpectedResult(List<List<?>> expectedRows) {
return QueryResult.builder().databaseName(ConnectionInfo.getInstance().getDatabase())
.tableName("prepared_statement_test")
.columns(Arrays.asList(QueryResult.Column.builder().name("sales").type(FireboltDataType.BIG_INT).build(),
QueryResult.Column.builder().name("make").type(FireboltDataType.TEXT).build()))
.columns(
Arrays.asList(QueryResult.Column.builder().name("sales").type(FireboltDataType.BIG_INT).build(),
QueryResult.Column.builder().name("make").type(FireboltDataType.TEXT).build()))
.rows(expectedRows).build();

}
Expand Down
Loading
Loading