From 628d3f276d88af8f682d3207ad763d335741305f Mon Sep 17 00:00:00 2001 From: Salman Rana Date: Tue, 19 Sep 2023 08:07:18 -0400 Subject: [PATCH] Add sw version validation to all release param endpoints Incorporate the existing release version validation with all the dcmanager operations/commands endpoints that allow the user to specify a release version: - subcloud add - subcloud prestage - subcloud redeploy - subcloud deploy show - subcloud deploy resume - subcloud deploy create - subcloud deploy install - prestage-strategy create - subcloud-backup restore Please note that the "subcloud deploy upload" release validation was addressed in a previous change [1]. These changes add the validate_release_version_supported check (previously introduced [1]) to all the endpoints that consume a release parameter. This is done to check whether a specified release version is supported by the current active version. [1] https://review.opendev.org/c/starlingx/distcloud/+/891911 Test Plan: For each command listed above, test the release parameter (--release): 1. PASS: Verify that the current active version is accepted as a valid parameter. 2. PASS: Verify that all the supported upgrade versions in /usr/rootdirs/opt/upgrades/metadata.xml are accepted as a valid release parameter. 3. PASS: Verify that any upgrade version that's not included in metadata.xml is rejected with an error " is not a supported release version". The only exception to this is the current active version, it must always be valid. 4. PASS: Delete all the "supported_upgrades" elements in metadata.xml and verify the error "Unable to validate the release version" is printed. 5. PASS: Delete metadata.xml and verify that "Unable to validate the release version" error is printed. 6. PASS: Verify that the current active version is valid regardless of metadata.xml file and its contents. 7. PASS: Exclude the release parameter and verify that the command is successful (completed with the current active version). Closes-Bug: 2036479 Change-Id: I1608a68ce6863f51dc0b90e0a6f6b9b588e85689 Signed-off-by: Salman Rana --- .../controllers/v1/phased_subcloud_deploy.py | 14 ++++-- .../api/controllers/v1/subcloud_backup.py | 4 +- .../api/controllers/v1/subcloud_deploy.py | 24 ++-------- .../dcmanager/api/controllers/v1/subclouds.py | 10 ++-- .../api/controllers/v1/sw_update_strategy.py | 6 ++- .../common/phased_subcloud_deploy.py | 4 +- distributedcloud/dcmanager/common/utils.py | 27 ++++++++++- .../test_phased_subcloud_deploy.py | 8 ++-- .../v1/controllers/test_subcloud_backup.py | 8 ++-- .../v1/controllers/test_subcloud_deploy.py | 16 +++---- .../unit/api/v1/controllers/test_subclouds.py | 48 +++++++++++-------- .../tests/unit/common/fake_subcloud.py | 8 ++++ 12 files changed, 107 insertions(+), 70 deletions(-) diff --git a/distributedcloud/dcmanager/api/controllers/v1/phased_subcloud_deploy.py b/distributedcloud/dcmanager/api/controllers/v1/phased_subcloud_deploy.py index bc72167cb..e4cd27028 100644 --- a/distributedcloud/dcmanager/api/controllers/v1/phased_subcloud_deploy.py +++ b/distributedcloud/dcmanager/api/controllers/v1/phased_subcloud_deploy.py @@ -206,7 +206,11 @@ class PhasedSubcloudDeployController(object): pecan.abort(400, _('The deploy install command can only be used ' 'during initial deployment.')) - payload['software_version'] = payload.get('release', subcloud.software_version) + unvalidated_sw_version = payload.get('release', subcloud.software_version) + # get_sw_version will simply return back + # the passed unvalidated_sw_version after validating it. + payload['software_version'] = utils.get_sw_version(unvalidated_sw_version) + psd_common.populate_payload_with_pre_existing_data( payload, subcloud, SUBCLOUD_INSTALL_GET_FILE_CONTENTS) @@ -426,11 +430,15 @@ class PhasedSubcloudDeployController(object): # Consider the incoming release parameter only if install is one # of the pending deploy states if INSTALL in deploy_states_to_run: - payload['software_version'] = payload.get('release', subcloud.software_version) + unvalidated_sw_version = payload.get('release', subcloud.software_version) else: LOG.debug('Disregarding release parameter for %s as installation is complete.' % subcloud.name) - payload['software_version'] = subcloud.software_version + unvalidated_sw_version = subcloud.software_version + + # get_sw_version will simply return back the passed + # unvalidated_sw_version after validating it. + payload['software_version'] = utils.get_sw_version(unvalidated_sw_version) # Need to remove bootstrap_values from the list of files to populate # pre existing data so it does not overwrite newly loaded values diff --git a/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py b/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py index 5503c14ba..de295b84a 100644 --- a/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py +++ b/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py @@ -15,7 +15,6 @@ import pecan from pecan import expose from pecan import request as pecan_request from pecan import response -import tsconfig.tsconfig as tsc import yaml from dcmanager.api.controllers import restcomm @@ -370,7 +369,8 @@ class SubcloudBackupController(object): if payload.get('with_install'): # Confirm the requested or active load is still in dc-vault - payload['software_version'] = payload.get('release', tsc.SW_VERSION) + payload['software_version'] = utils.get_sw_version( + payload.get('release')) matching_iso, err_msg = utils.get_matching_iso(payload['software_version']) if err_msg: LOG.exception(err_msg) diff --git a/distributedcloud/dcmanager/api/controllers/v1/subcloud_deploy.py b/distributedcloud/dcmanager/api/controllers/v1/subcloud_deploy.py index 4f051fad0..6025debbb 100644 --- a/distributedcloud/dcmanager/api/controllers/v1/subcloud_deploy.py +++ b/distributedcloud/dcmanager/api/controllers/v1/subcloud_deploy.py @@ -32,12 +32,9 @@ from dcmanager.api.controllers import restcomm from dcmanager.api.policies import subcloud_deploy as subcloud_deploy_policy from dcmanager.api import policy from dcmanager.common import consts -from dcmanager.common import exceptions from dcmanager.common.i18n import _ from dcmanager.common import utils -import tsconfig.tsconfig as tsc - CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -106,20 +103,9 @@ class SubcloudDeployController(object): error_msg = "error: argument %s is required" % missing_str.rstrip() pecan.abort(httpclient.BAD_REQUEST, error_msg) - software_version = tsc.SW_VERSION - if request.POST.get('release'): - try: - utils.validate_release_version_supported(request.POST.get('release')) - software_version = request.POST.get('release') - except exceptions.ValidateFail as e: - pecan.abort(httpclient.BAD_REQUEST, - _("Error: invalid release version parameter. %s" % e)) - except Exception: - pecan.abort(httpclient.INTERNAL_SERVER_ERROR, - _('Error: unable to validate the release version.')) - deploy_dicts['software_version'] = software_version + deploy_dicts['software_version'] = utils.get_sw_version(request.POST.get('release')) - dir_path = os.path.join(dccommon_consts.DEPLOY_DIR, software_version) + dir_path = os.path.join(dccommon_consts.DEPLOY_DIR, deploy_dicts['software_version']) for f in consts.DEPLOY_COMMON_FILE_OPTIONS: if f not in request.POST: continue @@ -152,10 +138,8 @@ class SubcloudDeployController(object): policy.authorize(subcloud_deploy_policy.POLICY_ROOT % "get", {}, restcomm.extract_credentials_for_policy()) deploy_dicts = dict() - if not release: - release = tsc.SW_VERSION - deploy_dicts['software_version'] = release - dir_path = os.path.join(dccommon_consts.DEPLOY_DIR, release) + deploy_dicts['software_version'] = utils.get_sw_version(release) + dir_path = os.path.join(dccommon_consts.DEPLOY_DIR, deploy_dicts['software_version']) for f in consts.DEPLOY_COMMON_FILE_OPTIONS: filename = None if os.path.isdir(dir_path): diff --git a/distributedcloud/dcmanager/api/controllers/v1/subclouds.py b/distributedcloud/dcmanager/api/controllers/v1/subclouds.py index 9c06022bd..89826431e 100644 --- a/distributedcloud/dcmanager/api/controllers/v1/subclouds.py +++ b/distributedcloud/dcmanager/api/controllers/v1/subclouds.py @@ -41,8 +41,6 @@ from dccommon import exceptions as dccommon_exceptions from keystoneauth1 import exceptions as keystone_exceptions -import tsconfig.tsconfig as tsc - from dcmanager.api.controllers import restcomm from dcmanager.api.policies import subclouds as subclouds_policy from dcmanager.api import policy @@ -748,9 +746,7 @@ class SubcloudsController(object): LOG.warning(msg) pecan.abort(400, msg) - # If a subcloud release is not passed, use the current - # system controller software_version - payload['software_version'] = payload.get('release', tsc.SW_VERSION) + payload['software_version'] = utils.get_sw_version(payload.get('release')) # Don't load previously stored bootstrap_values if they are present in # the request, as this would override the already loaded values from it. @@ -831,8 +827,8 @@ class SubcloudsController(object): LOG.exception("validate_prestage failed") pecan.abort(400, _(str(exc))) - prestage_software_version = payload.get( - consts.PRESTAGE_REQUEST_RELEASE, tsc.SW_VERSION) + prestage_software_version = utils.get_sw_version( + payload.get(consts.PRESTAGE_REQUEST_RELEASE)) try: self.dcmanager_rpc_client.prestage_subcloud(context, payload) diff --git a/distributedcloud/dcmanager/api/controllers/v1/sw_update_strategy.py b/distributedcloud/dcmanager/api/controllers/v1/sw_update_strategy.py index af3862c0e..d90e67336 100755 --- a/distributedcloud/dcmanager/api/controllers/v1/sw_update_strategy.py +++ b/distributedcloud/dcmanager/api/controllers/v1/sw_update_strategy.py @@ -1,5 +1,5 @@ # Copyright (c) 2017 Ericsson AB. -# Copyright (c) 2017-2022 Wind River Systems, Inc. +# Copyright (c) 2017-2023 Wind River Systems, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -205,6 +205,10 @@ class SwUpdateStrategyController(object): if group is None: pecan.abort(400, _('Invalid group_id')) + # get_sw_version is used here to validate the + # release parameter if specified. + utils.get_sw_version(payload.get('release')) + # Not adding validation for extra args. Passing them through. try: # Ask dcmanager-manager to create the strategy. diff --git a/distributedcloud/dcmanager/common/phased_subcloud_deploy.py b/distributedcloud/dcmanager/common/phased_subcloud_deploy.py index 9a7fa6378..f17727a6c 100644 --- a/distributedcloud/dcmanager/common/phased_subcloud_deploy.py +++ b/distributedcloud/dcmanager/common/phased_subcloud_deploy.py @@ -972,9 +972,7 @@ def pre_deploy_create(payload: dict, context: RequestContext, validate_bootstrap_values(payload) - # If a subcloud release is not passed, use the current - # system controller software_version - payload['software_version'] = payload.get('release', tsc.SW_VERSION) + payload['software_version'] = utils.get_sw_version(payload.get('release')) validate_subcloud_name_availability(context, payload['name']) diff --git a/distributedcloud/dcmanager/common/utils.py b/distributedcloud/dcmanager/common/utils.py index d06872d89..c1dccbbfa 100644 --- a/distributedcloud/dcmanager/common/utils.py +++ b/distributedcloud/dcmanager/common/utils.py @@ -21,6 +21,7 @@ import itertools import json import netaddr import os +import pecan import pwd import re import resource as sys_resource @@ -46,6 +47,7 @@ from dccommon import exceptions as dccommon_exceptions from dccommon import kubeoperator from dcmanager.common import consts from dcmanager.common import exceptions +from dcmanager.common.i18n import _ from dcmanager.db import api as db_api LOG = logging.getLogger(__name__) @@ -1225,6 +1227,28 @@ def create_subcloud_rehome_data_template(): return {'saved_payload': {}} +def get_sw_version(release=None): + """Get the sw_version to be used. + + Return the sw_version by first validating a set release version. + If a release is not specified then use the current system controller + software_version. + """ + + if release: + try: + validate_release_version_supported(release) + return release + except exceptions.ValidateFail as e: + pecan.abort(400, + _("Error: invalid release version parameter. %s" % e)) + except Exception: + pecan.abort(500, + _('Error: unable to validate the release version.')) + else: + return tsc.SW_VERSION + + def validate_release_version_supported(release_version_to_check): """Given a release version, check whether it's supported by the current active version. @@ -1242,7 +1266,8 @@ def validate_release_version_supported(release_version_to_check): supported_versions = get_current_supported_upgrade_versions() if release_version_to_check not in supported_versions: - msg = "%s is not a supported release version" % release_version_to_check + msg = "%s is not a supported release version (%s)" % \ + (release_version_to_check, ",".join(supported_versions)) raise exceptions.ValidateFail(msg) return True diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py index ffb3a30f1..ce25d35af 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py @@ -404,9 +404,11 @@ class TestSubcloudDeployInstall(testroot.DCManagerApiTest): self.mock_rpc_client().subcloud_deploy_install.return_value = True self.mock_get_vault_load_files.return_value = ('iso_file_path', 'sig_file_path') - response = self.app.patch_json( - FAKE_URL + '/' + str(subcloud.id) + '/install', - headers=FAKE_HEADERS, params=install_payload) + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.patch_json( + FAKE_URL + '/' + str(subcloud.id) + '/install', + headers=FAKE_HEADERS, params=install_payload) self.assertEqual(response.status_int, 200) self.assertEqual(consts.DEPLOY_STATE_PRE_INSTALL, diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py index ccf78a804..5900d5595 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py @@ -1272,9 +1272,11 @@ class TestSubcloudRestore(testroot.DCManagerApiTest): mock_listdir.return_value = ['test.iso', 'test.sig'] mock_rpc_client().restore_subcloud_backups.return_value = True - response = self.app.patch_json(FAKE_URL_RESTORE, - headers=FAKE_HEADERS, - params=data) + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.patch_json(FAKE_URL_RESTORE, + headers=FAKE_HEADERS, + params=data) self.assertEqual(response.status_int, 200) diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_deploy.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_deploy.py index 695ae42eb..201b0c6d4 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_deploy.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_deploy.py @@ -25,6 +25,7 @@ from dcmanager.common import consts from dcmanager.common import phased_subcloud_deploy as psd_common from dcmanager.common import utils as dutils from dcmanager.tests.unit.api import test_root_controller as testroot +from dcmanager.tests.unit.common import fake_subcloud from dcmanager.tests import utils from tsconfig.tsconfig import SW_VERSION @@ -48,12 +49,6 @@ FAKE_DEPLOY_FILES = { FAKE_DEPLOY_CHART_PREFIX: FAKE_DEPLOY_CHART_FILE, } -FAKE_UPGRADES_METADATA = ''' - \n0.2\n - \n\n%s\nPATCH_0001 - \n\n\n -''' % FAKE_SOFTWARE_VERSION - class TestSubcloudDeploy(testroot.DCManagerApiTest): def setUp(self): @@ -72,7 +67,8 @@ class TestSubcloudDeploy(testroot.DCManagerApiTest): mock_upload_files.return_value = True params += fields - with mock.patch('builtins.open', mock.mock_open(read_data=FAKE_UPGRADES_METADATA)): + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): response = self.app.post(FAKE_URL, headers=FAKE_HEADERS, params=params) @@ -218,7 +214,11 @@ class TestSubcloudDeploy(testroot.DCManagerApiTest): mock_get_filename_by_prefix.side_effect = \ get_filename_by_prefix_side_effect url = FAKE_URL + '/' + FAKE_SOFTWARE_VERSION - response = self.app.get(url, headers=FAKE_HEADERS) + + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.get(url, headers=FAKE_HEADERS) + self.assertEqual(response.status_code, http_client.OK) self.assertEqual(FAKE_SOFTWARE_VERSION, response.json['subcloud_deploy']['software_version']) diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py index 4bac9c6d7..c06077b99 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py @@ -554,11 +554,13 @@ class TestSubcloudPost(testroot.DCManagerApiTest, base64.b64encode('fake pass'.encode("utf-8")).decode("utf-8"), 'release': '21.12'}) - response = self.app.post(self.get_api_prefix(), - params=params, - upload_files=upload_files, - headers=self.get_api_headers(), - expect_errors=True) + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.post(self.get_api_prefix(), + params=params, + upload_files=upload_files, + headers=self.get_api_headers(), + expect_errors=True) # Verify the request was rejected self.assertEqual(response.status_code, http_client.BAD_REQUEST) @@ -585,11 +587,13 @@ class TestSubcloudPost(testroot.DCManagerApiTest, base64.b64encode('fake pass'.encode("utf-8")).decode("utf-8"), 'release': software_version}) - response = self.app.post(self.get_api_prefix(), - params=params, - upload_files=upload_files, - headers=self.get_api_headers(), - expect_errors=True) + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.post(self.get_api_prefix(), + params=params, + upload_files=upload_files, + headers=self.get_api_headers(), + expect_errors=True) self.assertEqual(response.status_code, http_client.OK) self.assertEqual(software_version, response.json['software-version']) @@ -719,11 +723,15 @@ class TestSubcloudPost(testroot.DCManagerApiTest, self.set_list_of_post_files(subclouds.SUBCLOUD_ADD_GET_FILE_CONTENTS) self.install_data = copy.copy(self.FAKE_INSTALL_DATA) upload_files = self.get_post_upload_files() - response = self.app.post(self.get_api_prefix(), - params=params, - upload_files=upload_files, - headers=self.get_api_headers(), - expect_errors=True) + + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.post(self.get_api_prefix(), + params=params, + upload_files=upload_files, + headers=self.get_api_headers(), + expect_errors=True) + self.assertEqual(response.status_code, http_client.BAD_REQUEST) # Revert the change of bootstrap_data @@ -1689,10 +1697,12 @@ class TestSubcloudAPIOther(testroot.DCManagerApiTest): ("deploy_config", "config_fake_filename", json.dumps(config_data).encode("utf-8"))] - response = self.app.patch( - FAKE_URL + '/' + str(subcloud.id) + '/redeploy', - headers=FAKE_HEADERS, params=redeploy_data, - upload_files=upload_files) + with mock.patch('builtins.open', + mock.mock_open(read_data=fake_subcloud.FAKE_UPGRADES_METADATA)): + response = self.app.patch( + FAKE_URL + '/' + str(subcloud.id) + '/redeploy', + headers=FAKE_HEADERS, params=redeploy_data, + upload_files=upload_files) mock_validate_bootstrap_values.assert_called_once() mock_validate_subcloud_config.assert_called_once() diff --git a/distributedcloud/dcmanager/tests/unit/common/fake_subcloud.py b/distributedcloud/dcmanager/tests/unit/common/fake_subcloud.py index 37918fe4b..6cbe37575 100644 --- a/distributedcloud/dcmanager/tests/unit/common/fake_subcloud.py +++ b/distributedcloud/dcmanager/tests/unit/common/fake_subcloud.py @@ -116,6 +116,14 @@ FAKE_SUBCLOUD_INSTALL_VALUES_WITH_PERSISTENT_SIZE = { "persistent_size": 40000, } +FAKE_UPGRADES_METADATA = ''' + \n0.1\n + \n\n%s\n + \n\n21.12\n + \n\n22.12\n + \n\n +''' % FAKE_SOFTWARE_VERSION + def create_fake_subcloud(ctxt, **kwargs): values = {