Merge pull request #154 from Cray-HPE/casmcms-9196-9197-9198

CSM 1.5 Backport of CASMCMS-9196/CASMCMS-9197/CASMCMS-9198

CHANGELOG.md

@@ -6,6 +6,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [1.18.9] - 11/13/2024
+### Changed
+- Do not make database call to look for configuration with null name
+- CASMCMS-9197: Bypass needless code when listing configurations and sources
+
+### Fixed
+- CASMCMS-9196: Use the default value for `authentication_method` when creating a source, if the request does not specify it.
+- CASMCMS-9198: Enforce same value restrictions on CFS options between v2 and v3 (for options that exist in both).
+
 ## [1.18.8] - 11/06/2024
 ### Fixed
 - CASMCMS-9189: Two corrections to the CFS API spec

api/openapi.yaml

@@ -411,61 +411,34 @@ components:
       type: object
       properties:
         hardwareSyncInterval:
-          type: integer
-          description: How frequently the CFS hardware-sync-agent checks with the Hardware State Manager to update its known hardware (in seconds)
-          example: 5
+          $ref: '#/components/schemas/V3Options/properties/hardware_sync_interval'
         batcherCheckInterval:
-          type: integer
-          description: How frequently the batcher checks the configuration states to see if work needs to be done (in seconds)
-          example: 5
+          $ref: '#/components/schemas/V3Options/properties/batcher_check_interval'
         batchSize:
-          type: integer
-          description: The maximum number of nodes the batcher will run a single CFS session against.
-          example: 120
+          $ref: '#/components/schemas/V3Options/properties/batch_size'
         batchWindow:
-          type: integer
-          description: The maximum number of seconds the batcher will wait to run a CFS session once a node has been detected that needs configuration.
-          example: 120
+          $ref: '#/components/schemas/V3Options/properties/batch_window'
         defaultBatcherRetryPolicy:
-          type: integer
-          description: The default maximum number retries per node when configuration fails.
-          example: 1
+          $ref: '#/components/schemas/V3Options/properties/default_batcher_retry_policy'
         defaultPlaybook:
           type: string
           description: The default playbook to be used if not specified in a node's desired state.
           example: site.yml
           pattern: '^[^\s;]*$'
         defaultAnsibleConfig:
-          type: string
-          description: The Kubernetes ConfigMap which holds the default ansible.cfg for a given CFS session. This ConfigMap must be present in the same Kubernetes namespace as the CFS service.
-          example: cfs-default-ansible-cfg
+          $ref: '#/components/schemas/V3Options/properties/default_ansible_config'
         sessionTTL:
-          type: string
-          description: >
-            A time-to-live applied to all completed CFS sessions.
-            Specified in minutes, hours, days, or weeks. e.g. 3d or 24h.
-            Set to an empty string to disable.
-          example: 24h
+          $ref: '#/components/schemas/V3Options/properties/session_ttl'
         additionalInventoryUrl:
-          type: string
-          description: >
-            The git clone URL of a repo with additional inventory files.  All files in the repo will be copied into the hosts directory of CFS.
-          example: https://api-gw-service-nmn.local/vcs/cray/inventory.git
-          pattern: '^[^\s;]*$'
+          $ref: '#/components/schemas/V3Options/properties/additional_inventory_url'
         batcherMaxBackoff:
-          type: integer
-          description: >
-            The maximum number of seconds that batcher will backoff from session creation if problems are detected.
-          example: 3600
+          $ref: '#/components/schemas/V3Options/properties/batcher_max_backoff'
         batcherDisable:
-          type: boolean
-          description: Disables cfs-batcher's automatic session creation if set to True.
+          $ref: '#/components/schemas/V3Options/properties/batcher_disable'
         batcherPendingTimeout:
-          type: integer
-          description: How long cfs-batcher will wait on a pending session before deleting and recreating it (in seconds).
+          $ref: '#/components/schemas/V3Options/properties/batcher_pending_timeout'
         loggingLevel:
-          type: string
-          description: The logging level for core CFS services.  This does not affect the Ansible logging level.
+          $ref: '#/components/schemas/V3Options/properties/logging_level'
       additionalProperties: false
     V3Options:
       description: |

src/server/cray/cfs/api/controllers/configurations.py

@@ -1,7 +1,7 @@
 #
 # MIT License
 #
-# (C) Copyright 2020-2023 Hewlett Packard Enterprise Development LP
+# (C) Copyright 2020-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,6 +21,7 @@
 # ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 # OTHER DEALINGS IN THE SOFTWARE.
 #
