diff --git a/CHANGELOG.md b/CHANGELOG.md index 349a4275..8eee05a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.29.0] - 2024-10-01 +### Added +- Run `pylint` during builds +- When validating boot sets, if a boot artifact is missing from a manifest, include the manifest S3 URL + in the associated message. +- Add check that at least one hardware-specifier field is non-empty to `validate_sanitize_boot_sets` +- When validating boot sets (at session template patch/creation/validation or session creation), + attempt to validate that the specified architecture matches that of the corresponding IMS image. + Mismatches are fatal errors. + +### Changed +- Marked PATCH session status endpoint to be ignored by the CLI. +- Eliminate redundancies in API spec by defining `V2SessionNameOrEmpty` schema. +- Refactor `validate_boot_sets` function into multiple functions to make it easier to understand. +- During migration to this BOS version, for boot sets with no `arch` field, add the field with its + default value. + +### Fixed +- When validating boot sets, check all boot sets for severe errors before returning only warnings + ## [2.28.0] - 2024-09-11 ### Added - Added `reject_nids` BOS option, to reject Sessions and Session Template which appear to reference NIDs. diff --git a/Dockerfile b/Dockerfile index 47bae2d8..afc281cd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -79,7 +79,7 @@ RUN --mount=type=secret,id=netrc,target=/root/.netrc \ pip3 list --format freeze && \ python3 -m convert_oas30_schemas /app/openapi.json /app/lib/bos/server/openapi.jsonschema && \ cat /app/lib/bos/server/openapi.jsonschema - +RUN pylint /app/convert_oas/convert_oas.py || true # Base image FROM alpine-base AS base @@ -120,11 +120,30 @@ RUN --mount=type=secret,id=netrc,target=/root/.netrc \ FROM base AS testing WORKDIR /app COPY test-requirements.txt . -RUN --mount=type=secret,id=netrc,target=/root/.netrc cd /app && \ +RUN --mount=type=secret,id=netrc,target=/root/.netrc \ + cd /app && \ pip3 install --no-cache-dir -r test-requirements.txt && \ pip3 list --format freeze +# Pylint reporting +FROM base AS pylint-base +COPY srclist.txt docker_pylint.sh /app/venv/ +WORKDIR /app +RUN --mount=type=secret,id=netrc,target=/root/.netrc \ + pip3 install --no-cache-dir pylint -c constraints.txt && \ + pip3 list --format freeze + +# Pylint errors-only +FROM pylint-base AS pylint-errors-only +WORKDIR /app/venv +CMD [ "./docker_pylint.sh", "--errors-only" ] + +# Pylint full +FROM pylint-base AS pylint-full +WORKDIR /app/venv +CMD [ "./docker_pylint.sh", "--fail-under", "9" ] + # Codestyle reporting FROM testing AS codestyle WORKDIR /app diff --git a/Jenkinsfile.github b/Jenkinsfile.github index 8b216c4f..b5c4aa24 100644 --- a/Jenkinsfile.github +++ b/Jenkinsfile.github @@ -93,7 +93,12 @@ pipeline { } steps { echo "Docker args are ${env.DOCKER_ARGS}" - sh "make image" + sh "make image_setup" + sh "make image_build" + sh "make image_build_pylint_errors" + sh "make image_run_pylint_errors" + sh "make image_build_pylint_full" + sh "make image_run_pylint_full" } } stage('Chart') { diff --git a/Makefile b/Makefile index e6085777..35d6b8d3 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # # MIT License # -# (C) Copyright 2021-2023 Hewlett Packard Enterprise Development LP +# (C) Copyright 2021-2024 Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -55,6 +55,7 @@ RPTR_SOURCE_PATH := ${RPTR_BUILD_DIR}/SOURCES/${RPTR_SOURCE_NAME}.tar.bz2 all : runbuildprep lint image chart rptr_rpm local: cms_meta_tools runbuildprep image chart_setup chart_package chart: chart_setup chart_package chart_test +image: image_setup image_build image_build_pylint_errors image_run_pylint_errors image_build_pylint_full image_run_pylint_full rptr_rpm: rptr_rpm_package_source rptr_rpm_build_source rptr_rpm_build clone_input_files: @@ -90,9 +91,25 @@ rptr_rpm_prepare: cp $(RPTR_SPEC_FILE) $(RPTR_BUILD_DIR)/SPECS/ cat $(RPTR_SPEC_FILE) $(RPTR_BUILD_DIR)/SPECS/bos-reporter.spec -image: +image_setup: + # Create list of BOS Python source files, to be checked later by pylint + find src/bos -type f -name \*.py -print | sed 's#^src/#/app/lib/#' | tr '\n' ' ' | tee srclist.txt + +image_build: docker build --pull ${DOCKER_ARGS} --tag '${NAME}:${DOCKER_VERSION}' . +image_build_pylint_errors: + docker build --pull ${DOCKER_ARGS} --target pylint-errors-only --tag 'pylint-errors-only:${DOCKER_VERSION}' . + +image_run_pylint_errors: + docker run --rm 'pylint-errors-only:${DOCKER_VERSION}' + +image_build_pylint_full: + docker build --pull ${DOCKER_ARGS} --target pylint-full --tag 'pylint-full:${DOCKER_VERSION}' . + +image_run_pylint_full: + docker run --rm 'pylint-full:${DOCKER_VERSION}' + chart_package: helm dep up ${CHART_PATH}/${NAME} helm package ${CHART_PATH}/${NAME} -d ${CHART_PATH}/.packaged --app-version ${DOCKER_VERSION} --version ${CHART_VERSION} diff --git a/api/openapi.yaml.in b/api/openapi.yaml.in index e8e989a2..0da76c3c 100644 --- a/api/openapi.yaml.in +++ b/api/openapi.yaml.in @@ -436,6 +436,10 @@ components: maxLength: 127 pattern: '^[a-zA-Z0-9](?:[-._a-zA-Z0-9]{0,125}[a-zA-Z0-9])?$' example: "session-20190728032600" + V2SessionNameOrEmpty: + oneOf: + - $ref: '#/components/schemas/V2SessionName' + - $ref: '#/components/schemas/EmptyString' V2SessionOperation: type: string enum: ['boot', 'reboot', 'shutdown'] @@ -762,9 +766,7 @@ components: configuration: $ref: '#/components/schemas/CfsConfiguration' session: - oneOf: - - $ref: '#/components/schemas/V2SessionName' - - $ref: '#/components/schemas/EmptyString' + $ref: '#/components/schemas/V2SessionNameOrEmpty' last_updated: $ref: '#/components/schemas/V2ComponentLastUpdated' additionalProperties: false @@ -851,9 +853,7 @@ components: description: A description of the most recent error to impact the Component. maxLength: 65536 session: - oneOf: - - $ref: '#/components/schemas/V2SessionName' - - $ref: '#/components/schemas/EmptyString' + $ref: '#/components/schemas/V2SessionNameOrEmpty' retry_policy: type: integer description: | @@ -892,9 +892,7 @@ components: description: A description of the most recent error to impact the Component. maxLength: 65536 session: - oneOf: - - $ref: '#/components/schemas/V2SessionName' - - $ref: '#/components/schemas/EmptyString' + $ref: '#/components/schemas/V2SessionNameOrEmpty' retry_policy: type: integer description: | @@ -1567,6 +1565,7 @@ paths: tags: - v2 - sessions + - cli_ignore x-openapi-router-controller: bos.server.controllers.v2.sessions operationId: patch_v2_session requestBody: diff --git a/src/__init__.py b/docker_pylint.sh old mode 100644 new mode 100755 similarity index 90% rename from src/__init__.py rename to docker_pylint.sh index f4345184..899f09a4 --- a/src/__init__.py +++ b/docker_pylint.sh @@ -1,7 +1,8 @@ +#!/usr/bin/env sh # # MIT License # -# (C) Copyright 2021-2022 Hewlett Packard Enterprise Development LP +# (C) Copyright 2024 Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -21,3 +22,5 @@ # ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR # OTHER DEALINGS IN THE SOFTWARE. # + +pylint "$@" $(cat ./srclist.txt) diff --git a/src/bos/common/utils.py b/src/bos/common/utils.py index dfcff337..35fe4df5 100644 --- a/src/bos/common/utils.py +++ b/src/bos/common/utils.py @@ -150,14 +150,14 @@ def components_by_id(components: List[dict]) -> dict: """ return { component["id"]: component for component in components } -def reverse_components_by_id(components_by_id: dict) -> List[dict]: +def reverse_components_by_id(components_by_id_map: dict) -> List[dict]: """ Input: - components_by_id: a dictionary with the name of each component as the + components_by_id_map: a dictionary with the name of each component as the key and the value being the entire component itself. Return: A list with each component as an element Purpose: Reverse the effect of components_by_id. """ - return list(components_by_id.values()) + return list(components_by_id_map.values()) diff --git a/src/bos/operators/status.py b/src/bos/operators/status.py index e60ffd7a..83980f07 100644 --- a/src/bos/operators/status.py +++ b/src/bos/operators/status.py @@ -123,6 +123,7 @@ def _check_status(self, component, power_state, cfs_component): Calculate the component's current status based upon its power state and CFS configuration state. If its status differs from the status in the database, return this information. """ + error = None if power_state and cfs_component: phase, override, disable, error, action_failed = self._calculate_status(component, power_state, diff --git a/src/bos/operators/utils/boot_image_metadata/__init__.py b/src/bos/operators/utils/boot_image_metadata/__init__.py index c27e84a4..6ec154f5 100644 --- a/src/bos/operators/utils/boot_image_metadata/__init__.py +++ b/src/bos/operators/utils/boot_image_metadata/__init__.py @@ -66,6 +66,12 @@ def rootfs(self): """ return None + @property + def arch(self): + """ + Get the arch + """ + return None class BootImageMetaDataBadRead(Exception): """ diff --git a/src/bos/operators/utils/boot_image_metadata/s3_boot_image_metadata.py b/src/bos/operators/utils/boot_image_metadata/s3_boot_image_metadata.py index a0ad8f28..798edb29 100644 --- a/src/bos/operators/utils/boot_image_metadata/s3_boot_image_metadata.py +++ b/src/bos/operators/utils/boot_image_metadata/s3_boot_image_metadata.py @@ -27,7 +27,9 @@ from bos.common.utils import exc_type_msg from bos.operators.utils.boot_image_metadata import BootImageMetaData, BootImageMetaDataBadRead -from bos.operators.utils.clients.s3 import S3BootArtifacts, S3MissingConfiguration, \ +from bos.operators.utils.clients.ims import get_image, get_ims_id_from_s3_url, ImageNotFound, \ + DEFAULT_IMS_IMAGE_ARCH +from bos.operators.utils.clients.s3 import S3BootArtifacts, S3MissingConfiguration, S3Url, \ ArtifactNotFound LOGGER = logging.getLogger('bos.operators.utils.boot_image_metadata.s3_boot_image_metadata') @@ -197,3 +199,50 @@ def boot_parameters_etag(self): if bp: return bp['link']['etag'] return None + + @property + def manifest_s3_url(self) -> S3Url: + """ + Returns the S3 URL to the boot manifest + """ + return self.boot_artifacts.s3url + + @property + def arch(self): + """ + Extract the IMS image ID from the S3 manifest path. + Query IMS to get the architecture of the image. + Return the 'arch' field from the IMS image + If image is not in IMS, or 'arch' field not set, log warnings and return None. + (since technically BOS does not require the images to be in IMS) + """ + s3_url = self.manifest_s3_url + ims_id = get_ims_id_from_s3_url(s3_url) + if not ims_id: + LOGGER.warning( + "Boot artifact S3 URL '%s' does not follow the expected IMS image convention", + s3_url) + return None + try: + ims_image_data = get_image(ims_id) + except ImageNotFound: + LOGGER.warning( + "Can't determine architecture of '%s' because image '%s' does not exist in IMS", + s3_url.url, ims_id) + return None + except Exception as err: + LOGGER.error("Error getting IMS image data for '%s' (S3 path '%s'): %s", ims_id, + s3_url.url, exc_type_msg(err)) + return None + try: + return ims_image_data["arch"] + except KeyError: + LOGGER.warning("Defaulting to '%s' because 'arch' field not set in IMS image '%s' " + "(s3 path '%s'): %s", DEFAULT_IMS_IMAGE_ARCH, ims_id, s3_url.url, + ims_image_data) + return DEFAULT_IMS_IMAGE_ARCH + except Exception as err: + LOGGER.error("IMS image '%s' (s3 path '%s'): %s", ims_id, s3_url.url, ims_image_data) + LOGGER.error("Error getting 'arch' field for IMS image '%s': %s", ims_id, + exc_type_msg(err)) + return None diff --git a/src/bos/operators/utils/clients/ims.py b/src/bos/operators/utils/clients/ims.py index d192aec0..1c940fbe 100644 --- a/src/bos/operators/utils/clients/ims.py +++ b/src/bos/operators/utils/clients/ims.py @@ -23,9 +23,12 @@ # import logging +import re from requests.exceptions import HTTPError +from requests.sessions import Session as RequestsSession from bos.common.utils import compact_response_text, exc_type_msg, requests_retry_session, PROTOCOL +from bos.operators.utils.clients.s3 import S3Url SERVICE_NAME = 'cray-ims' IMS_VERSION = 'v3' @@ -35,26 +38,81 @@ LOGGER = logging.getLogger('bos.operators.utils.clients.ims') IMS_TAG_OPERATIONS = ['set', 'remove'] +# Making minimal assumptions about the IMS ID itself, this pattern just makes sure that the +# S3 key is some string, then a /, then at least one more character. +IMS_S3_KEY_RE = r'^([^/]+)/.+' +IMS_S3_KEY_RE_PROG = re.compile(IMS_S3_KEY_RE) + +# If an IMS image does not have the arch field, default to x86_64 for purposes of +# backward-compatibility +DEFAULT_IMS_IMAGE_ARCH = 'x86_64' + class TagFailure(Exception): pass -def patch_image(image_id, data, session=None): + +class ImageNotFound(Exception): + """ + Raised if querying IMS for an image and it is not found + """ + def __init__(self, image_id: str): + super().__init__(f"IMS image id '{image_id}' does not exist in IMS") + + +def get_image(image_id: str, session: RequestsSession|None=None) -> dict: + """ + Queries IMS to retrieve the specified image and return it. + If the image does not exist, raise ImageNotFound. + Other errors (like a failure to query IMS) will result in appropriate exceptions being raised. + """ + if not session: + session = requests_retry_session() + url=f"{IMAGES_ENDPOINT}/{image_id}" + LOGGER.debug("GET %s", url) + response = session.get(url) + LOGGER.debug("Response status code=%d, reason=%s, body=%s", response.status_code, + response.reason, compact_response_text(response.text)) + try: + response.raise_for_status() + except HTTPError as err: + msg = f"Failed asking IMS to get image {image_id}: {exc_type_msg(err)}" + if response.status_code == 404: + # If it's not found, we just log it as a warning, because we may be + # okay with that -- that will be for the caller to decide + LOGGER.warning(msg) + raise ImageNotFound(image_id) from err + LOGGER.error(msg) + raise + try: + return response.json() + except Exception as err: + LOGGER.error("Failed decoding JSON response from getting IMS image %s: %s", image_id, + exc_type_msg(err)) + raise + + +def patch_image(image_id: str, data: dict, session: RequestsSession|None=None) -> None: if not data: LOGGER.warning("patch_image called without data; returning without action.") return if not session: session = requests_retry_session() - LOGGER.debug("PATCH %s with body=%s", IMAGES_ENDPOINT, data) - response = session.patch(f"{IMAGES_ENDPOINT}/{image_id}", json=data) + url=f"{IMAGES_ENDPOINT}/{image_id}" + LOGGER.debug("PATCH %s with body=%s", url, data) + response = session.patch(url, json=data) LOGGER.debug("Response status code=%d, reason=%s, body=%s", response.status_code, response.reason, compact_response_text(response.text)) try: response.raise_for_status() except HTTPError as err: - LOGGER.error("Failed asking IMS to tag image: %s", exc_type_msg(err)) + LOGGER.error("Failed asking IMS to patch image %s: %s", image_id, exc_type_msg(err)) + if response.status_code == 404: + raise ImageNotFound(image_id) from err raise -def tag_image(image_id: str, operation: str, key: str, value: str = None, session=None) -> None: + +def tag_image(image_id: str, operation: str, key: str, value: str=None, + session: RequestsSession|None=None) -> None: if operation not in IMS_TAG_OPERATIONS: msg = f"{operation} not valid. Expecting one of {IMS_TAG_OPERATIONS}" LOGGER.error(msg) @@ -81,3 +139,14 @@ def tag_image(image_id: str, operation: str, key: str, value: str = None, sessio } } patch_image(image_id=image_id, data=data, session=session) + + +def get_ims_id_from_s3_url(s3_url: S3Url) -> str|None: + """ + If the s3_url matches the expected format of an IMS image path, then return the IMS image ID. + Otherwise return None. + """ + try: + return IMS_S3_KEY_RE_PROG.match(s3_url.key).group(1) + except (AttributeError, IndexError): + return None diff --git a/src/bos/server/controllers/v2/boot_set.py b/src/bos/server/controllers/v2/boot_set.py index 7854e134..a4dd52e8 100644 --- a/src/bos/server/controllers/v2/boot_set.py +++ b/src/bos/server/controllers/v2/boot_set.py @@ -22,8 +22,10 @@ # OTHER DEALINGS IN THE SOFTWARE. # +from functools import partial import logging from bos.common.utils import exc_type_msg +from bos.operators.utils.boot_image_metadata import BootImageMetaData from bos.operators.utils.boot_image_metadata.factory import BootImageMetaDataFactory from bos.operators.utils.clients.s3 import S3Object, ArtifactNotFound from bos.server.controllers.v2.options import get_v2_options_data @@ -38,6 +40,33 @@ # Valid boot sets are required to have at least one of these fields HARDWARE_SPECIFIER_FIELDS = ( "node_list", "node_roles_groups", "node_groups" ) +DEFAULT_ARCH = "X86" + +# Mapping from BOS boot set arch values to expected IMS image arch values +# Omits BOS Other value, since there is no corresponding IMS image arch value +EXPECTED_IMS_ARCH = { + "ARM": "aarch64", + "Unknown": "x86_64", + "X86": "x86_64" +} + + +class BootSetError(Exception): + """ + Generic error class for fatal problems found during boot set validation + """ + + +class BootSetArchMismatch(BootSetError): + def __init__(self, bs_arch: str, expected_ims_arch: str, actual_ims_arch: str): + super().__init__(f"Boot set arch '{bs_arch}' means IMS image arch should be " + f"'{expected_ims_arch}', but actual IMS image arch is '{actual_ims_arch}'") + + +class CannotValidateBootSetArch(BootSetError): + def __init__(self, msg: str): + super().__init__(f"Can't validate boot image arch: {msg}") + def validate_boot_sets(session_template: dict, operation: str, @@ -61,8 +90,6 @@ def validate_boot_sets(session_template: dict, 0 -- Success 1 -- Warning, not fatal 2 -- Error, fatal - - """ # Verify boot sets exist. if not session_template.get('boot_sets', None): @@ -72,78 +99,135 @@ def validate_boot_sets(session_template: dict, if reject_nids is None: reject_nids = get_v2_options_data().get('reject_nids', False) + warning_msgs = [] for bs_name, bs in session_template['boot_sets'].items(): - warning_msgs = [] - - # Verify that the hardware is specified - specified = [bs.get(field, None) - for field in HARDWARE_SPECIFIER_FIELDS] - if not any(specified): - msg = f"Session template: '{template_name}' boot set: '{bs_name}' " \ - f"must have at least one non-empty" \ - f"hardware specifier field provided (%s); None were provided." \ - % (', '.join(sorted(HARDWARE_SPECIFIER_FIELDS))) + bs_msg = partial(_bs_msg, template_name=template_name, bs_name=bs_name) + try: + bs_warning_msgs = _validate_boot_set(bs=bs, operation=operation, + reject_nids=reject_nids) + except BootSetError as err: + msg = bs_msg(str(err)) LOGGER.error(msg) return BOOT_SET_ERROR, msg + except Exception as err: + LOGGER.error( + bs_msg(f"Unexpected exception in _validate_boot_set: {exc_type_msg(err)}")) + raise + for msg in map(bs_msg, bs_warning_msgs): + LOGGER.warning(msg) + warning_msgs.append(msg) + + if warning_msgs: + return BOOT_SET_WARNING, "; ".join(warning_msgs) + + return BOOT_SET_SUCCESS, "Valid" + + +def _bs_msg(msg: str, template_name: str, bs_name: str) -> str: + """ + Shortcut for creating validation error/warning messages for a specific bootset + """ + return f"Session template: '{template_name}' boot set: '{bs_name}': {msg}" + + +def _validate_boot_set(bs: dict, operation: str, reject_nids: bool) -> list[str]: + """ + Helper function for validate_boot_sets that performs validation on a single boot set. + Raises BootSetError if fatal errors found. + Returns a list of warning messages (if any) + """ + warning_msgs = [] + + # Verify that the hardware is specified + specified = [bs.get(field, None) + for field in HARDWARE_SPECIFIER_FIELDS] + if not any(specified): + raise BootSetError(f"No non-empty hardware specifier field {HARDWARE_SPECIFIER_FIELDS}") + try: + if any(node[:3] == "nid" for node in bs["node_list"]): + msg = "Has NID in 'node_list'" + if reject_nids: + raise BootSetError(msg) + # Otherwise, log this as a warning -- even if reject_nids is not set, + # BOS still doesn't support NIDs, so this is still undesirable + warning_msgs.append(msg) + except KeyError: + # If there is no node_list field, not a problem + pass + + if operation in ['boot', 'reboot']: + # Verify that the boot artifacts exist + try: + image_metadata = BootImageMetaDataFactory(bs)() + except Exception as err: + raise BootSetError(f"Can't find boot artifacts. Error: {exc_type_msg(err)}") from err + try: - if any(node[:3] == "nid" for node in bs["node_list"]): - msg = f"Session template: '{template_name}' boot set: '{bs_name}' "\ - "has NID in 'node_list'" - if reject_nids: - LOGGER.error(msg) - return BOOT_SET_ERROR, msg - # Otherwise, log this as a warning -- even if reject_nids is not set, - # BOS still doesn't support NIDs, so this is still undesirable - LOGGER.warning(msg) - warning_msgs.append(msg) - except KeyError: - # If there is no node_list field, not a problem - pass - - if operation in ['boot', 'reboot']: - # Verify that the boot artifacts exist + validate_boot_set_arch(bs, image_metadata) + except CannotValidateBootSetArch as err: + warning_msgs.append(str(err)) + except BootSetError as err: + raise BootSetError(f"Arch validation error: {err}") from err + except Exception as err: + raise BootSetError(f"Arch validation error: {exc_type_msg(err)}") from err + + # Check boot artifacts' S3 headers + for boot_artifact in ["kernel"]: try: - image_metadata = BootImageMetaDataFactory(bs)() + artifact = getattr(image_metadata.boot_artifacts, boot_artifact) + path = artifact ['link']['path'] + etag = artifact['link']['etag'] + obj = S3Object(path, etag) + _ = obj.object_header except Exception as err: - msg = f"Session template: '{template_name}' boot set: '{bs_name}' " \ - f"could not locate its boot artifacts. Error: " + exc_type_msg(err) - LOGGER.error(msg) - return BOOT_SET_ERROR, msg - - # Check boot artifacts' S3 headers - for boot_artifact in ["kernel"]: - try: - artifact = getattr(image_metadata.boot_artifacts, boot_artifact) - path = artifact ['link']['path'] - etag = artifact['link']['etag'] - obj = S3Object(path, etag) - _ = obj.object_header - except Exception as err: - msg = f"Session template: '{template_name}' boot set: '{bs_name}' " \ - f"could not locate its {boot_artifact}. Error: " + exc_type_msg(err) - LOGGER.error(msg) - return BOOT_SET_ERROR, msg - - for boot_artifact in ["initrd", "boot_parameters"]: - try: - artifact = getattr(image_metadata.boot_artifacts, boot_artifact) - if not artifact: - raise ArtifactNotFound(f"Session template: '{template_name}' " - f"boot set: '{bs_name}' " - f"does not contain a {boot_artifact}.") - path = artifact ['link']['path'] - etag = artifact['link']['etag'] - obj = S3Object(path, etag) - _ = obj.object_header - except Exception as err: - msg = f"Session template: '{template_name}' boot set: '{bs_name}' " \ - f"could not locate its {boot_artifact}. Warning: " + exc_type_msg(err) - LOGGER.warning(msg) - warning_msgs.append(msg) - if warning_msgs: - return BOOT_SET_WARNING, "; ".join(warning_msgs) + raise BootSetError(f"Can't find {boot_artifact} in " + f"{image_metadata.manifest_s3_url.url}. " + f"Error: {exc_type_msg(err)}") from err - return BOOT_SET_SUCCESS, "Valid" + for boot_artifact in ["initrd", "boot_parameters"]: + try: + artifact = getattr(image_metadata.boot_artifacts, boot_artifact) + if not artifact: + raise ArtifactNotFound() + path = artifact ['link']['path'] + etag = artifact['link']['etag'] + obj = S3Object(path, etag) + _ = obj.object_header + except ArtifactNotFound as err: + warning_msgs.append( + f"{image_metadata.manifest_s3_url.url} doesn't contain a {boot_artifact}") + except Exception as err: + warning_msgs.append(f"Can't find {boot_artifact} in " + f"{image_metadata.manifest_s3_url.url}. " + f"Warning: {exc_type_msg(err)}") + + return warning_msgs + + +def validate_boot_set_arch(bs: dict, image_metadata: BootImageMetaData|None=None) -> None: + """ + If the boot set architecture is not set to Other, check that the IMS image + architecture matches the boot set architecture (treating a boot set architecture + of Unknown as X86) + """ + arch = bs.get("arch", DEFAULT_ARCH) + if arch == 'Other': + raise CannotValidateBootSetArch("Boot set arch set to 'Other'") + + if image_metadata is None: + try: + image_metadata = BootImageMetaDataFactory(bs)() + except Exception as err: + raise CannotValidateBootSetArch( + f"Can't find boot artifacts: {exc_type_msg(err)}") from err + + ims_image_arch = image_metadata.arch + + if ims_image_arch is None: + raise CannotValidateBootSetArch("Can't determine architecture of boot artifacts") + if EXPECTED_IMS_ARCH[arch] != ims_image_arch: + raise BootSetArchMismatch(bs_arch=arch, expected_ims_arch=EXPECTED_IMS_ARCH[arch], + actual_ims_arch=ims_image_arch) def validate_sanitize_boot_sets(template_data: dict) -> None: @@ -188,11 +272,26 @@ def validate_sanitize_boot_set(bs_name: str, bs_data: dict, reject_nids: bool=Fa raise ParsingException(f"boot_sets key ({bs_name}) does not match 'name' " f"field of corresponding boot set ({bs_data['name']})") + # Validate the boot set architecture + try: + validate_boot_set_arch(bs_data) + except CannotValidateBootSetArch as err: + LOGGER.warning('%s', bs_data) + LOGGER.warning("Bboot set '%s': %s", bs_name, err) + except Exception as err: + raise ParsingException( + f"Error found validating arch of boot set '{bs_name}': {exc_type_msg(err)}") from err + # Validate that the boot set has at least one of the HARDWARE_SPECIFIER_FIELDS if not any(field_name in bs_data for field_name in HARDWARE_SPECIFIER_FIELDS): raise ParsingException(f"Boot set {bs_name} has none of the following " f"fields: {HARDWARE_SPECIFIER_FIELDS}") + # Validate that at least one of the HARDWARE_SPECIFIER_FIELDS is non-empty + if not any(field_name in bs_data and bs_data[field_name] for field_name in HARDWARE_SPECIFIER_FIELDS): + raise ParsingException(f"Boot set {bs_name} has no non-empty hardware-specifier fields: " + f"{HARDWARE_SPECIFIER_FIELDS}") + # Last thing to do is validate/sanitize the node_list field, if it is present try: node_list = bs_data["node_list"] diff --git a/src/bos/server/migrations/sanitize.py b/src/bos/server/migrations/sanitize.py index 34443f04..d4171686 100644 --- a/src/bos/server/migrations/sanitize.py +++ b/src/bos/server/migrations/sanitize.py @@ -28,7 +28,7 @@ import string from bos.common.tenant_utils import get_tenant_aware_key -from bos.server.controllers.v2.boot_set import HARDWARE_SPECIFIER_FIELDS +from bos.server.controllers.v2.boot_set import DEFAULT_ARCH, HARDWARE_SPECIFIER_FIELDS from bos.server.schema import validator from .db import TEMP_DB, delete_component, delete_session, delete_template @@ -196,6 +196,10 @@ def sanitize_bootset(bsname: str, bsdata: dict) -> str|None: # be stored inside the boot set under the current API spec bsdata.pop("name", None) + # If the arch field is not present, set it to its default value + if "arch" not in bsdata: + bsdata["arch"] = DEFAULT_ARCH + # Remove any fields that are no longer in the spec bad_fields = [ field for field in bsdata if field not in validator.boot_set_fields ] for field in bad_fields: