Be configuration driven when referencing document names/schemas

Currently, any document name or schema referenced in the Shipyard
code base is a hard-coded string. Often times, these strings are
repeated throughout the code. This patch set adds a new configuration
section to shipyard.conf to define document names and schemas so they
can then be referenced in the Shipyard code via the oslo
configuration object. This functionality will be important for
upcoming Shipyard features which will call for more documents to be
validated as well as some new Shipyard-created docs.

Change-Id: I34ae8cd578bab730d004c3d176e3817b5a45c89e

charts/shipyard/values.yaml

@@ -415,6 +415,16 @@ conf:
       # If non-existent rule is used, the request should be denied. The
       # deny_all rule is hard coded in the policy.py code to allow no access.
       policy_default_rule: deny_all
+    document_info:
+      # The name of the deployment configuration document that Shipyard expects
+      # and validates
+      deployment_configuration_name: deployment-configuration
+      # The schema of the deployment configuration document that Shipyard
+      # expects and validates
+      deployment_configuration_schema: shipyard/DeploymentConfiguration/v1
+      # The schema of the deployment strategy document that Shipyard expects
+      # and validates.
+      deployment_strategy_schema: shipyard/DeploymentStrategy/v1
   airflow_config_file:
     path: /usr/local/airflow/airflow.cfg
   airflow:

doc/source/_static/shipyard.conf.sample

@@ -84,6 +84,27 @@
 #service_type = deckhand
 
 
+[document_info]
+
+#
+# From shipyard_api
+#
+
+# The name of the deployment-configuration document that Shipyard expects and
+# validates (string value)
+#deployment_configuration_name = deployment-configuration
+
+# The schema of the deployment-configuration document that Shipyard expects and
+# validates (string value)
+#deployment_configuration_schema = shipyard/DeploymentConfiguration/v1
+
+# The schema of the deployment strategy document that Shipyard expects and
+# validates. Note that the name of this document is not configurable, because
+# it is controlled by a field in the deployment configuration document. (string
+# value)
+#deployment_strategy_schema = shipyard/DeploymentStrategy/v1
+
+
 [drydock]
 
 #

doc/source/sampleconf.rst

@@ -12,6 +12,7 @@
     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
     See the License for the specific language governing permissions and
     limitations under the License.
+.. _sample-configuration:
 
 Sample Configuration File
 ==========================

doc/source/site-definition-documents.rst

@@ -37,6 +37,11 @@ of the Armada manifest that will be used during the deployment/update.
 
 A `sample deployment-configuration`_ shows a completely specified example.
 
+Note that the name and schema Shipyard expects the deployment configuration
+document to have is conifgurable via the document_info section in the
+:ref:`Shipyard configuration <sample-configuration>`, but should be left
+defaulted in most cases.
+
 `Default configuration values`_ are provided for most values.
 
 Supported values
@@ -179,6 +184,11 @@ document for the site. Example::
 -  The success criteria indicates that all nodes must be succssful to consider
    the group a success.
 
+Note that the schema Shipyard expects the deployment strategy document to have
+is conifgurable via the document_info section in the
+:ref:`Shipyard configuration <sample-configuration>`, but should be left
+defaulted in most cases.
+
 In short, the default behavior is to deploy everything all at once, and halt
 if there are any failures.
 

src/bin/shipyard_airflow/etc/shipyard/shipyard.conf.sample

@@ -84,6 +84,27 @@
 #service_type = deckhand
 
 
+[document_info]
+
+#
+# From shipyard_api
+#
+
+# The name of the deployment-configuration document that Shipyard expects and
+# validates (string value)
+#deployment_configuration_name = deployment-configuration
+
+# The schema of the deployment-configuration document that Shipyard expects and
+# validates (string value)
+#deployment_configuration_schema = shipyard/DeploymentConfiguration/v1
+
+# The schema of the deployment strategy document that Shipyard expects and
+# validates. Note that the name of this document is not configurable, because
+# it is controlled by a field in the deployment configuration document. (string
+# value)
+#deployment_strategy_schema = shipyard/DeploymentStrategy/v1
+
+
 [drydock]
 
 #

src/bin/shipyard_airflow/shipyard_airflow/conf/config.py

@@ -286,6 +286,34 @@ SECTIONS = [
             ),
         ]
     ),
