-
Notifications
You must be signed in to change notification settings - Fork 53
Update Debezium & Azure Identity libraries #315
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,8 +60,6 @@ spec: | |
- database | ||
- host | ||
- port | ||
- password | ||
- user | ||
- tables | ||
--- | ||
apiVersion: v1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package io.drasi.databases; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import com.azure.identity.DefaultAzureCredentialBuilder; | ||
import com.azure.core.credential.TokenRequestContext; | ||
|
||
public class AzureIdentityTokenFetcher { | ||
private static final Logger log = LoggerFactory.getLogger(AzureIdentityTokenFetcher.class); | ||
|
||
private static final String SCOPE = "https://ossrdbms-aad.database.windows.net/.default"; | ||
|
||
public String getToken() { | ||
|
||
log.info("Fetching Azure AD token"); | ||
|
||
// Fetch Azure AD token | ||
var credential = new DefaultAzureCredentialBuilder().build(); | ||
var tokenContext = new TokenRequestContext().addScopes(SCOPE); | ||
String accessToken = credential.getToken(tokenContext).block().getToken(); | ||
|
||
return accessToken; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,8 @@ | |
import io.drasi.models.NodeMapping; | ||
import io.drasi.models.RelationalGraphMapping; | ||
import io.drasi.source.sdk.Reactivator; | ||
import io.drasi.source.sdk.SourceProxy; | ||
|
||
import org.checkerframework.checker.units.qual.t; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -90,7 +90,7 @@ public String getTablesListConfigName() { | |
@Override | ||
public Configuration createConnectorConfig(Configuration baseConfig) { | ||
var publicationSlotName = "rg_" + baseConfig.getString("name"); | ||
return Configuration.create() | ||
var result = Configuration.create() | ||
// Start with the base configuration. | ||
.with(baseConfig) | ||
// Specify the Postgres connector class. | ||
|
@@ -104,8 +104,22 @@ public Configuration createConnectorConfig(Configuration baseConfig) { | |
// Name of replication slot for streaming changes from the database. Default is debezium. | ||
.with("slot.name", publicationSlotName) | ||
// If started first time, start from beginning, else start from last stored LSN. | ||
.with("snapshot.mode", "no_data") | ||
.build(); | ||
.with("snapshot.mode", "no_data"); | ||
|
||
String identityType = SourceProxy.GetConfigValue("IDENTITY_TYPE"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we reconsider (maybe in a separate PR) adding fields to source-provider instead of relying on undocumented environment variables like Should the SDK provide a better way of achieving this? I worry that we're not using the schema-driven approach properly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have varying degrees of support for abstracting that environment variable across our SDKs, I think the .net version has this. We also do not yet have any documentation on building a source, I think that would be the place to document these. Could you clarify what you mean by "schema-driven approach"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By schema-driven I meant that source-provider defines the schema of the Source Config, so it can drive what fields are supported in the config. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The identity type is a higher level field in the YAML that is applicable to all sources / reactions. The config schema is set for the configuration section, which is specific per component. |
||
if ("MicrosoftEntraWorkloadID".equals(identityType)) { | ||
|
||
var tokenFetcher = new AzureIdentityTokenFetcher(); | ||
var accessToken = tokenFetcher.getToken(); | ||
|
||
result = result | ||
.with("database.password", accessToken) | ||
.with("database.sslmode", "require"); | ||
|
||
log.info("Using Azure Identity PostgreSQL Driver for authentication"); | ||
} | ||
|
||
return result.build(); | ||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
package io.drasi; | ||
|
||
import com.azure.identity.DefaultAzureCredentialBuilder; | ||
import com.azure.core.credential.AccessToken; | ||
import io.drasi.source.sdk.SourceProxy; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.sql.Connection; | ||
import java.sql.DriverManager; | ||
import java.sql.SQLException; | ||
import java.util.Properties; | ||
|
||
public class AzureIdentityPostgreSQLConnection { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(AzureIdentityPostgreSQLConnection.class); | ||
private static final String POSTGRES_SCOPE = "https://ossrdbms-aad.database.windows.net/.default"; | ||
|
||
public static Connection getConnection() throws SQLException { | ||
try { | ||
log.info("Using Azure Identity for PostgreSQL authentication"); | ||
|
||
// Get access token using DefaultAzureCredential | ||
var credential = new DefaultAzureCredentialBuilder().build(); | ||
AccessToken token = credential.getTokenSync( | ||
new com.azure.core.credential.TokenRequestContext() | ||
.addScopes(POSTGRES_SCOPE) | ||
); | ||
|
||
// Set up connection properties | ||
var props = new Properties(); | ||
props.setProperty("user", SourceProxy.GetConfigValue("user")); | ||
props.setProperty("password", token.getToken()); | ||
|
||
// For Azure PostgreSQL with managed identity, we need to set these additional properties | ||
props.setProperty("ssl", "true"); | ||
props.setProperty("sslmode", "require"); | ||
|
||
String connectionUrl = "jdbc:postgresql://" | ||
+ SourceProxy.GetConfigValue("host") + ":" | ||
+ SourceProxy.GetConfigValue("port") + "/" | ||
+ SourceProxy.GetConfigValue("database"); | ||
|
||
log.debug("Connecting to PostgreSQL using Azure Identity token"); | ||
return DriverManager.getConnection(connectionUrl, props); | ||
|
||
} catch (Exception e) { | ||
log.error("Failed to authenticate using Azure Identity", e); | ||
throw new SQLException("Azure Identity authentication failed", e); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make
user
andpassword
optional here along withidentity
block, and make themgmt_api
enforce that atleast one of them is present?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should avoid making global assumptions about how all sources should work in the Management API. Ideally we want to offload this validation to the implementor of each source.