bug fixes

ads/opctl/backend/marketplace/helm_helper.py

@@ -13,7 +13,9 @@
 from ads.opctl import logger
 from ads.opctl.backend.marketplace.marketplace_utils import (
     StatusIcons,
-    get_docker_bearer_token, WARNING, Color,
+    get_docker_bearer_token,
+    WARNING,
+    Color,
 )
 
 
@@ -46,13 +48,18 @@ def run_helm_install(
     )
     helm_cmd = [
         _HELM_BINARY_,
-        cmd,
+        HelmCommand.Upgrade,
         name,
         chart,
         *_get_as_flags_(
-            namespace=namespace, values=values_yaml_path, version=version, timeout="300s",**kwargs
+            namespace=namespace,
+            values=values_yaml_path,
+            version=version,
+            timeout="300s",
+            **kwargs,
         ),
-        "--wait"
+        "--wait",
+        "-i",
     ]
     print(f"\n{Color.BLUE}{' '.join(helm_cmd)}{Color.END}")
     return subprocess.run(helm_cmd)

ads/opctl/operator/lowcode/feature_store_marketplace/models/apigw_config.py

@@ -1,5 +1,5 @@
 from enum import Enum
-from typing import Union, Optional
+from typing import Union, Optional, List
 
 from ads.opctl.operator.lowcode.feature_store_marketplace.models.serializable_yaml_model import (
     SerializableYAMLModel,

ads/opctl/operator/lowcode/feature_store_marketplace/operator_utils.py

@@ -139,6 +139,10 @@ def detect_or_create_stack(apigw_config: APIGatewayConfig):
         print(
             f"Auto-detected feature store APIGW stack: {stacks[0].display_name}({stacks[0].id}"
         )
+        click.prompt(
+            f"Auto detected existing feature store stack: '{stacks[0].display_name}({stacks[0].id}'\n.Provide an OCID to use or",
+            default=stacks[0].id,
+        )
         return stacks[0].id
     elif len(stacks) == 0:
         if not click.confirm(
@@ -268,7 +272,7 @@ def update_resource_manager_stack(
         )
     )
     update_stack_details = oci.resource_manager.models.UpdateStackDetails()
-    groups = ",".join(apigw_config.authorized_user_groups)
+    groups = ",".join(apigw_config.authorized_user_groups.split(","))
     update_stack_details.variables = {
         "nlb_id": nlb_id,
         "tenancy_ocid": apigw_config.root_compartment_id,

tests/unitary/with_extras/operator/feature-store/test_helm_helper_util.py

@@ -3,9 +3,18 @@
 import pandas as pd
 import pytest
 
-from ads.opctl.backend.marketplace.helm_helper import run_helm_install, _HELM_BINARY_, HelmCommand, _get_as_flags_, \
-    run_helm_list, run_helm_login, _check_if_chart_already_exists_, check_helm_pull, HelmPullStatus
-from unittest.mock import patch, Mock, create_autospec
+from ads.opctl.backend.marketplace.helm_helper import (
+    run_helm_install,
+    _HELM_BINARY_,
+    HelmCommand,
+    _get_as_flags_,
+    run_helm_list,
+    run_helm_login,
+    _check_if_chart_already_exists_,
+    check_helm_pull,
+    HelmPullStatus,
+)
+from unittest.mock import patch, Mock
 
 name = "NAME"
 chart = "CHART_NAME"
@@ -28,9 +37,13 @@ def test_helm_install(mock_check_chart_exist: Mock, subprocess_mock: Mock):
         name,
         chart,
         *_get_as_flags_(
-            namespace=namespace, values=values_yaml_path, version=version, timeout="300s", **kwargs
+            namespace=namespace,
+            values=values_yaml_path,
+            version=version,
+            timeout="300s",
+            **kwargs,
         ),
-        "--wait"
+        "--wait",
     ]
 
     subprocess_mock.assert_called_with(helm_cmd)
@@ -48,34 +61,65 @@ def test_helm_upgrade(mock_check_chart_exist: Mock, subprocess_mock: Mock):
         name,
         chart,
         *_get_as_flags_(
-            namespace=namespace, values=values_yaml_path, version=version, timeout="300s", **kwargs
+            namespace=namespace,
+            values=values_yaml_path,
+            version=version,
+            timeout="300s",
+            **kwargs,
         ),
-        "--wait"
+        "--wait",
     ]
     subprocess_mock.assert_called_with(helm_cmd)
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_list_returns_single_value(subprocess_mock: Mock):
-    header = ["NAME", "NAMESPACE", "REVISION", "UPDATED", "STATUS", "CHART", "APP VERSION"]
-    data = ["fs-dp-api-test", "feature-store", "4", "2023-11-22 10:22:13.425579296 +0530 IST", "deployed",
-            "feature-store-dp-api-1.0", "0.1.270.marketplace-vuls"]
+    header = [
+        "NAME",
+        "NAMESPACE",
+        "REVISION",
+        "UPDATED",
+        "STATUS",
+        "CHART",
+        "APP VERSION",
+    ]
+    data = [
+        "fs-dp-api-test",
+        "feature-store",
+        "4",
+        "2023-11-22 10:22:13.425579296 +0530 IST",
+        "deployed",
+        "feature-store-dp-api-1.0",
+        "0.1.270.marketplace-vuls",
+    ]
     std_out = "\t".join(header) + "\n" + "\t".join(data)
 