+from collections.abc import Container
 import connexion
 from datetime import datetime
 from functools import partial
@@ -72,23 +73,26 @@ def get_configurations_v3(in_use=None, limit=1, after_id=""):
 
 @options.defaults(limit="default_page_size")
 def _get_configurations_data(in_use=None, limit=1, after_id=""):
-    configuration_filter = partial(_configuration_filter, in_use=in_use, in_use_list=_get_in_use_list())
+    # CASMCMS-9197: Only specify a filter if we are actually filtering
+    if in_use is not None:
+        configuration_filter = partial(_configuration_filter, in_use=in_use, in_use_list=_get_in_use_list())
+    else:
+        configuration_filter = None
     configuration_data_page, next_page_exists = DB.get_all(limit=limit, after_id=after_id, data_filter=configuration_filter)
     return configuration_data_page, next_page_exists
 
 
-def _configuration_filter(configuration_data, in_use, in_use_list):
-    if in_use is not None:
-        return _matches_filter(configuration_data, in_use, in_use_list)
-    else:
-        # No filter is being used so all components are valid
-        return True
-
+def _configuration_filter(configuration_data: dict, in_use: bool, in_use_list: Container[str]) -> bool:
+    """
+    If in_use is true:
+        Returns True if the name of the specified configuration is in in_use_list,
+        Returns False otherwise
 
-def _matches_filter(configuration_data, in_use, in_use_list):
-    if in_use is not None and (configuration_data["name"] in in_use_list) != in_use:
-            return False
-    return True
+    If in_use is false:
+        Returns True if the name of the specified configuration is NOT in in_use_list,
+        Returns False otherwise
+    """
+    return (configuration_data["name"] in in_use_list) == in_use
 
 
 def _get_in_use_list():
@@ -436,7 +440,10 @@ def _get_ssl_info(source=None, tmp_dir=""):
 
 class Configurations(object):
     def __init__(self):
-        self.configs = {}
+        # Some callers call the get_config method without checking if the configuration name is
+        # set. If it is not set, calling the database will always just return None, so we can
+        # save ourselves the network traffic of a database call here.
+        self.configs = { "": None, None: None }
 
     """Helper class for other endpoints that need access to configurations"""
     def get_config(self, key):

src/server/cray/cfs/api/controllers/sources.py

@@ -21,6 +21,7 @@
 # ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 # OTHER DEALINGS IN THE SOFTWARE.
 #
+from collections.abc import Container
 import connexion
 from datetime import datetime
 from functools import partial
@@ -58,23 +59,26 @@ def get_sources_v3(in_use=None, limit=1, after_id=""):
 
 @options.defaults(limit="default_page_size")
 def _get_sources_data(in_use=None, limit=1, after_id=""):
-    source_filter = partial(_source_filter, in_use=in_use, in_use_list=_get_in_use_list())
+    # CASMCMS-9197: Only specify a filter if we are actually filtering
+    if in_use is not None:
+        source_filter = partial(_source_filter, in_use=in_use, in_use_list=_get_in_use_list())
+    else:
+        source_filter = None
     source_data_page, next_page_exists = DB.get_all(limit=limit, after_id=after_id, data_filter=source_filter)
     return source_data_page, next_page_exists
 
 
-def _source_filter(source_data, in_use, in_use_list):
-    if in_use is not None:
-        return _matches_filter(source_data, in_use, in_use_list)
-    else:
-        # No filter is being used so all components are valid
-        return True
-
+def _source_filter(source_data: dict, in_use: bool, in_use_list: Container[str]) -> bool:
+    """
+    If in_use is true:
+        Returns True if the name of the specified source is in in_use_list,
+        Returns False otherwise
 
-def _matches_filter(source_data, in_use, in_use_list):
-    if in_use is not None and (source_data["name"] in in_use_list) != in_use:
-            return False
-    return True
+    If in_use is false:
+        Returns True if the name of the specified source is NOT in in_use_list,
+        Returns False otherwise
+    """
+    return (source_data["name"] in in_use_list) == in_use
 
 
 def _get_in_use_list():
@@ -127,6 +131,13 @@ def post_source_v3():
             status=400, title="Error parsing the data provided.",
             detail=str(err))
 
+    # CASMCMS-9196: connexion does not fill in default values for parameters in the request
+    # body. So here we set the default value for authentication_method, if needed. Note that
+    # connexion DOES validate that the request is valid, so we know that the credentials field
+    # is present.
+    if "authentication_method" not in data["credentials"]:
+        data["credentials"]["authentication_method"] = "password"
+
     error = _validate_source(data)
     if error:
         return error