+    ConfigSection(
+        name='document_info',
+        title=('Information about some of the documents Shipyard needs to '
+               'handle'),
+        options=[
+            cfg.StrOpt(
+                'deployment_configuration_name',
+                default='deployment-configuration',
+                help=('The name of the deployment-configuration document that '
+                      'Shipyard expects and validates')
+            ),
+            cfg.StrOpt(
+                'deployment_configuration_schema',
+                default='shipyard/DeploymentConfiguration/v1',
+                help=('The schema of the deployment-configuration document '
+                      'that Shipyard expects and validates')
+            ),
+            cfg.StrOpt(
+                'deployment_strategy_schema',
+                default='shipyard/DeploymentStrategy/v1',
+                help=('The schema of the deployment strategy document that '
+                      'Shipyard expects and validates. Note that the name of '
+                      'this document is not configurable, because it is '
+                      'controlled by a field in the deployment configuration '
+                      'document.')
+            ),
+        ]
+    ),
 ]
 
 

src/bin/shipyard_airflow/shipyard_airflow/control/helpers/configdocs_helper.py

@@ -525,7 +525,7 @@ class ConfigdocsHelper(object):
                 service_clients.deckhand_client(),
                 revision_id,
                 [(ValidateDeploymentConfigurationFull,
-                  'deployment-configuration')]
+                  CONF.document_info.deployment_configuration_name)]
             )
             return sy_val_mgr.validate()
         except Exception as ex:

src/bin/shipyard_airflow/shipyard_airflow/control/validators/validate_deployment_action.py

@@ -14,6 +14,7 @@
 import logging
 
 import falcon
+from oslo_config import cfg
 
 from .validate_deployment_configuration \
     import ValidateDeploymentConfigurationBasic
@@ -24,6 +25,7 @@ from shipyard_airflow.common.document_validators.document_validator_manager \
 from shipyard_airflow.errors import ApiError
 
 LOG = logging.getLogger(__name__)
+CONF = cfg.CONF
 
 
 class ValidateDeploymentAction:
@@ -40,7 +42,7 @@ class ValidateDeploymentAction:
                 dh_client,
                 self.doc_revision,
                 [(ValidateDeploymentConfigurationFull,
-                  'deployment-configuration')]
+                  CONF.document_info.deployment_configuration_name)]
             )
         else:
             # Perform a basic validation only
@@ -48,7 +50,7 @@ class ValidateDeploymentAction:
                 dh_client,
                 self.doc_revision,
                 [(ValidateDeploymentConfigurationBasic,
-                  'deployment-configuration')]
+                  CONF.document_info.deployment_configuration_name)]
             )
 
     def validate(self):

src/bin/shipyard_airflow/shipyard_airflow/control/validators/validate_deployment_configuration.py

@@ -18,11 +18,14 @@ is performed by Deckhand on Shipyard's behalf.
 """
 import logging
 
+from oslo_config import cfg
+
 from shipyard_airflow.common.document_validators.document_validator import (
     DocumentValidator
 )
 from .validate_deployment_strategy import ValidateDeploymentStrategy
 
+CONF = cfg.CONF
 LOG = logging.getLogger(__name__)
 
 
@@ -36,7 +39,7 @@ class ValidateDeploymentConfigurationBasic(DocumentValidator):
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
 
-    schema = "shipyard/DeploymentConfiguration/v1"
+    schema = CONF.document_info.deployment_configuration_schema
     missing_severity = "Error"
 
     def do_validate(self):

src/bin/shipyard_airflow/shipyard_airflow/control/validators/validate_deployment_strategy.py

@@ -18,6 +18,8 @@ is performed by Deckhand on Shipyard's behalf.
 """
 import logging
 
+from oslo_config import cfg
+
 from shipyard_airflow.common.deployment_group.deployment_group_manager import (
     DeploymentGroupManager
 )
@@ -35,6 +37,7 @@ from shipyard_airflow.control.helpers.design_reference_helper import (
     DesignRefHelper
 )
 LOG = logging.getLogger(__name__)
+CONF = cfg.CONF
 
 
 def _get_node_lookup(revision_id):
@@ -56,7 +59,7 @@ class ValidateDeploymentStrategy(DocumentValidator):
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
 
-    schema = "shipyard/DeploymentStrategy/v1"
+    schema = CONF.document_info.deployment_strategy_schema
     missing_severity = "Error"
 
     def do_validate(self):

src/bin/shipyard_airflow/shipyard_airflow/plugins/deployment_configuration_operator.py

@@ -13,9 +13,10 @@
 # limitations under the License.
 """Deployment Configuration
 
-Retrieves the deployment configuration from Deckhand and places the values
+Retrieves the deployment-configuration from Deckhand and places the values
 retrieved into a dictionary
 """
+import configparser
 import logging
 
 from airflow.exceptions import AirflowException
@@ -34,12 +35,13 @@ except ImportError:
 from shipyard_airflow.shipyard_const import CustomHeaders
 
 LOG = logging.getLogger(__name__)