-    list_result = subprocess.CompletedProcess(args="", returncode=0, stdout=std_out.encode())
+    list_result = subprocess.CompletedProcess(
+        args="", returncode=0, stdout=std_out.encode()
+    )
     subprocess_mock.return_value = list_result
     result = run_helm_list(namespace, **kwargs)
     assert len(result) == 1
     assert "NAME" in result.columns
-    assert any(result['NAME'] == "fs-dp-api-test")
+    assert any(result["NAME"] == "fs-dp-api-test")
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_list_returns_no_value(subprocess_mock: Mock):
-    header = ["NAME", "NAMESPACE", "REVISION", "UPDATED", "STATUS", "CHART", "APP VERSION"]
+    header = [
+        "NAME",
+        "NAMESPACE",
+        "REVISION",
+        "UPDATED",
+        "STATUS",
+        "CHART",
+        "APP VERSION",
+    ]
     std_out = "\t".join(header)
 
-    list_result = subprocess.CompletedProcess(args="", returncode=0, stdout=std_out.encode())
+    list_result = subprocess.CompletedProcess(
+        args="", returncode=0, stdout=std_out.encode()
+    )
     subprocess_mock.return_value = list_result
     result = run_helm_list(namespace, **kwargs)
     assert len(result) == 0
@@ -84,7 +128,9 @@ def test_helm_list_returns_no_value(subprocess_mock: Mock):
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_list_throws_exception(subprocess_mock: Mock):
-    list_result = subprocess.CompletedProcess(args="", returncode=1, stderr="Some Exception")
+    list_result = subprocess.CompletedProcess(
+        args="", returncode=1, stderr="Some Exception"
+    )
     subprocess_mock.return_value = list_result
     with pytest.raises(Exception) as e_info:
         run_helm_list(namespace, **kwargs)
@@ -94,75 +140,133 @@ def test_helm_list_throws_exception(subprocess_mock: Mock):
 def test_helm_login_success(subprocess_mock: Mock):
     subprocess_return = subprocess.CompletedProcess(args="", returncode=0)
     subprocess_mock.return_value = subprocess_return
