CRAYSAT-1896: Validate playbook in validation method instead of schema

Instead of requiring that the playbook be present in the bootprep input
file through the schema, validate it specifically when `sat` is
configured to use the CFS v3 API, since the CFS v2 API does not require
that CFS configuration layers have a playbook. This will allow people to
continue using old bootprep input files that have CFS configuration
layers that do not specify a playbook as long as they use CFS v2. This
is slightly less disruptive than making a schema change.

Test Description:
Unit tests only so far.

sat/cli/bootprep/input/configuration.py

@@ -28,15 +28,16 @@
 import logging
 
 from csm_api_client.service.gateway import APIError
-from csm_api_client.service.cfs import CFSConfigurationError
+from csm_api_client.service.cfs import CFSV3Client, CFSConfigurationError
 
 from sat.cached_property import cached_property
 from sat.cli.bootprep.constants import LATEST_VERSION_VALUE
-from sat.cli.bootprep.errors import InputItemCreateError
+from sat.cli.bootprep.errors import InputItemCreateError, InputItemValidateError
 from sat.cli.bootprep.input.base import (
     BaseInputItem,
     BaseInputItemCollection,
-    jinja_rendered,
+    Validatable,
+    jinja_rendered
 )
 from sat.config import get_config_value
 from sat.util import get_val_by_path
@@ -108,15 +109,31 @@ class InputConfigurationLayer(InputConfigurationLayerBase, ABC):
     @property
     @jinja_rendered
     def playbook(self):
-        """str: the playbook specified in the layer"""
-        # playbook is now required by the schema
-        return self.layer_data['playbook']
+        """str or None: the playbook specified in the layer"""
+        # Note that CFS v3 requires a playbook but CFS v2 does not, so the
+        # schema does not require a playbook, but it is validated in the
+        # validate_playbook_specified_with_cfs_v3 method below.
+        return self.layer_data.get('playbook')
 
     @property
     def ims_require_dkms(self):
         """str or None: whether to enable DKMS when this layer customizes an IMS image"""
         return get_val_by_path(self.layer_data, 'special_parameters.ims_require_dkms')
 