+DOCUMENT_INFO = 'document_info'
 
 
 class DeploymentConfigurationOperator(BaseOperator):
     """Deployment Configuration Operator
 
-    Retrieve the deployment configuration from Deckhand for use throughout
+    Retrieve the deployment-configuration from Deckhand for use throughout
     the workflow. Put the configuration into a dictionary.
 
     Failures are raised:
@@ -89,18 +91,23 @@ class DeploymentConfigurationOperator(BaseOperator):
         :param main_dag_name: Parent Dag
         :param shipyard_conf: Location of shipyard.conf
         """
-
         super(DeploymentConfigurationOperator, self).__init__(*args, **kwargs)
         self.main_dag_name = main_dag_name
         self.shipyard_conf = shipyard_conf
         self.action_info = {}
 
+    def _read_config(self):
+        """Read in and parse the shipyard config"""
+        self.config = configparser.ConfigParser()
+        self.config.read(self.shipyard_conf)
+
     def execute(self, context):
         """Perform Deployment Configuration extraction"""
-
+        self._read_config()
         revision_id = self.get_revision_id(context.get('task_instance'))
         doc = self.get_doc(revision_id)
         converted = self.map_config_keys(doc)
+
         # return the mapped configuration so that it can be placed on xcom
         return converted
 
@@ -116,7 +123,7 @@ class DeploymentConfigurationOperator(BaseOperator):
             revision_id = self.action_info['committed_rev_id']
 
             if revision_id:
