Fix#6517: Add clusterName property to the application config yaml (#6610)

* Add cluster name in the app configuration and start using it to create secrets id

* Update secret manager client in openmetadata for using default auth provider

* Add missing property in test config file
This commit is contained in:
Nahuel 2022-08-09 09:00:43 +02:00 committed by GitHub
parent c9c99d6506
commit cf2cb6d531
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 161 additions and 71 deletions

View File

@ -96,7 +96,8 @@ public class CatalogApplication extends Application<CatalogApplicationConfig> {
jdbi.setTimingCollector(new MicrometerJdbiTimingCollector());
final SecretsManager secretsManager =
SecretsManagerFactory.createSecretsManager(catalogConfig.getSecretsManagerConfiguration());
SecretsManagerFactory.createSecretsManager(
catalogConfig.getSecretsManagerConfiguration(), catalogConfig.getClusterName());
secretsManager.encryptAirflowConnection(catalogConfig.getAirflowConfiguration());

View File

@ -92,6 +92,9 @@ public class CatalogApplicationConfig extends Configuration {
@JsonProperty("secretsManagerConfiguration")
private SecretsManagerConfiguration secretsManagerConfiguration;
@JsonProperty("clusterName")
private String clusterName;
@Override
public String toString() {
return "catalogConfig{"

View File

@ -53,7 +53,6 @@ import javax.ws.rs.core.UriInfo;
import lombok.extern.slf4j.Slf4j;
import org.openmetadata.catalog.CatalogApplicationConfig;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.airflow.AirflowConfiguration;
import org.openmetadata.catalog.airflow.AirflowRESTClient;
import org.openmetadata.catalog.api.services.ingestionPipelines.CreateIngestionPipeline;
import org.openmetadata.catalog.api.services.ingestionPipelines.TestServiceConnection;
@ -81,7 +80,7 @@ import org.openmetadata.catalog.util.ResultList;
public class IngestionPipelineResource extends EntityResource<IngestionPipeline, IngestionPipelineRepository> {
public static final String COLLECTION_PATH = "v1/services/ingestionPipelines/";
private PipelineServiceClient pipelineServiceClient;
private AirflowConfiguration airflowConfiguration;
private CatalogApplicationConfig catalogApplicationConfig;
private final SecretsManager secretsManager;
@Override
@ -97,8 +96,8 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
}
public void initialize(CatalogApplicationConfig config) {
this.airflowConfiguration = config.getAirflowConfiguration();
this.pipelineServiceClient = new AirflowRESTClient(config.getAirflowConfiguration());
this.catalogApplicationConfig = config;
this.pipelineServiceClient = new AirflowRESTClient(catalogApplicationConfig.getAirflowConfiguration());
dao.setPipelineServiceClient(pipelineServiceClient);
}
@ -550,7 +549,8 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
private IngestionPipeline getIngestionPipeline(CreateIngestionPipeline create, String user) throws IOException {
OpenMetadataServerConnection openMetadataServerConnection =
secretsManager.decryptServerConnection(airflowConfiguration);
secretsManager.decryptServerConnection(catalogApplicationConfig.getAirflowConfiguration());
openMetadataServerConnection.setClusterName(catalogApplicationConfig.getClusterName());
openMetadataServerConnection.setSecretsManagerProvider(this.secretsManager.getSecretsManagerProvider());
return copy(new IngestionPipeline(), create, user)
.withPipelineType(create.getPipelineType())

View File

@ -28,8 +28,10 @@ public class AWSSecretsManager extends SecretsManager {
private SecretsManagerClient secretsClient;
private AWSSecretsManager(
OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider, SecretsManagerConfiguration config) {
super(secretsManagerProvider);
OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider,
SecretsManagerConfiguration config,
String clusterPrefix) {
super(secretsManagerProvider, clusterPrefix);
if (config == null) {
throw new SecretsManagerException("Secrets manager configuration is empty.");
}
@ -52,7 +54,8 @@ public class AWSSecretsManager extends SecretsManager {
@Override
public Object encryptOrDecryptServiceConnectionConfig(
Object connectionConfig, String connectionType, String connectionName, ServiceType serviceType, boolean encrypt) {
String secretName = buildSecretId(serviceType.value().toLowerCase(Locale.ROOT), connectionType, connectionName);
String secretName =
buildSecretId("service", serviceType.value().toLowerCase(Locale.ROOT), connectionType, connectionName);
try {
if (encrypt) {
String connectionConfigJson = JsonUtils.pojoToJson(connectionConfig);
@ -160,8 +163,8 @@ public class AWSSecretsManager extends SecretsManager {
return this.secretsClient.getSecretValue(getSecretValueRequest).secretString();
}
public static AWSSecretsManager getInstance(SecretsManagerConfiguration config) {
if (INSTANCE == null) INSTANCE = new AWSSecretsManager(AWS, config);
public static AWSSecretsManager getInstance(SecretsManagerConfiguration config, String clusterPrefix) {
if (INSTANCE == null) INSTANCE = new AWSSecretsManager(AWS, config, clusterPrefix);
return INSTANCE;
}

View File

@ -19,8 +19,9 @@ public class LocalSecretsManager extends SecretsManager {
private Fernet fernet;
private LocalSecretsManager(OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider) {
super(secretsManagerProvider);
private LocalSecretsManager(
OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider, String clusterPrefix) {
super(secretsManagerProvider, clusterPrefix);
this.fernet = Fernet.getInstance();
}
@ -89,8 +90,8 @@ public class LocalSecretsManager extends SecretsManager {
}
}
public static LocalSecretsManager getInstance() {
if (INSTANCE == null) INSTANCE = new LocalSecretsManager(LOCAL);
public static LocalSecretsManager getInstance(String clusterPrefix) {
if (INSTANCE == null) INSTANCE = new LocalSecretsManager(LOCAL, clusterPrefix);
return INSTANCE;
}

View File

@ -13,12 +13,14 @@ import org.openmetadata.catalog.services.connections.metadata.OpenMetadataServer
public abstract class SecretsManager {
public static final String OPENMETADATA_PREFIX = "openmetadata";
@Getter private final String clusterPrefix;
@Getter private final OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider;
protected SecretsManager(OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider) {
protected SecretsManager(
OpenMetadataServerConnection.SecretsManagerProvider secretsManagerProvider, String clusterPrefix) {
this.secretsManagerProvider = secretsManagerProvider;
this.clusterPrefix = clusterPrefix;
}
public abstract boolean isLocal();
@ -41,16 +43,26 @@ public abstract class SecretsManager {
protected abstract Object decryptAuthProviderConfig(
OpenMetadataServerConnection.AuthProvider authProvider, AuthConfiguration authConfig);
protected String buildSecretId(String... suffixes) {
protected String getSecretSeparator() {
return "/";
}
protected boolean startsWithSeparator() {
return true;
}
protected String buildSecretId(String... secretIdValues) {
StringBuilder format = new StringBuilder();
format.append(OPENMETADATA_PREFIX);
for (String suffix : List.of(suffixes)) {
if (isNull(suffix)) {
format.append(startsWithSeparator() ? getSecretSeparator() : "");
format.append(clusterPrefix);
for (String secretIdValue : List.of(secretIdValues)) {
if (isNull(secretIdValue)) {
throw new SecretsManagerException("Cannot build a secret id with null values.");
}
format.append("-%s");
format.append(getSecretSeparator());
format.append("%s");
}
return String.format(format.toString(), (Object[]) suffixes).toLowerCase();
return String.format(format.toString(), (Object[]) secretIdValues).toLowerCase();
}
protected Class<?> createConnectionConfigClass(String connectionType, String connectionPackage)

View File

@ -4,16 +4,16 @@ import org.openmetadata.catalog.services.connections.metadata.OpenMetadataServer
public class SecretsManagerFactory {
public static SecretsManager createSecretsManager(SecretsManagerConfiguration config) {
public static SecretsManager createSecretsManager(SecretsManagerConfiguration config, String clusterName) {
SecretsManagerProvider secretManager =
config != null && config.getSecretsManager() != null
? config.getSecretsManager()
: SecretsManagerConfiguration.DEFAULT_SECRET_MANAGER;
switch (secretManager) {
case LOCAL:
return LocalSecretsManager.getInstance();
return LocalSecretsManager.getInstance(clusterName);
case AWS:
return AWSSecretsManager.getInstance(config);
return AWSSecretsManager.getInstance(config, clusterName);
default:
throw new IllegalArgumentException("Not implemented secret manager store: " + secretManager);
}

View File

@ -14,6 +14,11 @@
}
},
"properties": {
"clusterName": {
"description": "Cluster name to differentiate OpenMetadata Server instance",
"type": "string",
"default": "openmetadata"
},
"type": {
"description": "Service Type",
"$ref": "#/definitions/openmetadataType",

View File

@ -51,12 +51,11 @@ import software.amazon.awssdk.services.secretsmanager.model.UpdateSecretRequest;
public class AWSSecretsManagerTest {
private static final String AUTH_PROVIDER_SECRET_ID_SUFFIX = "auth-provider";
private static final boolean ENCRYPT = true;
private static final boolean DECRYPT = false;
private static final String EXPECTED_CONNECTION_JSON =
"{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"password\":\"openmetadata-test\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true}";
private static final String EXPECTED_SECRET_ID = "openmetadata-database-mysql-test";
private static final String EXPECTED_SECRET_ID = "/openmetadata/service/database/mysql/test";
@Mock private SecretsManagerClient secretsManagerClient;
@ -70,7 +69,7 @@ public class AWSSecretsManagerTest {
parameters.put("secretAccessKey", "654321");
SecretsManagerConfiguration config = new SecretsManagerConfiguration();
config.setParameters(parameters);
secretsManager = AWSSecretsManager.getInstance(config);
secretsManager = AWSSecretsManager.getInstance(config, "openmetadata");
secretsManager.setSecretsClient(secretsManagerClient);
reset(secretsManagerClient);
}
@ -136,7 +135,7 @@ public class AWSSecretsManagerTest {
OpenMetadataServerConnection.AuthProvider authProvider,
AuthConfiguration authConfig)
throws JsonProcessingException {
String expectedSecretId = String.format("openmetadata-%s-%s", AUTH_PROVIDER_SECRET_ID_SUFFIX, authProvider);
String expectedSecretId = String.format("/openmetadata/%s/%s", AUTH_PROVIDER_SECRET_ID_SUFFIX, authProvider);
AirflowConfiguration airflowConfiguration = ConfigurationFixtures.buildAirflowConfig(authProvider);
airflowConfiguration.setAuthConfig(authConfig);
AirflowConfiguration expectedAirflowConfiguration = ConfigurationFixtures.buildAirflowConfig(authProvider);

View File

@ -47,7 +47,7 @@ public class LocalSecretsManagerTest {
@BeforeAll
static void setUp() {
secretsManager = LocalSecretsManager.getInstance();
secretsManager = LocalSecretsManager.getInstance("openmetadata");
Fernet fernet = Mockito.mock(Fernet.class);
lenient().when(fernet.decrypt(anyString())).thenReturn(DECRYPTED_VALUE);
lenient().when(fernet.encrypt(anyString())).thenReturn(ENCRYPTED_VALUE);

View File

@ -11,6 +11,8 @@ public class SecretsManagerFactoryTest {
private SecretsManagerConfiguration config;
private static final String CLUSTER_NAME = "openmetadata";
@BeforeEach
void setUp() {
config = new SecretsManagerConfiguration();
@ -19,18 +21,18 @@ public class SecretsManagerFactoryTest {
@Test
void testDefaultIsCreatedIfNullConfig() {
assertTrue(SecretsManagerFactory.createSecretsManager(config) instanceof LocalSecretsManager);
assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof LocalSecretsManager);
}
@Test
void testDefaultIsCreatedIfMissingSecretManager() {
assertTrue(SecretsManagerFactory.createSecretsManager(config) instanceof LocalSecretsManager);
assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof LocalSecretsManager);
}
@Test
void testIsCreatedIfLocalSecretsManager() {
config.setSecretsManager(SecretsManagerProvider.LOCAL);
assertTrue(SecretsManagerFactory.createSecretsManager(config) instanceof LocalSecretsManager);
assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof LocalSecretsManager);
}
@Test
@ -39,6 +41,6 @@ public class SecretsManagerFactoryTest {
config.getParameters().put("region", "eu-west-1");
config.getParameters().put("accessKeyId", "123456");
config.getParameters().put("secretAccessKey", "654321");
assertTrue(SecretsManagerFactory.createSecretsManager(config) instanceof AWSSecretsManager);
assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof AWSSecretsManager);
}
}

View File

@ -9,6 +9,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
clusterName: openmetadata
swagger:
resourcePackage: org.openmetadata.catalog.webservice.resources

View File

@ -9,6 +9,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
clusterName: ${OPENMETADATA_CLUSTER_NAME:-openmetadata}
swagger:
resourcePackage: org.openmetadata.catalog.resources

View File

@ -426,7 +426,8 @@ access_control_allow_origin =
[openmetadata_airflow_apis]
dag_generated_configs = /airflow/dag_generated_configs
[openmetadata_secrets_manager]
# this section is optional, the default auth provider for the secrets' manager service will be used if it is not set
# [openmetadata_secrets_manager]
# aws_access_key_id =
# aws_secret_access_key =
# aws_region =

View File

@ -423,7 +423,8 @@ auth_provider_type = no-auth
dag_runner_template = /airflow/dag_templates/dag_runner.j2
dag_generated_configs = /airflow/dag_generated_configs
[openmetadata_secrets_manager]
# this section is optional, the default auth provider for the secrets' manager service will be used if it is not set
# [openmetadata_secrets_manager]
# aws_access_key_id =
# aws_secret_access_key =
# aws_region =

View File

@ -168,7 +168,7 @@ class OpenMetadata(
# Load the secrets' manager client
self.secrets_manager_client = get_secrets_manager(
config.secretsManagerProvider, config.secretsManagerCredentials
config, config.secretsManagerCredentials
)
# Load auth provider config from Secret Manager if necessary

View File

@ -84,6 +84,11 @@ class SecretsManager(metaclass=Singleton):
providers.
"""
cluster_prefix: str
def __init__(self, cluster_prefix: str):
self.cluster_prefix = cluster_prefix
@abstractmethod
def retrieve_service_connection(
self,
@ -105,8 +110,15 @@ class SecretsManager(metaclass=Singleton):
"""
pass
@staticmethod
def build_secret_id(*args: str) -> str:
@property
def secret_id_separator(self) -> str:
return "/"
@property
def starts_with_separator(self) -> bool:
return True
def build_secret_id(self, *args: str) -> str:
"""
Returns a secret_id used by the secrets' manager providers for retrieving a secret.
For example:
@ -114,8 +126,8 @@ class SecretsManager(metaclass=Singleton):
:param args: sorted parameters for building the secret_id
:return: the secret_id
"""
secret_suffix = "-".join([arg.lower() for arg in args])
return f"openmetadata-{secret_suffix}"
secret_id = self.secret_id_separator.join([arg.lower() for arg in args])
return f"{self.secret_id_separator if self.starts_with_separator else ''}{self.cluster_prefix}{self.secret_id_separator}{secret_id}"
@staticmethod
def get_service_connection_class(service_type: str) -> object:
@ -181,13 +193,19 @@ class LocalSecretsManager(SecretsManager):
class AWSSecretsManager(SecretsManager):
def __init__(self, credentials: AWSCredentials):
session = boto3.Session(
aws_access_key_id=credentials.awsAccessKeyId,
aws_secret_access_key=credentials.awsSecretAccessKey.get_secret_value(),
region_name=credentials.awsRegion,
)
self.secretsmanager_client = session.client("secretsmanager")
def __init__(self, credentials: AWSCredentials, cluster_prefix: str):
super().__init__(cluster_prefix)
# initialize the secret client depending on the SecretsManagerConfiguration passed
if credentials:
session = boto3.Session(
aws_access_key_id=credentials.awsAccessKeyId,
aws_secret_access_key=credentials.awsSecretAccessKey.get_secret_value(),
region_name=credentials.awsRegion,
)
self.secretsmanager_client = session.client("secretsmanager")
else:
# initialized with the credentials loaded from running machine
self.secretsmanager_client = boto3.client("secretsmanager")
def retrieve_service_connection(
self,
@ -197,7 +215,7 @@ class AWSSecretsManager(SecretsManager):
service_connection_type = service.serviceType.value
service_name = service.name.__root__
secret_id = self.build_secret_id(
service_type, service_connection_type, service_name
"service", service_type, service_connection_type, service_name
)
connection_class = self.get_connection_class(
service_type, service_connection_type
@ -251,12 +269,14 @@ class AWSSecretsManager(SecretsManager):
def get_secrets_manager(
secret_manager: SecretsManagerProvider,
open_metadata_config: OpenMetadataConnection,
credentials: Optional[Union[AWSCredentials]] = None,
) -> SecretsManager:
if secret_manager == SecretsManagerProvider.local:
return LocalSecretsManager()
elif secret_manager == SecretsManagerProvider.aws:
return AWSSecretsManager(credentials)
if open_metadata_config.secretsManagerProvider == SecretsManagerProvider.local:
return LocalSecretsManager(open_metadata_config.clusterName)
elif open_metadata_config.secretsManagerProvider == SecretsManagerProvider.aws:
return AWSSecretsManager(credentials, open_metadata_config.clusterName)
else:
raise NotImplementedError(f"[{secret_manager}] is not implemented.")
raise NotImplementedError(
f"[{open_metadata_config.secretsManagerProvider}] is not implemented."
)

View File

@ -41,6 +41,7 @@ from metadata.generated.schema.security.client.googleSSOClientConfig import (
from metadata.generated.schema.security.credentials.awsCredentials import AWSCredentials
from metadata.utils.secrets_manager import (
AUTH_PROVIDER_MAPPING,
SecretsManager,
Singleton,
get_secrets_manager,
)
@ -80,7 +81,9 @@ class TestSecretsManager(TestCase):
Singleton.clear_all()
def test_local_manager_add_service_config_connection(self):
local_manager = get_secrets_manager(SecretsManagerProvider.local, None)
local_manager = get_secrets_manager(
self._build_open_metadata_connection(SecretsManagerProvider.local), None
)
expected_service_connection = self.service_connection
actual_service_connection: ServiceConnection = (
@ -93,7 +96,9 @@ class TestSecretsManager(TestCase):
)
def test_local_manager_add_auth_provider_security_config(self):
local_manager = get_secrets_manager(SecretsManagerProvider.local, None)
local_manager = get_secrets_manager(
self._build_open_metadata_connection(SecretsManagerProvider.local), None
)
actual_om_connection = deepcopy(self.om_connection)
actual_om_connection.securityConfig = self.auth_provider_config
@ -113,6 +118,12 @@ class TestSecretsManager(TestCase):
aws_manager.retrieve_service_connection(self.service, self.service_type)
)
expected_call = {
"SecretId": "/openmetadata/service/database/mysql/test_service"
}
aws_manager.secretsmanager_client.get_secret_value.assert_called_once_with(
**expected_call
)
self.assertEqual(expected_service_connection, actual_service_connection)
assert id(actual_service_connection.__root__.config) != id(
expected_service_connection.__root__.config
@ -138,6 +149,10 @@ class TestSecretsManager(TestCase):
aws_manager.add_auth_provider_security_config(actual_om_connection)
expected_call = {"SecretId": "/openmetadata/auth-provider/google"}
aws_manager.secretsmanager_client.get_secret_value.assert_called_once_with(
**expected_call
)
self.assertEqual(self.auth_provider_config, actual_om_connection.securityConfig)
assert id(self.auth_provider_config) != id(actual_om_connection.securityConfig)
@ -155,7 +170,11 @@ class TestSecretsManager(TestCase):
def test_get_not_implemented_secret_manager(self):
with self.assertRaises(NotImplementedError) as not_implemented_error:
get_secrets_manager("any")
om_connection: OpenMetadataConnection = (
self._build_open_metadata_connection(SecretsManagerProvider.local)
)
om_connection.secretsManagerProvider = "aws"
get_secrets_manager(om_connection)
self.assertEqual(
"[any] is not implemented.", not_implemented_error.exception
)
@ -167,10 +186,12 @@ class TestSecretsManager(TestCase):
for auth_provider in auth_provider_with_client:
assert AUTH_PROVIDER_MAPPING.get(auth_provider, None) is not None
def _build_secret_manager(self, mocked_boto3: Mock, expected_json: Dict[str, Any]):
def _build_secret_manager(
self, mocked_boto3: Mock, expected_json: Dict[str, Any]
) -> SecretsManager:
self._init_boto3_mock(mocked_boto3, expected_json)
aws_manager = get_secrets_manager(
SecretsManagerProvider.aws,
self._build_open_metadata_connection(SecretsManagerProvider.aws),
AWSCredentials(
awsAccessKeyId="fake_key",
awsSecretAccessKey="fake_access",
@ -179,6 +200,16 @@ class TestSecretsManager(TestCase):
)
return aws_manager
@staticmethod
def _build_open_metadata_connection(
secret_manager_provider: SecretsManagerProvider,
) -> OpenMetadataConnection:
return OpenMetadataConnection(
secretsManagerProvider=secret_manager_provider,
clusterName="openmetadata",
hostPort="http://localhost:8585/api",
)
@staticmethod
def _init_boto3_mock(boto3_mock: Mock, client_return: Dict[str, Any]):
mocked_client = Mock()

View File

@ -9,16 +9,18 @@ from metadata.utils.secrets_manager import SECRET_MANAGER_AIRFLOW_CONF
def build_aws_credentials():
credentials = AWSCredentials(
awsRegion=conf.get(SECRET_MANAGER_AIRFLOW_CONF, "aws_region", fallback="")
)
credentials.awsAccessKeyId = conf.get(
SECRET_MANAGER_AIRFLOW_CONF, "aws_access_key_id", fallback=""
)
credentials.awsSecretAccessKey = SecretStr(
conf.get(SECRET_MANAGER_AIRFLOW_CONF, "aws_secret_access_key", fallback="")
)
return credentials
if conf.has_section(SECRET_MANAGER_AIRFLOW_CONF):
credentials = AWSCredentials(
awsRegion=conf.get(SECRET_MANAGER_AIRFLOW_CONF, "aws_region", fallback="")
)
credentials.awsAccessKeyId = conf.get(
SECRET_MANAGER_AIRFLOW_CONF, "aws_access_key_id", fallback=""
)
credentials.awsSecretAccessKey = SecretStr(
conf.get(SECRET_MANAGER_AIRFLOW_CONF, "aws_secret_access_key", fallback="")
)
return credentials
return None
def build_secrets_manager_credentials(secrets_manager: SecretsManagerProvider):

View File

@ -14,6 +14,11 @@
}
},
"properties": {
"clusterName": {
"description": "Cluster name to differentiate OpenMetadata Server instance",
"type": "string",
"default": "openmetadata"
},
"type": {
"description": "Service Type",
"$ref": "#/definitions/openmetadataType",