Read SSL cert and key from files (#38)

* first sketch of the change fixes removed extra blank lines changes regarding param renaming added ssl tests added more detail to the unit test additional import moved pem files out of public folder fixed import chenged import changed import unit tests fix unit test fix fixed unit tests fixed unit test unit test fix changes int test cert and key * test protected config * fix test typo * temporary changes reverted * fixing tests * fixed file paths * removed accidentally added file * changes according to review comments * more changes according to review comments * changes according to review comments Co-authored-by: Jonathan Innis <[email protected]>
AzureArcForKubernetes · Jun 2, 2021 · 6955567 · 6955567
1 parent 9100ff5
commit 6955567
Show file tree

Hide file tree

Showing 8 changed files with 127 additions and 14 deletions.
diff --git a/src/k8s-extension/azext_k8s_extension/partner_extensions/AzureMLKubernetes.py b/src/k8s-extension/azext_k8s_extension/partner_extensions/AzureMLKubernetes.py
@@ -66,6 +66,12 @@ def __init__(self):
         self.SERVICE_BUS_JOB_STATE_TOPIC = 'jobstate-updatedby-computeprovider'
         self.SERVICE_BUS_JOB_STATE_SUB = 'compute-scheduler-jobstate'
 
+        # constants for enabling SSL in inference
+        self.sslKeyPemFile = 'sslKeyPemFile'
+        self.sslCertPemFile = 'sslCertPemFile'
+        self.allowInsecureConnections = 'allowInsecureConnections'
+        self.privateEndpointILB = 'privateEndpointILB'
+
         # reference mapping
         self.reference_mapping = {
             self.RELAY_SERVER_CONNECTION_STRING: [self.RELAY_CONNECTION_STRING_KEY, self.RELAY_CONNECTION_STRING_DEPRECATED_KEY],
@@ -168,6 +174,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 "
@@ -181,32 +188,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)
+            self.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.%s' % self.privateEndpointILB] = feIsInternalLoadBalancer
+
+    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'
+        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()
+                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 enabled. Allowing insecure connections to the deployed services.')
 
     def __create_required_resource(
             self, cmd, configuration_settings, configuration_protected_settings, subscription_id, resource_group_name,

diff --git a/src/k8s-extension/azext_k8s_extension/tests/latest/data/azure_ml/cert_and_key_encoded.txt b/src/k8s-extension/azext_k8s_extension/tests/latest/data/azure_ml/cert_and_key_encoded.txt
@@ -0,0 +1,2 @@
+dGVzdGNlcnQ=
+dGVzdGtleQ==
diff --git a/src/k8s-extension/azext_k8s_extension/tests/latest/data/azure_ml/test_cert.pem b/src/k8s-extension/azext_k8s_extension/tests/latest/data/azure_ml/test_cert.pem
@@ -0,0 +1 @@
+testcert
diff --git a/src/k8s-extension/azext_k8s_extension/tests/latest/data/azure_ml/test_key.pem b/src/k8s-extension/azext_k8s_extension/tests/latest/data/azure_ml/test_key.pem
@@ -0,0 +1 @@
+testkey
diff --git a/src/k8s-extension/azext_k8s_extension/tests/latest/test_azureml_extension.py b/src/k8s-extension/azext_k8s_extension/tests/latest/test_azureml_extension.py
@@ -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', 'azure_ml', 'test_key.pem')
+        sslCertPemFile = os.path.join(TEST_DIR, 'data', 'azure_ml', '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', 'azure_ml', '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'])
diff --git a/testing/test/extensions/data/azure_ml/test_cert.pem b/testing/test/extensions/data/azure_ml/test_cert.pem
@@ -0,0 +1 @@
+testcert
diff --git a/testing/test/extensions/data/azure_ml/test_key.pem b/testing/test/extensions/data/azure_ml/test_key.pem
@@ -0,0 +1 @@
+testkey
diff --git a/testing/test/extensions/public/AzureMLKubernetes.Tests.ps1 b/testing/test/extensions/public/AzureMLKubernetes.Tests.ps1
@@ -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 (Join-Path (Split-Path $PSScriptRoot -Parent) "data") "azure_ml") "test_key.pem"
+        $sslCertPemFile = Join-Path (Join-Path (Join-Path (Split-Path $PSScriptRoot -Parent) "data") "azure_ml") "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
+    }
 }