Skip to content

Add connector SPI for returning redactable properties #24562

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add connector SPI for returning redactable properties #24562

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2eb48a4
Add SPI for returning connectors' sensitive properties
piotrrzysko Dec 23, 2024
c214ce6
Expose security-sensitive properties for PostgreSQL connector
piotrrzysko Dec 23, 2024
a152dae
Expose security-sensitive properties for HDFS
piotrrzysko Jan 9, 2025
3fa3827
Expose security-sensitive properties for Hive connector
piotrrzysko Jan 9, 2025
d63f50b
Expose security-sensitive properties for Iceberg connector
piotrrzysko Jan 9, 2025
a7cb1f0
Expose security-sensitive properties for Delta Lake connector
piotrrzysko Jan 9, 2025
f8d59f2
Expose security-sensitive properties for Hudi connector
piotrrzysko Jan 9, 2025
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
18 changes: 18 additions & 0 deletions core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,31 @@
package io.trino.spi.connector;

import com.google.errorprone.annotations.CheckReturnValue;
import io.trino.spi.Experimental;

import java.util.Map;
import java.util.Set;

public interface ConnectorFactory
{
String getName();

@CheckReturnValue
Connector create(String catalogName, Map<String, String> config, ConnectorContext context);

/**
* Returns property names that may contain security-sensitive information.
* In addition to properties that are explicitly known to the connector as
* security-sensitive, it may also return properties that are unknown or unused.
* In other words, if the connector cannot classify a property, it should treat it as
* security-sensitive by default for safety.
* <p>
* The engine uses the properties returned by this method to mask the corresponding
* values, preventing the leakage of security-sensitive information.
*/
@Experimental(eta = "2025-12-31")
default Set<String> getSecuritySensitivePropertyNames(String catalogName, Map<String, String> config, ConnectorContext context)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the method take the full config map and not just the names?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this PR should go in until we have more clarity on exactly how it will be used by the engine. An SPI in a vacuum and without usages doesn't make sense to include.

The PR linked in the description is still work in progress, so let's narrow down on that first.

Copy link
Member Author

@piotrrzysko piotrrzysko Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the method take the full config map and not just the names?

We use the values to get security-sensitive properties from Airlift. Here’s an example:

trino/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcConnectorFactory.java

Lines 71 to 82 in c214ce6

@Override
public Set<String> getSecuritySensitivePropertyNames(String catalogName, Map<String, String> config, ConnectorContext context)
{
Bootstrap app = createBootstrap(catalogName, config, context);
Set<ConfigPropertyMetadata> usedProperties = app
.quiet()
.skipErrorReporting()
.configure();
return ConfigUtils.getSecuritySensitivePropertyNames(config, usedProperties);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martint that's because config properties are dependant on each other and without actual values you don't know which ones will be bound during bootstrap

{
return Set.copyOf(config.keySet());
}
}