+    @Validatable.validation_method()
+    def validate_playbook_specified_with_cfs_v3(self, **_):
+        """Validate that a playbook is specified when using CFS v3.
+
+        The CFS v3 API will respond with a clear error, but validating here
+        allows this issue to be caught in --dry-run mode.
+
+        Raises:
+            InputItemValidateError: if a playbook is not specified when using CFS v3
+        """
+        if isinstance(self.cfs_client, CFSV3Client) and not self.playbook:
+            raise InputItemValidateError('A playbook is required when using the '
+                                         'CFS v3 API to create configurations.')
+
     @staticmethod
     def get_configuration_layer(layer_data, jinja_env, cfs_client, product_catalog):
         """Get and return a new InputConfigurationLayer for the given layer data.

sat/data/schema/bootprep_schema.yaml

@@ -1,7 +1,7 @@
 #
 # MIT License
 #
-# (C) Copyright 2021-2024 Hewlett Packard Enterprise Development LP
+# (C) Copyright 2021-2023 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"),
@@ -41,7 +41,7 @@ $schema: "https://json-schema.org/draft/2020-12/schema"
 # ... patch component when all input files that were valid under the old schema
 #     are still valid under the new schema
 #
-version: '1.1.0'
+version: '1.0.7'
 title: Bootprep Input File
 description: >
   A description of the set of CFS configurations to create, the set of IMS
@@ -113,7 +113,7 @@ properties:
                 A layer of the CFS configuration defined using an explicit git
                 repository URL and commit hash or branch name.
               type: object
-              required: ['git', 'playbook']
+              required: ['git']
               additionalProperties: false
               properties:
                 name:
@@ -150,7 +150,7 @@ properties:
                 A layer of the CFS configuration defined using a product's
                 configuration management repository.
               type: object
-              required: ['product', 'playbook']
+              required: ['product']
               additionalProperties: false
               properties:
                 name:

tests/cli/bootprep/input/test_configuration.py

@@ -24,16 +24,14 @@
 """
 Tests for sat.cli.bootprep.input.configuration
 """
-from copy import deepcopy
 import logging
 import unittest
 from unittest.mock import Mock, patch
 
-from csm_api_client.service.vcs import VCSError
 from jinja2.sandbox import SandboxedEnvironment
 
-from csm_api_client.service.cfs import CFSClientBase, CFSConfigurationError
-from sat.cli.bootprep.errors import InputItemCreateError
+from csm_api_client.service.cfs import CFSClientBase, CFSConfigurationError, CFSV2Client, CFSV3Client
+from sat.cli.bootprep.errors import InputItemCreateError, InputItemValidateError
 from sat.cli.bootprep.input.configuration import (
     AdditionalInventory,
     InputConfigurationLayer,
@@ -156,11 +154,18 @@ def tearDown(self):
         patch.stopall()
 
     def test_playbook_property_present(self):
-        """Test the playbook property"""
+        """Test the playbook property when a playbook is in the layer data"""
         layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,
                                            self.mock_cfs_client)
         self.assertEqual(self.playbook, layer.playbook)
 
+    def test_playbook_property_not_present(self):
+        """Test the playbook property when a playbook is not in the layer data"""
+        del self.branch_layer_data['playbook']
+        layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,
+                                           self.mock_cfs_client)
+        self.assertIsNone(layer.playbook)
+
     def test_playbook_property_jinja_template(self):
         """Test the playbook property when the playbook uses Jinja2 templating"""
         self.branch_layer_data['playbook'] = 'shs_{{shs.network_type}}_install.yaml'
@@ -225,6 +230,38 @@ def test_commit_property_not_present(self):
                                            self.mock_cfs_client)
         self.assertIsNone(layer.commit)
 
+    def test_validate_playbook_cfs_v3(self):
+        """Test the validate_playbook_specified_with_cfs_v3 method with a CFSV3Client and present playbook"""
+        mock_cfs_v3_client = Mock(spec=CFSV3Client)
+        layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,
+                                           mock_cfs_v3_client)
+        layer.validate_playbook_specified_with_cfs_v3()
+
+    def test_validate_playbook_missing_cfs_v3(self):
+        """Test the validate_playbook_specified_with_cfs_v3 method with a CFSV3Client and missing playbook"""
+        mock_cfs_v3_client = Mock(spec=CFSV3Client)
+        del self.branch_layer_data['playbook']
+        layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,
+                                           mock_cfs_v3_client)
+        err_regex = 'A playbook is required when using the CFS v3 API'
+        with self.assertRaisesRegex(InputItemValidateError, err_regex):
+            layer.validate_playbook_specified_with_cfs_v3()
+
+    def test_validate_playbook_cfs_v2(self):
+        """Test the validate_playbook_specified_with_cfs_v3 method with a CFSV2Client and present playbook"""
+        mock_cfs_v2_client = Mock(spec=CFSV2Client)
+        layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,
+                                           mock_cfs_v2_client)
+        layer.validate_playbook_specified_with_cfs_v3()
+
+    def test_validate_playbook_missing_cfs_v2(self):
+        """Test the validate_playbook_specified_with_cfs_v3 method with a CFSV2Client and missing playbook"""
+        mock_cfs_v2_client = Mock(spec=CFSV2Client)
+        del self.branch_layer_data['playbook']
+        layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,
+                                           mock_cfs_v2_client)
+        layer.validate_playbook_specified_with_cfs_v3()
+
     def test_get_cfs_api_data_no_resolve_branches(self):
         """Test get_cfs_api_data method without branch resolution"""
         layer = GitInputConfigurationLayer(self.branch_layer_data, self.jinja_env,

tests/cli/bootprep/test_validate.py

@@ -1,7 +1,7 @@
 #
 # MIT License
 #
-# (C) Copyright 2021-2024 Hewlett Packard Enterprise Development LP
+# (C) Copyright 2021-2023 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"),
@@ -50,8 +50,7 @@
     'product': {
         'name': 'sma',
         'version': '1.4.2'
-    },
-    'playbook': 'sma-ldms-compute.yml'
+    }
 }
 
 VALID_CONFIG_LAYER_PRODUCT_BRANCH = {
@@ -60,7 +59,6 @@
         'name': 'cos',
         'branch': 'integration'
     },
-    'playbook': 'cos-compute.yml',
     'special_parameters': {
         'ims_require_dkms': True
     }
@@ -71,8 +69,7 @@
     'git': {
         'url': 'https://api-gw-service-nmn.local/vcs/cray/cpe-config-management.git',
         'branch': 'integration'
-    },
-    'playbook': 'pe_deploy.yml'
+    }
 }
 
 VALID_CONFIG_LAYER_GIT_COMMIT = {
@@ -83,8 +80,7 @@
     },
     'special_parameters': {
         'ims_require_dkms': True
-    },
-    'playbook': 'site.yml'
+    }
 }
 
 VALID_IMAGE_IMS_NAME_WITH_CONFIG_V1 = {
@@ -785,17 +781,6 @@ def test_invalid_config_layer_missing_keys(self):
         ]
         self.assert_invalid_instance(instance, expected_errs)
 
-    def test_invalid_config_layer_missing_playbook(self):
-        """Invalid configuration missing 'playbook' key"""
-        layer = deepcopy(VALID_CONFIG_LAYER_PRODUCT_VERSION)
-        del layer['playbook']
-        instance = self.get_instance_with_config_layer(layer)
-        expected_errs = [
-            (('configurations', 0, 'layers', 0), NOT_VALID_ANY_OF_MESSAGE, 1),
-            (('configurations', 0, 'layers', 0), "'playbook' is a required property", 2)
-        ]
-        self.assert_invalid_instance(instance, expected_errs)
-
     def test_invalid_config_layer_both_keys(self):
         """Invalid configuration layer with both 'git' and 'product' keys"""
         layer = {