-    run_helm_login('oci_repo', 'token')
+    run_helm_login("oci_repo", "token")
     subprocess_mock.assert_called_with(
-        ['helm', 'registry', 'login', 'oci_repo', '--username', 'BEARER_TOKEN', '--password', 'token'],
-        capture_output=True)
+        [
+            "helm",
+            "registry",
+            "login",
+            "oci_repo",
+            "--username",
+            "BEARER_TOKEN",
+            "--password",
+            "token",
+        ],
+        capture_output=True,
+    )
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_login_throws_exception(subprocess_mock: Mock):
-    subprocess_return = subprocess.CompletedProcess(args="", returncode=1, stderr="Some Exception")
+    subprocess_return = subprocess.CompletedProcess(
+        args="", returncode=1, stderr="Some Exception"
+    )
     subprocess_mock.return_value = subprocess_return
     with pytest.raises(Exception) as e_info:
-        run_helm_login('oci_repo', 'token')
+        run_helm_login("oci_repo", "token")
     subprocess_mock.assert_called_with(
-        ['helm', 'registry', 'login', 'oci_repo', '--username', 'BEARER_TOKEN', '--password', 'token'],
-        capture_output=True)
+        [
+            "helm",
+            "registry",
+            "login",
+            "oci_repo",
+            "--username",
+            "BEARER_TOKEN",
+            "--password",
+            "token",
+        ],
+        capture_output=True,
+    )
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_pull_success(subprocess_mock: Mock):
     subprocess_return = subprocess.CompletedProcess(args="", returncode=0, stderr=b"")
     subprocess_mock.return_value = subprocess_return
-    assert check_helm_pull('helm_chart_url', 'version') == HelmPullStatus.SUCCESS
+    assert check_helm_pull("helm_chart_url", "version") == HelmPullStatus.SUCCESS
     subprocess_mock.assert_called_with(
-        ['helm', 'pull', 'helm_chart_url', '--version', 'version'],
-        capture_output=True)
+        ["helm", "pull", "helm_chart_url", "--version", "version"], capture_output=True
+    )
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_pull_unauthorized(subprocess_mock: Mock):
-    subprocess_return = subprocess.CompletedProcess(args="", returncode=1, stderr=b"unauthorized")
+    subprocess_return = subprocess.CompletedProcess(
+        args="", returncode=1, stderr=b"unauthorized"
+    )
     subprocess_mock.return_value = subprocess_return
-    assert check_helm_pull('helm_chart_url', 'version') == HelmPullStatus.AUTHENTICATION_FAILURE
+    assert (
+        check_helm_pull("helm_chart_url", "version")
+        == HelmPullStatus.AUTHENTICATION_FAILURE
+    )
     subprocess_mock.assert_called_with(
-        ['helm', 'pull', 'helm_chart_url', '--version', 'version'],
-        capture_output=True)
+        ["helm", "pull", "helm_chart_url", "--version", "version"], capture_output=True
+    )
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.subprocess.run")
 def test_helm_pull_unknown_failure(subprocess_mock: Mock):
-    subprocess_return = subprocess.CompletedProcess(args="", returncode=1, stderr=b"some failure")
+    subprocess_return = subprocess.CompletedProcess(
+        args="", returncode=1, stderr=b"some failure"
+    )
     subprocess_mock.return_value = subprocess_return
-    assert check_helm_pull('helm_chart_url', 'version') == HelmPullStatus.UNKNOWN_FAILURE
+    assert (
+        check_helm_pull("helm_chart_url", "version") == HelmPullStatus.UNKNOWN_FAILURE
+    )
     subprocess_mock.assert_called_with(
-        ['helm', 'pull', 'helm_chart_url', '--version', 'version'],
-        capture_output=True)
+        ["helm", "pull", "helm_chart_url", "--version", "version"], capture_output=True
+    )
 
 
 def test_get_as_flags_():
-    args = {
-        "key1": "value1",
-        "key2": "value2"
-    }
+    args = {"key1": "value1", "key2": "value2"}
     assert _get_as_flags_(**args) == ["--key1", "value1", "--key2", "value2"]
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.run_helm_list")
 def test_check_helm_chart_exist_when_chart_do_exist(helm_list_cmd: Mock):