-                LOG.info("Revision is set to: %s for deployment configuration",
+                LOG.info("Revision is set to: %s for deployment-configuration",
                          revision_id)
                 return revision_id
         # either revision id was not on xcom, or the task_instance is messed
@@ -127,14 +134,16 @@ class DeploymentConfigurationOperator(BaseOperator):
 
     def get_doc(self, revision_id):
         """Get the DeploymentConfiguration document dictionary from Deckhand"""
-        LOG.info(
-            "Attempting to retrieve shipyard/DeploymentConfiguration/v1, "
-            "deployment-configuration from Deckhand"
-        )
-        filters = {
-            "schema": "shipyard/DeploymentConfiguration/v1",
-            "metadata.name": "deployment-configuration"
-        }
+        schema_fallback = 'shipyard/DeploymentConfiguration/v1'
+        schema = self.config.get(DOCUMENT_INFO,
+                                 'deployment_configuration_schema',
+                                 fallback=schema_fallback)
+        name = self.config.get(DOCUMENT_INFO,
+                               'deployment_configuration_name',
+                               fallback='deployment-configuration')
+        LOG.info("Attempting to retrieve {}, {} from Deckhand".format(schema,
+                                                                      name))
+        filters = {"schema": schema, "metadata.name": name}
 
         # Create additional headers dict to pass context marker
         # and end user
@@ -160,7 +169,7 @@ class DeploymentConfigurationOperator(BaseOperator):
             except AttributeError:
                 failed_url = "No URL generated"
             LOG.exception(ex)
-            raise AirflowException("Failed to retrieve deployment "
+            raise AirflowException("Failed to retrieve deployment-"
                                    "configuration yaml using url: "
                                    "{}".format(failed_url))
 
@@ -178,7 +187,7 @@ class DeploymentConfigurationOperator(BaseOperator):
 
         Converts to a more simple map of key-value pairs
         """
-        LOG.info("Mapping keys from deployment configuration")
+        LOG.info("Mapping keys from deployment-configuration")
         return {
             cfg_key: self.get_cfg_value(cfg_data, cfg_key, cfg_default)
             for cfg_key, cfg_default in

src/bin/shipyard_airflow/shipyard_airflow/plugins/drydock_nodes.py

@@ -53,6 +53,7 @@ except ImportError:
     )
 
 LOG = logging.getLogger(__name__)
+DOCUMENT_INFO = 'document_info'
 
 
 class DrydockNodesOperator(DrydockBaseOperator):
@@ -287,10 +288,12 @@ class DrydockNodesOperator(DrydockBaseOperator):
             strat_name = self.dc['physical_provisioner.deployment_strategy']
             if strat_name:
                 # if there is a deployment strategy specified, use it
-                strategy = self.get_unique_doc(
-                    name=strat_name,
-                    schema="shipyard/DeploymentStrategy/v1"
-                )
+                schema_fallback = 'shipyard/DeploymentStrategy/v1'
+                schema = self.config.get(DOCUMENT_INFO,
+                                         'deployment_strategy_schema',
+                                         fallback=schema_fallback)
+
+                strategy = self.get_unique_doc(name=strat_name, schema=schema)
             else:
                 # The default behavior is to deploy all nodes, and fail if
                 # any nodes fail to deploy.

src/bin/shipyard_airflow/shipyard_airflow/plugins/ucp_base_operator.py

@@ -253,10 +253,7 @@ class UcpBaseOperator(BaseOperator):
         if revision_id is None:
             revision_id = self.revision_id
 
-        LOG.info(
-            "Retrieve shipyard/DeploymentConfiguration/v1, "
-            "deployment-configuration from Deckhand"
-        )
+        LOG.info("Retrieve {}, {} from Deckhand".format(schema, name))
         try:
             return self.doc_utils.get_unique_doc(revision_id=revision_id,
                                                  name=name,

src/bin/shipyard_airflow/tests/unit/plugins/test_deployment_configuration_operator.py

@@ -11,6 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+from configparser import ConfigParser
 from unittest import mock
 
 import pytest
@@ -40,11 +41,13 @@ ACTION_INFO_NO_COMMIT = {
 
 try:
     from deployment_configuration_operator import (
-        DeploymentConfigurationOperator
+        DeploymentConfigurationOperator,
+        DOCUMENT_INFO
     )
 except ImportError:
     from shipyard_airflow.plugins.deployment_configuration_operator import (
-        DeploymentConfigurationOperator
+        DeploymentConfigurationOperator,
+        DOCUMENT_INFO
     )
 
 try:
@@ -55,6 +58,21 @@ except ImportError:
     )
 
 
+def make_fake_config():
+    """Make/return a fake config using configparser that we can use for testing
+
+    :returns: A fake configuration object
+    :rtype: ConfigParser
+    """
+    cfg = ConfigParser()
+    cfg.add_section(DOCUMENT_INFO)
+    cfg.set(DOCUMENT_INFO, 'deployment_configuration_name',
+            'deployment-configuration')
+    cfg.set(DOCUMENT_INFO, 'deployment_configuration_schema',
+            'shipyard/DeploymentConfiguration/v1')
+    return cfg
+
+
 def test_execute_exception():
     """Test that execute results in a failure with bad context"""
 
@@ -67,18 +85,21 @@ def test_execute_exception():
     assert ("Design_revision is not set. Cannot proceed with retrieval"
             " of the design configuration") in str(expected_exc)
 
-
+@mock.patch.object(DeploymentConfigurationOperator, '_read_config')
 @mock.patch.object(DeploymentConfigurationOperator, 'get_revision_id',
                    return_value=99)
-def test_execute_no_client(*args):
+def test_execute_no_client(get_revision_id, read_config):
     # no keystone authtoken present in configuration
     dco = DeploymentConfigurationOperator(main_dag_name="main",
                                           shipyard_conf="shipyard.conf",
                                           task_id="t1")
+    dco.config = make_fake_config()
     with pytest.raises(AirflowException) as expected_exc:
-        dco.execute(context={})
-    assert ("Failed to retrieve deployment configuration yaml") in str(
+        dco.execute(context={'task_instance': 'asdf'})
+    assert ("Failed to retrieve deployment-configuration yaml") in str(
         expected_exc)
+    get_revision_id.assert_called_once_with('asdf')
+    read_config.assert_called_once_with()
 
 
 @mock.patch.object(airflow.models.TaskInstance, 'xcom_pull',
@@ -113,6 +134,7 @@ def test_get_doc_no_deckhand():
     dco = DeploymentConfigurationOperator(main_dag_name="main",
                                           shipyard_conf="shipyard.conf",
                                           task_id="t1")
+    dco.config = make_fake_config()
     with pytest.raises(AirflowException) as expected_exc:
         dco.get_doc(99)
     assert "Failed to retrieve deployment" in str(expected_exc)
@@ -134,7 +156,7 @@ def test_get_doc_mock_deckhand(*args):
     dco = DeploymentConfigurationOperator(main_dag_name="main",
                                           shipyard_conf="shipyard.conf",
                                           task_id="t1")
-
+    dco.config = make_fake_config()
     doc = dco.get_doc(99)
     assert doc == 'abcdefg'
 
@@ -146,6 +168,7 @@ def test_get_doc_mock_deckhand_invalid(*args):
     dco = DeploymentConfigurationOperator(main_dag_name="main",
                                           shipyard_conf="shipyard.conf",
                                           task_id="t1")
+    dco.config = make_fake_config()
 
     with pytest.raises(AirflowException) as airflow_ex:
         dco.get_doc(99)

tools/resources/shipyard.conf

@@ -45,3 +45,7 @@ service_type = shipyard
 [oslo_policy]
 policy_file = /etc/shipyard/policy.yaml
 policy_default_rule = deny_all
+[document_info]
+deployment_configuration_name = deployment-configuration
+deployment_configuration_schema = shipyard/DeploymentConfiguration/v1
+deployment_strategy_schema = shipyard/DeploymentStrategy/v1