diff --git a/src/mas/devops/olm.py b/src/mas/devops/olm.py index d9351031..63f5f0d4 100644 --- a/src/mas/devops/olm.py +++ b/src/mas/devops/olm.py @@ -11,6 +11,7 @@ import logging from time import sleep from os import path +from typing import Optional from kubernetes.dynamic.exceptions import NotFoundError from openshift.dynamic import DynamicClient @@ -117,13 +118,17 @@ def getSubscription(dynClient: DynamicClient, namespace: str, packageName: str): return subscriptions.items[0] -def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str, packageChannel: str = None, catalogSource: str = None, catalogSourceNamespace: str = "openshift-marketplace", config: dict = None, installMode: str = "OwnNamespace"): +def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str, packageChannel: Optional[str] = None, catalogSource: Optional[str] = None, catalogSourceNamespace: str = "openshift-marketplace", config: Optional[dict] = None, installMode: str = "OwnNamespace", installPlanApproval: Optional[str] = None, startingCSV: Optional[str] = None): """ Create or update an operator subscription in a namespace. Automatically detects default channel and catalog source from PackageManifest if not provided. Ensures an OperatorGroup exists before creating the subscription. + When installPlanApproval is set to "Manual" and a startingCSV is specified, this function will + automatically approve the InstallPlan for the first-time installation to move to that startingCSV. + Subsequent upgrades will still require manual approval. + Parameters: dynClient (DynamicClient): OpenShift Dynamic Client namespace (str): The namespace to create the subscription in @@ -133,14 +138,20 @@ def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str catalogSourceNamespace (str, optional): Namespace of the catalog source. Defaults to "openshift-marketplace". config (dict, optional): Additional subscription configuration. Defaults to None. installMode (str, optional): Install mode for the OperatorGroup. Defaults to "OwnNamespace". + installPlanApproval (str, optional): Install plan approval mode ("Automatic" or "Manual"). Defaults to None. + startingCSV (str, optional): The specific CSV version to install. When combined with Manual approval, + the first InstallPlan to this CSV will be automatically approved. Required when installPlanApproval is "Manual". Defaults to None. Returns: Subscription: The created or updated subscription resource Raises: - OLMException: If the package is not available in any catalog + OLMException: If the package is not available in any catalog, or if installPlanApproval is "Manual" without a startingCSV NotFoundError: If resources cannot be created """ + # Validate that startingCSV is provided when installPlanApproval is Manual + if installPlanApproval == "Manual" and startingCSV is None: + raise OLMException("When installPlanApproval is 'Manual', a startingCSV must be provided") if catalogSourceNamespace is None: catalogSourceNamespace = "openshift-marketplace" @@ -190,7 +201,9 @@ def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str package_name=packageName, package_channel=packageChannel, catalog_name=catalogSource, - catalog_namespace=catalogSourceNamespace + catalog_namespace=catalogSourceNamespace, + install_plan_approval=installPlanApproval, + starting_csv=startingCSV ) subscription = yaml.safe_load(renderedTemplate) subscriptionsAPI.apply(body=subscription, namespace=namespace) @@ -199,6 +212,7 @@ def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str logger.debug(f"Waiting for {packageName}.{namespace} InstallPlans") installPlanAPI = dynClient.resources.get(api_version="operators.coreos.com/v1alpha1", kind="InstallPlan") + # Use label selector to get InstallPlans (standard approach) installPlanResources = installPlanAPI.get(label_selector=labelSelector, namespace=namespace) while len(installPlanResources.items) == 0: installPlanResources = installPlanAPI.get(label_selector=labelSelector, namespace=namespace) @@ -207,17 +221,93 @@ def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str if len(installPlanResources.items) == 0: raise OLMException(f"Found 0 InstallPlans for {packageName}") elif len(installPlanResources.items) > 1: - logger.warning(f"More than 1 InstallPlan found for {packageName}") + logger.warning(f"More than 1 InstallPlan found for {packageName} using label selector") + + # Select the InstallPlan to use + installPlanResource = None + + # Special handling for Manual approval with startingCSV + if installPlanApproval == "Manual" and startingCSV is not None: + logger.debug(f"Manual approval with startingCSV {startingCSV} - checking if label selector returned correct InstallPlan") + + # Check if any of the InstallPlans from label selector match the startingCSV + for plan in installPlanResources.items: + csvNames = getattr(plan.spec, "clusterServiceVersionNames", []) + logger.debug(f"InstallPlan {plan.metadata.name} (from label selector) contains CSVs: {csvNames}") + if csvNames and startingCSV in csvNames: + installPlanResource = plan + logger.info(f"Found InstallPlan {plan.metadata.name} matching startingCSV {startingCSV} via label selector") + break + + # If no match found via label selector, search all InstallPlans owned by this subscription + if installPlanResource is None: + logger.warning(f"Label selector did not return InstallPlan matching startingCSV {startingCSV}") + logger.debug(f"Searching all InstallPlans in {namespace} owned by subscription {name}") + + allInstallPlans = installPlanAPI.get(namespace=namespace) + for plan in allInstallPlans.items: + # Check if this InstallPlan is owned by our subscription + owner_refs = getattr(plan.metadata, 'ownerReferences', []) + is_owned_by_subscription = any( + ref.kind == "Subscription" and ref.name == name + for ref in owner_refs + ) + + if is_owned_by_subscription: + csvNames = getattr(plan.spec, "clusterServiceVersionNames", []) + logger.debug(f"InstallPlan {plan.metadata.name} (owned by subscription) contains CSVs: {csvNames}") + if csvNames and startingCSV in csvNames: + installPlanResource = plan + logger.info(f"Found InstallPlan {plan.metadata.name} matching startingCSV {startingCSV} via subscription ownership") + break + + if installPlanResource is None: + logger.warning(f"No InstallPlan found matching startingCSV {startingCSV}, using first from label selector") + installPlanResource = installPlanResources.items[0] else: - installPlanName = installPlanResources.items[0].metadata.name - - # Wait for InstallPlan to complete - logger.debug(f"Waiting for InstallPlan {installPlanName}") - installPlanPhase = installPlanResources.items[0].status.phase - while installPlanPhase != "Complete": - installPlanResource = installPlanAPI.get(name=installPlanName, namespace=namespace) - installPlanPhase = installPlanResource.status.phase - sleep(30) + # Standard case: use first InstallPlan from label selector + installPlanResource = installPlanResources.items[0] + + installPlanName = installPlanResource.metadata.name + installPlanPhase = installPlanResource.status.phase + + # If the InstallPlan for our startingCSV is already Complete, we're done + if installPlanPhase == "Complete": + logger.info(f"InstallPlan {installPlanName} for {startingCSV} is already Complete") + else: + # Wait for InstallPlan to complete + logger.debug(f"Waiting for InstallPlan {installPlanName}") + + # Track if we've already approved this install plan + approved_manual_install = False + + while installPlanPhase != "Complete": + installPlanResource = installPlanAPI.get(name=installPlanName, namespace=namespace) + installPlanPhase = installPlanResource.status.phase + + # If InstallPlan requires approval and this is the first installation to startingCSV + if installPlanPhase == "RequiresApproval" and not approved_manual_install: + # Check if this is the first installation by verifying the CSV matches startingCSV + if startingCSV is not None: + csvName = getattr(installPlanResource.spec, "clusterServiceVersionNames", []) + if csvName and startingCSV in csvName: + logger.info(f"Approving InstallPlan {installPlanName} for first-time installation to {startingCSV}") + # Patch the InstallPlan to approve it + installPlanResource.spec.approved = True + installPlanAPI.patch( + body=installPlanResource, + name=installPlanName, + namespace=namespace, + content_type="application/merge-patch+json" + ) + approved_manual_install = True + logger.info(f"InstallPlan {installPlanName} approved successfully") + else: + logger.debug(f"InstallPlan CSV {csvName} does not match startingCSV {startingCSV}, waiting for manual approval") + else: + logger.debug(f"No startingCSV specified, InstallPlan {installPlanName} requires manual approval") + + sleep(30) # Wait for Subscription to complete logger.debug(f"Waiting for Subscription {name} in {namespace}") @@ -225,9 +315,20 @@ def applySubscription(dynClient: DynamicClient, namespace: str, packageName: str subscriptionResource = subscriptionsAPI.get(name=name, namespace=namespace) state = getattr(subscriptionResource.status, "state", None) + # When manual approval is used with startingCSV, the state will be "UpgradePending" + # after the initial installation completes (indicating newer versions are available + # but require manual approval). For automatic approval, the state will be "AtLatestKnown". if state == "AtLatestKnown": logger.debug(f"Subscription {name} in {namespace} reached state: {state}") return subscriptionResource + elif state == "UpgradePending" and installPlanApproval == "Manual" and startingCSV is not None: + # Verify the installed CSV matches the startingCSV + installedCSV = getattr(subscriptionResource.status, "installedCSV", None) + if installedCSV == startingCSV: + logger.debug(f"Subscription {name} in {namespace} reached state: {state} with installedCSV: {installedCSV}") + return subscriptionResource + else: + logger.debug(f"Subscription {name} in {namespace} state is {state} but installedCSV ({installedCSV}) does not match startingCSV ({startingCSV}), retrying...") logger.debug(f"Subscription {name} in {namespace} not ready yet (state = {state}), retrying...") sleep(30) diff --git a/src/mas/devops/templates/subscription.yml.j2 b/src/mas/devops/templates/subscription.yml.j2 index e9439ece..23bb3a66 100644 --- a/src/mas/devops/templates/subscription.yml.j2 +++ b/src/mas/devops/templates/subscription.yml.j2 @@ -9,6 +9,12 @@ spec: channel: {{ package_channel }} source: {{ catalog_name }} sourceNamespace: {{ catalog_namespace }} + {%- if install_plan_approval is not none %} + installPlanApproval: {{ install_plan_approval }} + {%- endif %} + {%- if starting_csv is not none %} + startingCSV: {{ starting_csv }} + {%- endif %} {%- if subscription_config is not none %} config: {{ subscription_config }} {%- endif %} diff --git a/test/src/test_olm.py b/test/src/test_olm.py index 0e148388..354a9e2a 100644 --- a/test/src/test_olm.py +++ b/test/src/test_olm.py @@ -79,3 +79,89 @@ def test_crud_with_config(): olm.deleteSubscription(dynClient, namespace, "ibm-sls") olm.deleteSubscription(dynClient, namespace, "ibm-truststore-mgr") ocp.deleteNamespace(dynClient, namespace) + + +def test_crud_with_manual_approval(): + """ + Test that when installPlanApproval is Manual without a startingCSV, + an OLMException is raised. + """ + namespace = "cli-fvt-3" + + # This should raise an OLMException because Manual approval requires a startingCSV + try: + olm.applySubscription( + dynClient, + namespace, + "ibm-sls", + packageChannel="3.x", + installPlanApproval="Manual" + ) + # If we get here, the test should fail + assert False, "Expected OLMException to be raised when installPlanApproval is Manual without startingCSV" + except olm.OLMException as e: + # Verify the error message is correct + assert "When installPlanApproval is 'Manual', a startingCSV must be provided" in str(e) + # Test passed - exception was raised as expected + + +def test_crud_with_starting_csv(): + namespace = "cli-fvt-4" + # Note: This test assumes a specific CSV version exists in the catalog + # You may need to adjust the version based on what's available + subscription = olm.applySubscription( + dynClient, + namespace, + "ibm-sls", + packageChannel="3.x", + startingCSV="ibm-sls.v3.8.0" + ) + assert subscription.metadata.name == "ibm-sls" + assert subscription.metadata.namespace == namespace + assert subscription.spec.startingCSV == "ibm-sls.v3.8.0" + + # When we install the ibm-sls subscription OLM will automatically create the ibm-truststore-mgr + # subscription, but when we delete the subscription, OLM will not automatically remove the latter + olm.deleteSubscription(dynClient, namespace, "ibm-sls") + olm.deleteSubscription(dynClient, namespace, "ibm-truststore-mgr") + ocp.deleteNamespace(dynClient, namespace) + + +def test_crud_with_manual_approval_and_starting_csv(): + """ + Test that when installPlanApproval is Manual and startingCSV is specified, + the first InstallPlan is automatically approved to reach the startingCSV. + This allows the initial installation to proceed without manual intervention. + + Note: With Manual approval and startingCSV, the subscription state will be + "UpgradePending" after installation (indicating newer versions are available + but require manual approval), not "AtLatestKnown". + """ + namespace = "cli-fvt-5" + subscription = olm.applySubscription( + dynClient, + namespace, + "ibm-sls", + packageChannel="3.x", + installPlanApproval="Manual", + startingCSV="ibm-sls.v3.8.0" + ) + assert subscription.metadata.name == "ibm-sls" + assert subscription.metadata.namespace == namespace + assert subscription.spec.installPlanApproval == "Manual" + assert subscription.spec.startingCSV == "ibm-sls.v3.8.0" + + # Verify that the subscription reached UpgradePending state + # This confirms the InstallPlan was automatically approved and installed + # UpgradePending indicates newer versions are available but require manual approval + assert subscription.status.state == "UpgradePending" + + # Verify the installed CSV matches the startingCSV + installedCSV = subscription.status.installedCSV + assert installedCSV == "ibm-sls.v3.8.0" + + # When we install the ibm-sls subscription OLM will automatically create the ibm-truststore-mgr + # subscription, but when we delete the subscription, OLM will not automatically remove the latter + olm.deleteSubscription(dynClient, namespace, "ibm-sls") + olm.deleteSubscription(dynClient, namespace, "ibm-truststore-mgr") + ocp.deleteNamespace(dynClient, namespace) diff --git a/test/src/test_olm_installplan_selection.py b/test/src/test_olm_installplan_selection.py new file mode 100644 index 00000000..1101742c --- /dev/null +++ b/test/src/test_olm_installplan_selection.py @@ -0,0 +1,535 @@ +# ***************************************************************************** +# Copyright (c) 2024 IBM Corporation and other Contributors. +# +# All rights reserved. This program and the accompanying materials +# are made available under the terms of the Eclipse Public License v1.0 +# which accompanies this distribution, and is available at +# http://www.eclipse.org/legal/epl-v10.html +# +# ***************************************************************************** + +""" +Unit tests for InstallPlan selection logic in applySubscription. + +These tests mock up the resources rather than use real resources as the +OLM processing on a cluster is quite slow and it would take time to +exercise all these scenerios +""" + +import pytest +from unittest.mock import Mock, patch +from mas.devops import olm + + +class MockResource: + """Mock Kubernetes resource object""" + + def __init__(self, name, labels=None, owner_refs=None, csv_names=None, phase="Complete"): + self.metadata = Mock() + self.metadata.name = name + self.metadata.labels = labels or {} + self.metadata.ownerReferences = owner_refs or [] + + self.spec = Mock() + self.spec.clusterServiceVersionNames = csv_names or [] + + self.status = Mock() + self.status.phase = phase + + +class MockResourceList: + """Mock Kubernetes resource list""" + + def __init__(self, items): + self.items = items + # Add status attribute to match real ResourceList behavior + self.status = Mock() + + +@pytest.fixture +def mock_dyn_client(): + """Create a mock DynamicClient""" + client = Mock() + return client + + +@pytest.fixture +def mock_env(): + """Create a mock Jinja2 Environment""" + env = Mock() + template = Mock() + template.render.return_value = """ +apiVersion: operators.coreos.com/v1alpha1 +kind: Subscription +metadata: + name: test-operator + namespace: test-namespace +spec: + channel: stable + name: test-operator + source: test-catalog + sourceNamespace: openshift-marketplace +""" + env.get_template.return_value = template + return env + + +def create_owner_ref(kind, name): + """Helper to create an owner reference""" + ref = Mock() + ref.kind = kind + ref.name = name + return ref + + +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_automatic_approval_uses_label_selector_only( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_dyn_client, mock_env +): + """ + Test that automatic approval uses only the label selector (standard behavior). + Should NOT query all InstallPlans. + """ + # Setup mocks + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + # Mock subscription API + sub_api = Mock() + + # Mock subscription resource with proper status + mock_subscription = Mock() + mock_subscription.metadata.name = "test-operator" + mock_subscription.status = Mock() + mock_subscription.status.state = "AtLatestKnown" + mock_subscription.status.installedCSV = "test-operator.v1.0.0" + + # First call returns empty list (no existing subscription), subsequent calls return the subscription + sub_api.get.side_effect = [ + MockResourceList([]), # Initial check for existing subscription + mock_subscription # Subsequent calls when waiting for subscription to complete + ] + sub_api.apply.return_value = Mock() + + # Mock InstallPlan API - label selector returns one InstallPlan + install_plan_api = Mock() + install_plan = MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + install_plan_api.get.return_value = MockResourceList([install_plan]) + + # Setup resource API + mock_dyn_client.resources.get.side_effect = lambda **kwargs: { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }.get((kwargs.get("api_version"), kwargs.get("kind"))) + + with patch('mas.devops.olm.Environment', return_value=mock_env): + # Call applySubscription with Automatic approval (default) + olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable", + installPlanApproval="Automatic" + ) + + # Verify InstallPlan API was called with label selector only + install_plan_calls = [c for c in install_plan_api.get.call_args_list] + + # Should only use label selector, never query all InstallPlans + for call_args in install_plan_calls: + args, kwargs = call_args + assert 'label_selector' in kwargs, "Should use label selector" + assert kwargs.get('namespace') == "test-namespace" + + +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_manual_approval_without_starting_csv_uses_label_selector_only( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_dyn_client, mock_env +): + """ + Test that Manual approval with startingCSV uses only label selector when it finds a match. + Should NOT query all InstallPlans. + """ + # Setup mocks + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + # Mock subscription API + sub_api = Mock() + + # Mock subscription resource with proper status + mock_subscription = Mock() + mock_subscription.metadata.name = "test-operator" + mock_subscription.status = Mock() + mock_subscription.status.state = "UpgradePending" + mock_subscription.status.installedCSV = "test-operator.v1.0.0" + + # First call returns empty list (no existing subscription), subsequent calls return the subscription + sub_api.get.side_effect = [ + MockResourceList([]), # Initial check for existing subscription + mock_subscription # Subsequent calls when waiting for subscription to complete + ] + sub_api.apply.return_value = Mock() + + # Mock InstallPlan API + install_plan_api = Mock() + install_plan_requires_approval = MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], + phase="RequiresApproval" + ) + install_plan_complete = MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + + # Simulate phase transition: first returns RequiresApproval, then Complete after patch + def get_install_plan_side_effect(*args, **kwargs): + if 'name' in kwargs: + # After patch is called, return Complete phase + return install_plan_complete + else: + # Initial query with label selector + return MockResourceList([install_plan_requires_approval]) + + install_plan_api.get.side_effect = get_install_plan_side_effect + install_plan_api.patch.return_value = Mock() + + mock_dyn_client.resources.get.side_effect = lambda **kwargs: { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }.get((kwargs.get("api_version"), kwargs.get("kind"))) + + with patch('mas.devops.olm.Environment', return_value=mock_env): + # Call with Manual approval with startingCSV + olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable", + installPlanApproval="Manual", + startingCSV="test-operator.v1.0.0" + ) + + # Verify only label selector was used + install_plan_calls = [c for c in install_plan_api.get.call_args_list] + for call_args in install_plan_calls: + args, kwargs = call_args + # Should only use label selector or get by name, never query all + assert 'label_selector' in kwargs or 'name' in kwargs + + +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_manual_approval_with_starting_csv_label_selector_finds_match( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_dyn_client, mock_env +): + """ + Test Manual approval with startingCSV when label selector returns the correct InstallPlan. + Should use label selector result, NOT query all InstallPlans. + """ + # Setup mocks + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + # Mock subscription API + sub_api = Mock() + + # Mock subscription resource with proper status + mock_subscription = Mock() + mock_subscription.metadata.name = "test-operator" + mock_subscription.status = Mock() + mock_subscription.status.state = "UpgradePending" + mock_subscription.status.installedCSV = "test-operator.v1.0.0" + + # First call returns empty list (no existing subscription), subsequent calls return the subscription + sub_api.get.side_effect = [ + MockResourceList([]), # Initial check for existing subscription + mock_subscription # Subsequent calls when waiting for subscription to complete + ] + sub_api.apply.return_value = Mock() + + # Mock InstallPlan API - label selector returns matching InstallPlan + install_plan_api = Mock() + install_plan_requires_approval = MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], # Matches startingCSV + phase="RequiresApproval" + ) + install_plan_complete = MockResource( + name="install-plan-1", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + + # Simulate phase transition + def get_install_plan_side_effect(*args, **kwargs): + if 'name' in kwargs: + return install_plan_complete + else: + return MockResourceList([install_plan_requires_approval]) + + install_plan_api.get.side_effect = get_install_plan_side_effect + install_plan_api.patch.return_value = Mock() + + mock_dyn_client.resources.get.side_effect = lambda **kwargs: { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }.get((kwargs.get("api_version"), kwargs.get("kind"))) + + with patch('mas.devops.olm.Environment', return_value=mock_env): + olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable", + installPlanApproval="Manual", + startingCSV="test-operator.v1.0.0" + ) + + # Verify we found the InstallPlan via label selector + # Should NOT have queried all InstallPlans (no call without label_selector) + install_plan_calls = [c for c in install_plan_api.get.call_args_list] + + # Check that we never queried without a label_selector or name + for call_args in install_plan_calls: + args, kwargs = call_args + assert 'label_selector' in kwargs or 'name' in kwargs, \ + "Should only use label selector or get by name, not query all" + + +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_manual_approval_with_starting_csv_fallback_to_ownership_search( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_dyn_client, mock_env +): + """ + Test Manual approval with startingCSV when label selector misses the completed InstallPlan. + Should fall back to querying all InstallPlans and filter by subscription ownership. + This is the key scenario the bug fix addresses. + """ + # Setup mocks + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + # Mock subscription API + sub_api = Mock() + + # Mock subscription resource with proper status + mock_subscription = Mock() + mock_subscription.metadata.name = "test-operator" + mock_subscription.status = Mock() + mock_subscription.status.state = "UpgradePending" + mock_subscription.status.installedCSV = "test-operator.v1.0.0" + + # First call returns empty list (no existing subscription), subsequent calls return the subscription + sub_api.get.side_effect = [ + MockResourceList([]), # Initial check for existing subscription + mock_subscription # Subsequent calls when waiting for subscription to complete + ] + sub_api.apply.return_value = Mock() + + # Mock InstallPlan API + install_plan_api = Mock() + + # Label selector returns only the in-progress InstallPlan (wrong one) + wrong_install_plan = MockResource( + name="install-plan-2", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v2.0.0"], # Does NOT match startingCSV + phase="Installing" + ) + + # All InstallPlans query returns both (including the completed one) + correct_install_plan_requires_approval = MockResource( + name="install-plan-1", + labels={}, # Label might be removed from completed plan + owner_refs=[create_owner_ref("Subscription", "test-operator")], + csv_names=["test-operator.v1.0.0"], # Matches startingCSV + phase="RequiresApproval" + ) + + correct_install_plan_complete = MockResource( + name="install-plan-1", + labels={}, + owner_refs=[create_owner_ref("Subscription", "test-operator")], + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + + # Setup the mock to return different results based on parameters + def get_side_effect(*args, **kwargs): + if 'label_selector' in kwargs: + # Label selector query - returns only wrong InstallPlan + return MockResourceList([wrong_install_plan]) + elif 'name' in kwargs: + # Get by name - return the correct one (Complete after patch) + return correct_install_plan_complete + else: + # Query all InstallPlans - returns both + return MockResourceList([correct_install_plan_requires_approval, wrong_install_plan]) + + install_plan_api.get.side_effect = get_side_effect + install_plan_api.patch.return_value = Mock() + + mock_dyn_client.resources.get.side_effect = lambda **kwargs: { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }.get((kwargs.get("api_version"), kwargs.get("kind"))) + + with patch('mas.devops.olm.Environment', return_value=mock_env): + olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable", + installPlanApproval="Manual", + startingCSV="test-operator.v1.0.0" + ) + + # Verify the fallback behavior occurred + install_plan_calls = [c for c in install_plan_api.get.call_args_list] + + # Should have: + # 1. Called with label_selector (initial query) + # 2. Called without label_selector (fallback to query all) + has_label_selector_call = any( + 'label_selector' in call_args[1] + for call_args in install_plan_calls + ) + has_all_query_call = any( + 'label_selector' not in call_args[1] and 'name' not in call_args[1] + for call_args in install_plan_calls + ) + + assert has_label_selector_call, "Should have tried label selector first" + assert has_all_query_call, "Should have fallen back to querying all InstallPlans" + + +@patch('mas.devops.olm.createNamespace') +@patch('mas.devops.olm.ensureOperatorGroupExists') +@patch('mas.devops.olm.getPackageManifest') +@patch('mas.devops.olm.sleep') +def test_manual_approval_filters_by_subscription_ownership( + mock_sleep, mock_get_manifest, mock_ensure_og, mock_create_ns, mock_dyn_client, mock_env +): + """ + Test that when querying all InstallPlans, we correctly filter by subscription ownership. + This ensures we don't accidentally use InstallPlans from other subscriptions. + """ + # Setup mocks + mock_get_manifest.return_value = Mock( + status=Mock(defaultChannel="stable", catalogSource="test-catalog") + ) + + # Mock subscription API + sub_api = Mock() + + # Mock subscription resource with proper status + mock_subscription = Mock() + mock_subscription.metadata.name = "test-operator" + mock_subscription.status = Mock() + mock_subscription.status.state = "UpgradePending" + mock_subscription.status.installedCSV = "test-operator.v1.0.0" + + # First call returns empty list (no existing subscription), subsequent calls return the subscription + sub_api.get.side_effect = [ + MockResourceList([]), # Initial check for existing subscription + mock_subscription # Subsequent calls when waiting for subscription to complete + ] + sub_api.apply.return_value = Mock() + + # Mock InstallPlan API + install_plan_api = Mock() + + # Label selector returns wrong InstallPlan + wrong_install_plan = MockResource( + name="install-plan-wrong", + labels={"operators.coreos.com/test-operator.test-namespace": ""}, + csv_names=["test-operator.v2.0.0"], + phase="Installing" + ) + + # All InstallPlans includes: + # 1. Correct one owned by our subscription + correct_install_plan_requires_approval = MockResource( + name="install-plan-correct", + labels={}, + owner_refs=[create_owner_ref("Subscription", "test-operator")], + csv_names=["test-operator.v1.0.0"], + phase="RequiresApproval" + ) + + correct_install_plan_complete = MockResource( + name="install-plan-correct", + labels={}, + owner_refs=[create_owner_ref("Subscription", "test-operator")], + csv_names=["test-operator.v1.0.0"], + phase="Complete" + ) + + # 2. One owned by a different subscription (should be ignored) + other_subscription_plan = MockResource( + name="install-plan-other", + labels={}, + owner_refs=[create_owner_ref("Subscription", "other-operator")], + csv_names=["test-operator.v1.0.0"], # Same CSV but wrong subscription + phase="Complete" + ) + + def get_side_effect(*args, **kwargs): + if 'label_selector' in kwargs: + return MockResourceList([wrong_install_plan]) + elif 'name' in kwargs: + return correct_install_plan_complete + else: + # Return all three InstallPlans + return MockResourceList([correct_install_plan_requires_approval, other_subscription_plan, wrong_install_plan]) + + install_plan_api.get.side_effect = get_side_effect + install_plan_api.patch.return_value = Mock() + + mock_dyn_client.resources.get.side_effect = lambda **kwargs: { + ("operators.coreos.com/v1alpha1", "Subscription"): sub_api, + ("operators.coreos.com/v1alpha1", "InstallPlan"): install_plan_api, + }.get((kwargs.get("api_version"), kwargs.get("kind"))) + + with patch('mas.devops.olm.Environment', return_value=mock_env): + olm.applySubscription( + mock_dyn_client, + "test-namespace", + "test-operator", + packageChannel="stable", + installPlanApproval="Manual", + startingCSV="test-operator.v1.0.0" + ) + + # The test passes if it completes without error + # The code should have found the correct InstallPlan by checking ownership + # and ignored the one from the other subscription + +# Made with Bob