-    header = ["NAME", "NAMESPACE", "REVISION", "UPDATED", "STATUS", "CHART", "APP VERSION"]
-    data = ["fs-dp-api-test", "feature-store", "4", "2023-11-22 10:22:13.425579296 +0530 IST", "deployed",
-            "feature-store-dp-api-1.0", "0.1.270.marketplace-vuls"]
+    header = [
+        "NAME",
+        "NAMESPACE",
+        "REVISION",
+        "UPDATED",
+        "STATUS",
+        "CHART",
+        "APP VERSION",
+    ]
+    data = [
+        "fs-dp-api-test",
+        "feature-store",
+        "4",
+        "2023-11-22 10:22:13.425579296 +0530 IST",
+        "deployed",
+        "feature-store-dp-api-1.0",
+        "0.1.270.marketplace-vuls",
+    ]
     helm_list_cmd.return_value = pd.DataFrame([data], columns=header)
     assert _check_if_chart_already_exists_("fs-dp-api-test", namespace)
 
 
 @patch("ads.opctl.backend.marketplace.helm_helper.run_helm_list")
 def test_check_helm_chart_exist_when_chart_do_not_exist(helm_list_cmd: Mock):
-    header = ["NAME", "NAMESPACE", "REVISION", "UPDATED", "STATUS", "CHART", "APP VERSION"]
-    data = ["some-other-chart", "feature-store", "4", "2023-11-22 10:22:13.425579296 +0530 IST", "deployed",
-            "feature-store-dp-api-1.0", "0.1.270.marketplace-vuls"]
+    header = [
+        "NAME",
+        "NAMESPACE",
+        "REVISION",
+        "UPDATED",
+        "STATUS",
+        "CHART",
+        "APP VERSION",
+    ]
+    data = [
+        "some-other-chart",
+        "feature-store",
+        "4",
+        "2023-11-22 10:22:13.425579296 +0530 IST",
+        "deployed",
+        "feature-store-dp-api-1.0",
+        "0.1.270.marketplace-vuls",
+    ]
     helm_list_cmd.return_value = pd.DataFrame([data], columns=header)
     assert not _check_if_chart_already_exists_("fs-dp-api-test", namespace)
 

tests/unitary/with_extras/operator/feature-store/test_marketplace_utils.py

@@ -6,7 +6,7 @@
     get_docker_bearer_token,
     _export_helm_chart_,
     list_container_images,
-    get_tags_map,
+    _get_tags_map,
 )
 
 
@@ -18,13 +18,14 @@ def test_set_kubernetes_session_token_env():
     assert os.getenv("OCI_CLI_PROFILE") == profile_name
 
 
+@patch("ads.common.auth.default_signer")
 @patch("ads.opctl.backend.marketplace.marketplace_utils.OCIClientFactory")
-def test_get_docker_bearer_token(client_factory: Mock):
+def test_get_docker_bearer_token(client_factory: Mock, signer_mock: Mock):
     mock_token = '{"token":"TOKEN"}'
     token_client = Mock()
     token_client.call_api.return_value.data = mock_token
     client_factory.return_value.create_client.return_value = token_client
-    ocir_repo = "iad.ocir.io/idogsu2ylimg/feature-store-data-plane-api-helidon/"
+    ocir_repo = "iad.ocir.io/namespace/feature-store-data-plane-api-helidon/"
     assert get_docker_bearer_token(ocir_repo) == mock_token
     token_client.call_api.assert_called_once_with(
         resource_path="/docker/token", method="GET", response_type="SecurityToken"
@@ -62,6 +63,6 @@ def test_export_helm_chart_to_container_registry(list_api: Mock, export_api: Moc
     )
     listing_details = Mock()
     listing_details.container_tag_pattern = [pattern]
-    result = get_tags_map(listing_details)
+    result = _get_tags_map(listing_details)
     assert pattern in result
     assert result[pattern] == f"{pattern}-1"