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

Read SSL cert and key from files #38

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ def __init__(self):
self.SERVICE_BUS_JOB_STATE_TOPIC = 'jobstate-updatedby-computeprovider'
self.SERVICE_BUS_JOB_STATE_SUB = 'compute-scheduler-jobstate'

# constants for enableing SSL in inference
liakaz marked this conversation as resolved.
Show resolved Hide resolved
self.sslKeyPemFile = 'sslKeyPemFile'
liakaz marked this conversation as resolved.
Show resolved Hide resolved
self.sslCertPemFile = 'sslCertPemFile'
self.allowInsecureConnections = 'allowInsecureConnections'

# reference mapping
self.reference_mapping = {
self.RELAY_SERVER_CONNECTION_STRING: [self.RELAY_CONNECTION_STRING_KEY, self.RELAY_CONNECTION_STRING_DEPRECATED_KEY],
Expand Down Expand Up @@ -168,6 +173,7 @@ def __validate_config(self, configuration_settings, configuration_protected_sett
if enable_inference:
logger.warning("The installed AzureML extension for AML inference is experimental and not covered by customer support. Please use with discretion.")
self.__validate_scoring_fe_settings(configuration_settings, configuration_protected_settings)
self.__set_up_inference_ssl(configuration_settings, configuration_protected_settings)
elif not (enable_training or enable_inference):
raise InvalidArgumentValueError(
"Please create Microsoft.AzureML.Kubernetes extension instance either "
Expand All @@ -181,32 +187,53 @@ def __validate_config(self, configuration_settings, configuration_protected_sett
configuration_protected_settings.pop(self.ENABLE_INFERENCE, None)

def __validate_scoring_fe_settings(self, configuration_settings, configuration_protected_settings):
clusterPurpose = _get_value_from_config_protected_config(
'clusterPurpose', configuration_settings, configuration_protected_settings)
if clusterPurpose and clusterPurpose not in ["DevTest", "FastProd"]:
raise InvalidArgumentValueError(
"Accepted values for '--configuration-settings clusterPurpose' "
"are 'DevTest' and 'FastProd'")

feSslCert = _get_value_from_config_protected_config(
'scoringFe.sslCert', configuration_settings, configuration_protected_settings)
sslKey = _get_value_from_config_protected_config(
'scoringFe.sslKey', configuration_settings, configuration_protected_settings)
experimentalCluster = _get_value_from_config_protected_config(
'experimental', configuration_settings, configuration_protected_settings)
experimentalCluster = str(experimentalCluster).lower() == 'true'
if experimentalCluster:
configuration_settings['clusterPurpose'] = 'DevTest'
else:
configuration_settings['clusterPurpose'] = 'FastProd'
feSslCertFile = configuration_protected_settings.get(self.sslCertPemFile)
feSslKeyFile = configuration_protected_settings.get(self.sslKeyPemFile)
allowInsecureConnections = _get_value_from_config_protected_config(
'allowInsecureConnections', configuration_settings, configuration_protected_settings)
self.allowInsecureConnections, configuration_settings, configuration_protected_settings)
allowInsecureConnections = str(allowInsecureConnections).lower() == 'true'
if (not feSslCert or not sslKey) and not allowInsecureConnections:
if (not feSslCertFile or not feSslKeyFile) and not allowInsecureConnections:
raise InvalidArgumentValueError(
"Provide ssl certificate and key. "
"Otherwise explicitly allow insecure connection by specifying "
"'--configuration-settings allowInsecureConnections=true'")

feIsInternalLoadBalancer = _get_value_from_config_protected_config(
'scoringFe.serviceType.internalLoadBalancer', configuration_settings, configuration_protected_settings)
'privateEndpointILB', configuration_settings, configuration_protected_settings)
feIsInternalLoadBalancer = str(feIsInternalLoadBalancer).lower() == 'true'
if feIsInternalLoadBalancer:
logger.warning(
'Internal load balancer only supported on AKS and AKS Engine Clusters.')
configuration_protected_settings['scoringFe.privateEndpointILB'] = feIsInternalLoadBalancer
liakaz marked this conversation as resolved.
Show resolved Hide resolved

def __set_up_inference_ssl(self, configuration_settings, configuration_protected_settings):
allowInsecureConnections = _get_value_from_config_protected_config(
self.allowInsecureConnections, configuration_settings, configuration_protected_settings)
allowInsecureConnections = str(allowInsecureConnections).lower() == 'true'
liakaz marked this conversation as resolved.
Show resolved Hide resolved
if not allowInsecureConnections:
import base64
feSslCertFile = configuration_protected_settings.get(self.sslCertPemFile)
feSslKeyFile = configuration_protected_settings.get(self.sslKeyPemFile)
with open(feSslCertFile) as f:
cert_data = f.read()
liakaz marked this conversation as resolved.
Show resolved Hide resolved
cert_data_bytes = cert_data.encode("ascii")
ssl_cert = base64.b64encode(cert_data_bytes)
configuration_protected_settings['scoringFe.sslCert'] = ssl_cert
with open(feSslKeyFile) as f:
key_data = f.read()
key_data_bytes = key_data.encode("ascii")
ssl_key = base64.b64encode(key_data_bytes)
configuration_protected_settings['scoringFe.sslKey'] = ssl_key
else:
logger.warning(
'SSL is not set up. Allowing insecure connection to the deployed services')
liakaz marked this conversation as resolved.
Show resolved Hide resolved

def __create_required_resource(
self, cmd, configuration_settings, configuration_protected_settings, subscription_id, resource_group_name,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
dGVzdGNlcnQ=

Choose a reason for hiding this comment

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

Can you please put these files in extension-specific directories such as src/k8s-extension/azext_k8s_extension/tests/latest/partner_extensions/azure_ml/data/...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tbh, I would actually reverse the order and do data/azure_ml, otherwise it might be confusing for the future, since azureml related test are under src/k8s-extension/azext_k8s_extension/tests/latest/partner_extensions/public/ Hence I think it makes sense to introduce a data director that is going to have extension specific sub directories. Thoughts?

dGVzdGtleQ==
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testcert
liakaz marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testkey
liakaz marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import os
import unittest

from azext_k8s_extension.partner_extensions.AzureMLKubernetes import AzureMLKubernetes


TEST_DIR = os.path.abspath(os.path.join(os.path.abspath(__file__), '..'))


class TestAzureMlExtension(unittest.TestCase):

def test_set_up_inference_ssl(self):
azremlk8sInstance = AzureMLKubernetes()
config = {'allowInsecureConnections': 'false'}
# read and encode dummy cert and key
sslKeyPemFile = os.path.join(TEST_DIR, 'data', 'test_key.pem')
sslCertPemFile = os.path.join(TEST_DIR, 'data', 'test_cert.pem')
protected_config = {'sslKeyPemFile': sslKeyPemFile, 'sslCertPemFile': sslCertPemFile}
azremlk8sInstance._AzureMLKubernetes__set_up_inference_ssl(config, protected_config)
self.assertTrue('scoringFe.sslCert' in protected_config)
self.assertTrue('scoringFe.sslKey' in protected_config)
encoded_cert_and_key_file = os.path.join(TEST_DIR, 'data', 'cert_and_key_encoded.txt')
with open(encoded_cert_and_key_file, "rb") as text_file:
cert = text_file.readline().rstrip()
self.assertEquals(cert, protected_config['scoringFe.sslCert'])
key = text_file.readline()
self.assertEquals(key, protected_config['scoringFe.sslKey'])
1 change: 1 addition & 0 deletions testing/test/extensions/data/test_cert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testcert
liakaz marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions testing/test/extensions/data/test_key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testkey
47 changes: 47 additions & 0 deletions testing/test/extensions/public/AzureMLKubernetes.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,51 @@ Describe 'AzureML Kubernetes Testing' {
$badOut | Should -Not -BeNullOrEmpty
$output | Should -BeNullOrEmpty
}

It 'Creates the extension and checks that it onboards correctly with inference and SSL enabled' {
$sslKeyPemFile = Join-Path (Join-Path (Split-Path $PSScriptRoot -Parent) "data") "test_key.pem"
$sslCertPemFile = Join-Path (Join-Path (Split-Path $PSScriptRoot -Parent) "data") "test_cert.pem"
Invoke-Expression "az $Env:K8sExtensionName create -c $($ENVCONFIG.arcClusterName) -g $($ENVCONFIG.resourceGroup) --cluster-type connectedClusters --extension-type $extensionType -n $extensionName --release-train staging --config enableInference=true identity.proxy.remoteEnabled=True identity.proxy.remoteHost=https://master.experiments.azureml-test.net experimental=True --config-protected sslKeyPemFile=$sslKeyPemFile sslCertPemFile=$sslCertPemFile" -ErrorVariable badOut
$badOut | Should -BeNullOrEmpty

$output = Invoke-Expression "az $Env:K8sExtensionName show -c $($ENVCONFIG.arcClusterName) -g $($ENVCONFIG.resourceGroup) --cluster-type connectedClusters -n $extensionName" -ErrorVariable badOut
$badOut | Should -BeNullOrEmpty

$isAutoUpgradeMinorVersion = ($output | ConvertFrom-Json).autoUpgradeMinorVersion
$isAutoUpgradeMinorVersion.ToString() -eq "True" | Should -BeTrue

# Loop and retry until the extension installs
$n = 0
do
{
if (Get-ExtensionStatus $extensionName -eq $SUCCESS_MESSAGE) {
break
}
Start-Sleep -Seconds 20
$n += 1
} while ($n -le $MAX_RETRY_ATTEMPTS)
$n | Should -BeLessOrEqual $MAX_RETRY_ATTEMPTS

# check if relay is populated
$relayResourceID = Get-ExtensionConfigurationSettings $extensionName $relayResourceIDKey
$relayResourceID | Should -Not -BeNullOrEmpty
}

It "Deletes the extension from the cluster with inference enabled" {
# cleanup the relay and servicebus
$relayResourceID = Get-ExtensionConfigurationSettings $extensionName $relayResourceIDKey
$serviceBusResourceID = Get-ExtensionConfigurationSettings $extensionName $serviceBusResourceIDKey
$relayNamespaceName = $relayResourceID.split("/")[8]
$serviceBusNamespaceName = $serviceBusResourceID.split("/")[8]
az relay namespace delete --resource-group $ENVCONFIG.resourceGroup --name $relayNamespaceName
az servicebus namespace delete --resource-group $ENVCONFIG.resourceGroup --name $serviceBusNamespaceName

$output = Invoke-Expression "az $Env:K8sExtensionName delete -c $($ENVCONFIG.arcClusterName) -g $($ENVCONFIG.resourceGroup) --cluster-type connectedClusters -n $extensionName" -ErrorVariable badOut
$badOut | Should -BeNullOrEmpty

# Extension should not be found on the cluster
$output = Invoke-Expression "az $Env:K8sExtensionName show -c $($ENVCONFIG.arcClusterName) -g $($ENVCONFIG.resourceGroup) --cluster-type connectedClusters -n $extensionName" -ErrorVariable badOut
$badOut | Should -Not -BeNullOrEmpty
$output | Should -BeNullOrEmpty
}
}