#493 Load subject types from configuration
Merged 4 years ago by lholecek. Opened 4 years ago by lholecek.
lholecek/greenwave refactor-subjects-2  into  master

Load subject types from configuration
Lukas Holecek • 4 years ago  
@@ -0,0 +1,3 @@ 

+ --- !SubjectType

+ id: bodhi_update

+ ignore_missing_policy: true

@@ -0,0 +1,9 @@ 

+ --- !SubjectType

+ id: compose

+ item_key: "productmd.compose.id"

+ item_dict:

+   # {"productmd.compose.id": ITEM}

+   item_key: "productmd.compose.id"

+ latest_result_unique_keys:

+   - arch_variant

+   - system_architecture

@@ -0,0 +1,15 @@ 

+ --- !SubjectType

+ id: koji_build

+ aliases:

+   - brew-build

+ is_koji_build: true

+ is_nvr: true

+ supports_remote_rule: true

+ item_key: "original_spec_nvr"

+ result_queries:

+   # {"type": "koji_build,brew-build", "item": ITEM}

+   - item_key: "item"

+     keys:

+       type: "koji_build,brew-build"

+   # {"original_spec_nvr": ITEM}

+   - item_key: "original_spec_nvr"

@@ -0,0 +1,4 @@ 

+ --- !SubjectType

+ id: redhat-container-image

+ product_version: rhel-8

+ supports_remote_rule: true

@@ -0,0 +1,4 @@ 

+ --- !SubjectType

+ id: redhat-module

+ product_version: rhel-8

+ supports_remote_rule: true

file modified
+1
@@ -11,6 +11,7 @@ 

        - ./docker/home:/home/dev:Z

        - ./docker/greenwave-settings.py:/etc/greenwave/settings.py:ro,z

        - ./conf/policies/:/etc/greenwave/policies/:ro,z

+       - ./conf/subject_types/:/etc/greenwave/subject_types/:ro,z

      ports:

        - 8080:8080

      depends_on:

file modified
+39 -44
@@ -15,6 +15,11 @@ 

  from greenwave.safe_yaml import SafeYAMLError

  from greenwave.utils import insert_headers, jsonp

  from greenwave.waivers import waive_answers

+ from greenwave.subjects.factory import (

+     create_subject,

+     create_subject_from_data,

+     UnknownSubjectDataError,

+ )

  from greenwave.monitor import (

      registry,

      decision_exception_counter,
@@ -26,23 +31,14 @@ 

  log = logging.getLogger(__name__)

  

  

- def _decision_subject(subject):

-     subject_type = subject.get('type')

-     subject_identifier = subject.get('item')

- 

-     if 'productmd.compose.id' in subject:

-         return ('compose', subject['productmd.compose.id'])

- 

-     if 'original_spec_nvr' in subject:

-         return ('koji_build', subject['original_spec_nvr'])

- 

-     if subject_identifier:

-         if subject_type == 'brew-build':

-             return ('koji_build', subject_identifier)

-         return (subject_type, subject_identifier)

+ def _decision_subject(data):

+     try:

+         subject = create_subject_from_data(data)

+     except UnknownSubjectDataError:

+         log.info('Could not detect subject_identifier.')

+         raise BadRequest('Could not detect subject_identifier.')

  

-     log.info('Couldn\'t detect subject_identifier.')

-     raise BadRequest('Couldn\'t detect subject_identifier.')

+     return subject

  

  

  def _decision_subjects_for_request(data):
@@ -72,18 +68,7 @@ 

              log.error('Missing required "subject_identifier" parameter')

              raise BadRequest('Missing required "subject_identifier" parameter')

  

-         yield data['subject_type'], data['subject_identifier']

- 

- 

- def subject_type_identifier_to_list(subject_type, subject_identifier):

-     """

-     Inverse of the above function.

-     This is for backwards compatibility in emitted messages.

-     """

-     if subject_type == 'compose':

-         return [{'productmd.compose.id': subject_identifier}]

-     else:

-         return [{'type': subject_type, 'item': subject_identifier}]

+         yield create_subject(data['subject_type'], data['subject_identifier'])

  

  

  @api.route('/version', methods=['GET'])
@@ -171,6 +156,18 @@ 

      return resp

  

  

+ @api.route('/subject_types', methods=['GET'])

+ @jsonp

+ def get_subject_types():

+     """ Returns all currently loaded subject_types (sorted by "id")."""

+     subject_types = [type_.to_json() for type_ in current_app.config['subject_types']]

+     subject_types.sort(key=lambda x: x['id'])

+     resp = jsonify({'subject_types': subject_types})

+     resp = insert_headers(resp)

+     resp.status_code = 200

+     return resp

+ 

+ 

  @api.route('/decision', methods=['OPTIONS'])

  @jsonp

  def make_decision_options():
@@ -416,9 +413,9 @@ 

      on_demand_policies = []

      if rules:

          request_data = {key: data[key] for key in data if key not in ('subject', 'subject_type')}

-         for subject_type, subject_identifier in _decision_subjects_for_request(data):

-             request_data['subject_type'] = subject_type

-             request_data['subject_identifier'] = subject_identifier

+         for subject in _decision_subjects_for_request(data):

+             request_data['subject_type'] = subject.type

+             request_data['subject_identifier'] = subject.identifier

              on_demand_policy = OnDemandPolicy.create_from_json(request_data)

              on_demand_policies.append(on_demand_policy)

  
@@ -454,34 +451,32 @@ 

      waiver_filters = []

  

      policies = on_demand_policies or current_app.config['policies']

-     for subject_type, subject_identifier in _decision_subjects_for_request(data):

+     for subject in _decision_subjects_for_request(data):

          subject_policies = [

              policy for policy in policies

              if policy.matches(

                  decision_context=decision_context,

                  product_version=product_version,

-                 subject_type=subject_type)

+                 subject=subject)

          ]

  

          if not subject_policies:

-             # Ignore non-existent policy for Bodhi updates.

-             if subject_type == 'bodhi_update':

+             if subject.ignore_missing_policy:

                  continue

  

              log.error(

                  'Cannot find any applicable policies for %s subjects at gating point %s in %s',

-                 subject_type, decision_context, product_version)

+                 subject.type, decision_context, product_version)

              raise NotFound(

                  'Cannot find any applicable policies for %s subjects at gating point %s in %s' % (

-                     subject_type, decision_context, product_version))

+                     subject.type, decision_context, product_version))

  

          if verbose:

              # Retrieve test results and waivers for all items when verbose output is requested.

-             verbose_results.extend(

-                 results_retriever.retrieve(subject_type, subject_identifier))

+             verbose_results.extend(results_retriever.retrieve(subject))

              waiver_filters.append(dict(

-                 subject_type=subject_type,

-                 subject_identifier=subject_identifier,

+                 subject_type=subject.type,

+                 subject_identifier=subject.identifier,

                  product_version=product_version,

              ))

  
@@ -489,7 +484,7 @@ 

              answers.extend(

                  policy.check(

                      product_version,

-                     subject_identifier,

+                     subject,

                      results_retriever))

  

          applicable_policies.extend(subject_policies)
@@ -498,8 +493,8 @@ 

          for answer in answers:

              if not answer.is_satisfied:

                  waiver_filters.append(dict(

-                     subject_type=answer.subject_type,

-                     subject_identifier=answer.subject_identifier,

+                     subject_type=answer.subject.type,

+                     subject_identifier=answer.subject.identifier,

                      product_version=product_version,

                      testcase=answer.test_case_name,

                  ))

@@ -6,6 +6,7 @@ 

  from greenwave.monitor import monitor_api

  from greenwave.utils import json_error, load_config, sha1_mangle_key

  from greenwave.policies import load_policies, RemoteRule

+ from greenwave.subjects.subject_type import load_subject_types

  

  from dogpile.cache import make_region

  from requests import ConnectionError, Timeout
@@ -43,6 +44,10 @@ 

      log.debug("config: Loading policies from %r", policies_dir)

      app.config['policies'] = load_policies(policies_dir)

  

+     subject_types_dir = app.config['SUBJECT_TYPES_DIR']

+     log.debug("config: Loading subject types from %r", subject_types_dir)

+     app.config['subject_types'] = load_subject_types(subject_types_dir)

+ 

      if not _can_use_remote_rule(app.config) and _has_remote_rule(app.config['policies']):

          raise RuntimeError(

              "If you want to apply a RemoteRule"

file modified
+10 -8
@@ -2,6 +2,11 @@ 

  import os

  

  

+ def _local_conf_dir(subdir):

+     basedir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))

+     return os.path.join(basedir, 'conf', subdir)

+ 

+ 

  class Config(object):

      """

      A GreenWave Flask configuration.
@@ -26,6 +31,7 @@ 

      REQUESTS_VERIFY = True

  

      POLICIES_DIR = '/etc/greenwave/policies'

+     SUBJECT_TYPES_DIR = '/etc/greenwave/subject_types'

  

      MESSAGING = 'fedmsg'

  
@@ -46,10 +52,8 @@ 

      #WAIVERDB_API_URL = 'http://waiverdb-dev.fedorainfracloud.org/api/v1.0'

      WAIVERDB_API_URL = 'http://localhost:5004/api/v1.0'

      GREENWAVE_API_URL = 'http://localhost:5005/api/v1.0'

-     POLICIES_DIR = os.path.join(

-         os.path.dirname(os.path.dirname(os.path.abspath(__file__))), 'conf',

-         'policies'

-     )

+     POLICIES_DIR = _local_conf_dir('policies')

+     SUBJECT_TYPES_DIR = _local_conf_dir('subject_types')

  

  

  class TestingConfig(Config):
@@ -57,7 +61,5 @@ 

      WAIVERDB_API_URL = 'http://localhost:5004/api/v1.0'

      GREENWAVE_API_URL = 'http://localhost:5005/api/v1.0'

      KOJI_BASE_URL = 'http://localhost:5006/kojihub'

-     POLICIES_DIR = os.path.join(

-         os.path.dirname(os.path.dirname(os.path.abspath(__file__))), 'conf',

-         'policies'

-     )

+     POLICIES_DIR = _local_conf_dir('policies')

+     SUBJECT_TYPES_DIR = _local_conf_dir('subject_types')

@@ -6,7 +6,6 @@ 

  import fedmsg.consumers

  

  import greenwave.app_factory

- from greenwave.api_v1 import subject_type_identifier_to_list

  from greenwave.monitor import (

      publish_decision_exceptions_result_counter,

      messaging_tx_to_send_counter, messaging_tx_stopped_counter,
@@ -157,15 +156,13 @@ 

      def _publish_decision_change(

              self,

              submit_time,

-             subject_type,

-             subject_identifier,

+             subject,

              testcase,

              product_version,

              publish_testcase):

  

          policy_attributes = dict(

-             subject_type=subject_type,

-             subject_identifier=subject_identifier,

+             subject=subject,

              testcase=testcase,

          )

  
@@ -185,8 +182,8 @@ 

                  submit_time,

                  decision_context=decision_context,

                  product_version=product_version,

-                 subject_type=subject_type,

-                 subject_identifier=subject_identifier,

+                 subject_type=subject.type,

+                 subject_identifier=subject.identifier,

              )

              if decision is None:

                  continue
@@ -197,11 +194,10 @@ 

                  continue

  

              decision.update({

-                 'subject_type': subject_type,

-                 'subject_identifier': subject_identifier,

+                 'subject_type': subject.type,

+                 'subject_identifier': subject.identifier,

                  # subject is for backwards compatibility only:

-                 'subject': subject_type_identifier_to_list(subject_type,

-                                                            subject_identifier),

+                 'subject': [subject.to_dict()],

                  'decision_context': decision_context,

                  'product_version': product_version,

                  'previous': old_decision,

@@ -13,6 +13,10 @@ 

  

  from greenwave.consumers.consumer import Consumer

  from greenwave.product_versions import subject_product_version

+ from greenwave.subjects.factory import (

+     create_subject_from_data,

+     UnknownSubjectDataError,

+ )

  

  import xmlrpc.client

  
@@ -64,9 +68,9 @@ 

              self.koji_proxy = None

  

      @staticmethod

-     def announcement_subjects(message):

+     def announcement_subject(message):

          """

-         Yields pairs of (subject type, subject identifier) for announcement

+         Returns pairs of (subject type, subject identifier) for announcement

          consideration from the message.

  

          Args:
@@ -78,7 +82,16 @@ 

          except KeyError:

              data = message['msg']['task']  # Old format

  

-         _type = _unpack_value(data.get('type'))

+         unpacked = {

+             k: _unpack_value(v)

+             for k, v in data.items()

+         }

+ 

+         try:

+             subject = create_subject_from_data(unpacked)

+         except UnknownSubjectDataError:

+             return None

+ 

          # note: it is *intentional* that we do not handle old format

          # compose-type messages, because it is impossible to reliably

          # produce a decision from these. compose decisions can only be
@@ -88,21 +101,10 @@ 

          # https://pagure.io/taskotron/resultsdb/issue/92

          # https://pagure.io/taskotron/resultsdb/pull-request/101

          # https://pagure.io/greenwave/pull-request/262#comment-70350

-         if 'productmd.compose.id' in data:

-             yield ('compose', _unpack_value(data['productmd.compose.id']))

-         elif _type == 'compose':

-             pass

-         elif 'original_spec_nvr' in data:

-             nvr = _unpack_value(data['original_spec_nvr'])

-             # when the pipeline ignores a package, which happens

-             # *a lot*, we get a message with an 'original_spec_nvr'

-             # key with an empty value; let's not try and handle this

-             if nvr:

-                 yield ('koji_build', nvr)

-         elif _type == 'brew-build':

-             yield ('koji_build', _unpack_value(data['item']))

-         elif 'item' in data and _type:

-             yield (_type, _unpack_value(data['item']))

+         if subject.type == 'compose' and 'productmd.compose.id' not in data:

+             return None

+ 

+         return subject

  

      def _consume_message(self, message):

          msg = message['msg']
@@ -119,21 +121,22 @@ 

  

          brew_task_id = _get_brew_task_id(msg)

  

-         for subject_type, subject_identifier in self.announcement_subjects(message):

-             log.debug('Considering subject %s: %r', subject_type, subject_identifier)

- 

-             product_version = subject_product_version(

-                 subject_identifier,

-                 subject_type,

-                 self.koji_proxy,

-                 brew_task_id,

-             )

- 

-             self._publish_decision_change(

-                 submit_time=submit_time,

-                 subject_type=subject_type,

-                 subject_identifier=subject_identifier,

-                 testcase=testcase,

-                 product_version=product_version,

-                 publish_testcase=False,

-             )

+         subject = self.announcement_subject(message)

+         if subject is None:

+             return

+ 

+         log.debug('Considering subject: %r', subject)

+ 

+         product_version = subject_product_version(

+             subject,

+             self.koji_proxy,

+             brew_task_id,

+         )

+ 

+         self._publish_decision_change(

+             submit_time=submit_time,

+             subject=subject,

+             testcase=testcase,

+             product_version=product_version,

+             publish_testcase=False,

+         )

@@ -10,6 +10,7 @@ 

  """

  

  from greenwave.consumers.consumer import Consumer

+ from greenwave.subjects.factory import create_subject

  

  

  class WaiverDBHandler(Consumer):
@@ -30,14 +31,12 @@ 

  

          product_version = msg['product_version']

          testcase = msg['testcase']

-         subject_type = msg['subject_type']

-         subject_identifier = msg['subject_identifier']

+         subject = create_subject(msg['subject_type'], msg['subject_identifier'])

          submit_time = msg['timestamp']

  

          self._publish_decision_change(

              submit_time=submit_time,

-             subject_type=subject_type,

-             subject_identifier=subject_identifier,

+             subject=subject,

              testcase=testcase,

              product_version=product_version,

              publish_testcase=True,

file modified
+62 -117
@@ -11,7 +11,6 @@ 

  from greenwave.utils import remove_duplicates, to_hashable

  from greenwave.safe_yaml import (

      SafeYAMLBool,

-     SafeYAMLChoice,

      SafeYAMLList,

      SafeYAMLObject,

      SafeYAMLString,
@@ -42,17 +41,6 @@ 

      pass

  

  

- def subject_type_identifier_to_item(subject_type, subject_identifier):

-     """

-     Greenwave < 0.8 included an "item" key in the "unsatisfied_requirements".

-     This returns a suitable value for that key, for backwards compatibility.

-     """

-     if subject_type == 'compose':

-         return {'productmd.compose.id': subject_identifier}

-     else:

-         return {'type': subject_type, 'item': subject_identifier}

- 

- 

  class Answer(object):

      """

      Represents the result of evaluating a policy rule against a particular
@@ -118,9 +106,8 @@ 

      ResultsDB with a matching item and test case name).

      """

  

-     def __init__(self, subject_type, subject_identifier, test_case_name, scenario):

-         self.subject_type = subject_type

-         self.subject_identifier = subject_identifier

+     def __init__(self, subject, test_case_name, scenario):

+         self.subject = subject

          self.test_case_name = test_case_name

          self.scenario = scenario

  
@@ -128,17 +115,16 @@ 

          return {

              'type': 'test-result-missing',

              'testcase': self.test_case_name,

-             'subject_type': self.subject_type,

-             'subject_identifier': self.subject_identifier,

+             'subject_type': self.subject.type,

+             'subject_identifier': self.subject.identifier,

              'scenario': self.scenario,

              # For backwards compatibility only:

-             'item': subject_type_identifier_to_item(self.subject_type, self.subject_identifier),

+             'item': self.subject.to_dict()

          }

  

      def to_waived(self):

          return TestResultMissingWaived(

-             self.subject_type,

-             self.subject_identifier,

+             self.subject,

              self.test_case_name,

              self.scenario)

  
@@ -147,9 +133,8 @@ 

      """

      Same as TestResultMissing but the result was waived.

      """

-     def __init__(self, subject_type, subject_identifier, test_case_name, scenario):

-         self.subject_type = subject_type

-         self.subject_identifier = subject_identifier

+     def __init__(self, subject, test_case_name, scenario):

+         self.subject = subject

          self.test_case_name = test_case_name

          self.scenario = scenario

  
@@ -157,8 +142,8 @@ 

          return {

              'type': 'test-result-missing-waived',

              'testcase': self.test_case_name,

-             'subject_type': self.subject_type,

-             'subject_identifier': self.subject_identifier,

+             'subject_type': self.subject.type,

+             'subject_identifier': self.subject.identifier,

              'scenario': self.scenario,

          }

  
@@ -169,9 +154,8 @@ 

      not ``PASSED`` or ``INFO``) and no corresponding waiver was found.

      """

  

-     def __init__(self, subject_type, subject_identifier, test_case_name, scenario, result_id):

-         self.subject_type = subject_type

-         self.subject_identifier = subject_identifier

+     def __init__(self, subject, test_case_name, scenario, result_id):

+         self.subject = subject

          self.test_case_name = test_case_name

          self.scenario = scenario

          self.result_id = result_id
@@ -184,7 +168,7 @@ 

              # These are for backwards compatibility only

              # (the values are already visible in the result data itself, the

              # caller shouldn't need them repeated here):

-             'item': subject_type_identifier_to_item(self.subject_type, self.subject_identifier),

+             'item': self.subject.to_dict(),

              'scenario': self.scenario,

          }

  
@@ -197,9 +181,8 @@ 

      Remote policy parsing failed.

      """

  

-     def __init__(self, subject_type, subject_identifier, test_case_name, details):

-         self.subject_type = subject_type

-         self.subject_identifier = subject_identifier

+     def __init__(self, subject, test_case_name, details):

+         self.subject = subject

          self.test_case_name = test_case_name

          self.details = details

  
@@ -207,8 +190,8 @@ 

          return {

              'type': 'invalid-gating-yaml',

              'testcase': self.test_case_name,

-             'subject_type': self.subject_type,

-             'subject_identifier': self.subject_identifier,

+             'subject_type': self.subject.type,

+             'subject_identifier': self.subject.identifier,

              'details': self.details

          }

  
@@ -223,16 +206,15 @@ 

  

      test_case_name = 'missing-gating-yaml'

  

-     def __init__(self, subject_type, subject_identifier):

-         self.subject_type = subject_type

-         self.subject_identifier = subject_identifier

+     def __init__(self, subject):

+         self.subject = subject

  

      def to_json(self):

          return {

              'type': 'missing-gating-yaml',

              'testcase': self.test_case_name,

-             'subject_type': self.subject_type,

-             'subject_identifier': self.subject_identifier,

+             'subject_type': self.subject.type,

+             'subject_identifier': self.subject.identifier,

          }

  

      def to_waived(self):
@@ -333,7 +315,7 @@ 

              self,

              policy,

              product_version,

-             subject_identifier,

+             subject,

              results_retriever):

          """

          Evaluate this policy rule for the given item.
@@ -341,7 +323,7 @@ 

          Args:

              policy (Policy): Parent policy of the rule

              product_version (str): Product version we are making a decision about

-             subject_identifier (str): Item we are making a decision about (for

+             subject (Subject): Item we are making a decision about (for

                  example, Koji build NVR, Bodhi update id, ...)

              results_retriever (ResultsRetriever): Object for retrieving data

                  from ResultsDB.
@@ -402,13 +384,13 @@ 

          'required': SafeYAMLBool(optional=True, default=False),

      }

  

-     def _get_sub_policies(self, policy, subject_identifier):

-         if policy.subject_type not in ['koji_build', 'redhat-module', 'redhat-container-image']:

+     def _get_sub_policies(self, policy, subject):

+         if not subject.supports_remote_rule:

              return []

  

          try:

              pkg_namespace, pkg_name, rev = greenwave.resources.retrieve_scm_from_koji(

-                 subject_identifier

+                 subject.identifier

              )

          except greenwave.resources.NoSourceException as e:

              log.error(e)
@@ -442,26 +424,25 @@ 

              self,

              policy,

              product_version,

-             subject_identifier,

+             subject,

              results_retriever):

          try:

-             policies = self._get_sub_policies(policy, subject_identifier)

+             policies = self._get_sub_policies(policy, subject)

          except SafeYAMLError as e:

              return [

-                 InvalidGatingYaml(

-                     policy.subject_type, subject_identifier, 'invalid-gating-yaml', str(e))

+                 InvalidGatingYaml(subject, 'invalid-gating-yaml', str(e))

              ]

  

          if policies is None:

              if self.required:

-                 return [MissingGatingYaml(policy.subject_type, subject_identifier)]

+                 return [MissingGatingYaml(subject)]

              return []

  

          answers = []

          for remote_policy in policies:

              if remote_policy.matches_product_version(product_version):

                  response = remote_policy.check(

-                     product_version, subject_identifier, results_retriever)

+                     product_version, subject, results_retriever)

  

                  if isinstance(response, list):

                      answers.extend(response)
@@ -472,22 +453,21 @@ 

  

      def matches(self, policy, **attributes):

          #pylint: disable=broad-except

-         subject_identifier = attributes.get('subject_identifier')

-         if not subject_identifier:

+         subject = attributes.get('subject')

+         if not subject:

              return True

  

          sub_policies = []

          try:

-             sub_policies = self._get_sub_policies(policy, subject_identifier)

+             sub_policies = self._get_sub_policies(policy, subject)

          except SafeYAMLError:

-             logging.exception(

-                 'Failed to parse policies for %r', subject_identifier)

+             logging.exception('Failed to parse policies for %r', subject)

          except Exception:

-             logging.exception(

-                 'Failed to retrieve policies for %r', subject_identifier)

+             logging.exception('Failed to retrieve policies for %r', subject)

  

          if sub_policies is None:

-             return False

+             # RemoteRule matches if remote policy file is missing.

+             return True

  

          return any(sub_policy.matches(**attributes) for sub_policy in sub_policies)

  
@@ -514,10 +494,9 @@ 

              self,

              policy,

              product_version,

-             subject_identifier,

+             subject,

              results_retriever):

-         matching_results = results_retriever.retrieve(

-             policy.subject_type, subject_identifier, self.test_case_name)

+         matching_results = results_retriever.retrieve(subject, self.test_case_name)

  

          if self.scenario is not None:

              matching_results = [
@@ -526,43 +505,15 @@ 

  

          # Investigate the absence of result first.

          if not matching_results:

-             return TestResultMissing(

-                 policy.subject_type, subject_identifier, self.test_case_name, self.scenario)

- 

-         # For compose make decisions based on all architectures and variants.

-         if policy.subject_type == 'compose':

-             visited_arch_variants = set()

-             answers = []

-             for result in matching_results:

-                 result_data = result['data']

- 

-                 # Items under test result "data" are lists which are unhashable

-                 # types in Python. This converts anything that is stored there

-                 # to a string so we don't have to care about the stored value.

-                 arch_variant = (

-                     str(result_data.get('system_architecture')),

-                     str(result_data.get('system_variant')))

- 

-                 if arch_variant not in visited_arch_variants:

-                     visited_arch_variants.add(arch_variant)

-                     answer = self._answer_for_result(

-                         result,

-                         policy.subject_type,

-                         subject_identifier)

-                     answers.append(answer)

- 

-             return answers

+             return TestResultMissing(subject, self.test_case_name, self.scenario)

  

          # If we find multiple matching results, we always use the first one which

          # will be the latest chronologically, because ResultsDB always returns

          # results ordered by `submit_time` descending.

-         answers = []

-         for result in matching_results:

-             answers.append(self._answer_for_result(

-                 result,

-                 policy.subject_type,

-                 subject_identifier))

-         return answers

+         return [

+             self._answer_for_result(result, subject)

+             for result in subject.get_latest_results(matching_results)

+         ]

  

      def matches(self, policy, **attributes):

          testcase = attributes.get('testcase')
@@ -575,8 +526,7 @@ 

              'scenario': self.scenario,

          }

  

-     def _answer_for_result(

-             self, result, subject_type, subject_identifier):

+     def _answer_for_result(self, result, subject):

          if result['outcome'] in ('PASSED', 'INFO'):

              log.debug('Test result passed for the result_id %s and testcase %s,'

                        ' because the outcome is %s', result['id'], self.test_case_name,
@@ -584,17 +534,14 @@ 

              return TestResultPassed(self.test_case_name, result['id'])

  

          if result['outcome'] in ('QUEUED', 'RUNNING'):

-             log.debug('Test result MISSING for the subject_type %s, subject_identifier %s and '

-                       'testcase %s, because the outcome is %s', subject_type, subject_identifier,

+             log.debug('Test result MISSING for the %s and '

+                       'testcase %s, because the outcome is %s', subject,

                        self.test_case_name, result['outcome'])

-             return TestResultMissing(subject_type, subject_identifier, self.test_case_name,

-                                      self.scenario)

-         log.debug('Test result failed for the subject_type %s, subject_identifier %s and '

+             return TestResultMissing(subject, self.test_case_name, self.scenario)

+         log.debug('Test result failed for the %s and '

                    'testcase %s, because the outcome is %s and it didn\'t match any of the '

-                   'previous cases', subject_type, subject_identifier,

-                   self.test_case_name, result['outcome'])

-         return TestResultFailed(subject_type, subject_identifier, self.test_case_name,

-                                 self.scenario, result['id'])

+                   'previous cases', subject, self.test_case_name, result['outcome'])

+         return TestResultFailed(subject, self.test_case_name, self.scenario, result['id'])

  

  

  class ObsoleteRule(Rule):
@@ -613,7 +560,7 @@ 

              self,

              policy,

              product_version,

-             subject_identifier,

+             subject,

              results_retriever):

          raise ValueError('This rule is obsolete and can\'t be checked')

  
@@ -634,7 +581,6 @@ 

          'id': SafeYAMLString(),

          'product_versions': SafeYAMLList(str),

          'decision_context': SafeYAMLString(),

-         # TODO: Handle brew-build value better.

          'subject_type': SafeYAMLString(),

          'rules': SafeYAMLList(Rule),

          'blacklist': SafeYAMLList(str, optional=True),
@@ -662,8 +608,8 @@ 

          if product_version and not self.matches_product_version(product_version):

              return False

  

-         subject_type = attributes.get('subject_type')

-         if subject_type and subject_type != self.subject_type:

+         subject = attributes.get('subject')

+         if subject and subject.type != self.subject_type:

              return False

  

          return not self.rules or any(rule.matches(self, **attributes) for rule in self.rules)
@@ -672,16 +618,16 @@ 

      def check(

              self,

              product_version,

-             subject_identifier,

+             subject,

              results_retriever):

          # If an item is about a package and it is in the blacklist, return RuleSatisfied()

-         if self.subject_type == 'koji_build':

-             name = subject_identifier.rsplit('-', 2)[0]

+         name = subject.package_name

+         if name:

              if name in self.blacklist:

-                 return [BlacklistedInPolicy(subject_identifier) for rule in self.rules]

+                 return [BlacklistedInPolicy(subject.identifier) for rule in self.rules]

              for exclude in self.excluded_packages:

                  if fnmatch(name, exclude):

-                     return [ExcludedInPolicy(subject_identifier) for rule in self.rules]

+                     return [ExcludedInPolicy(subject.identifier) for rule in self.rules]

              if self.packages and not any(fnmatch(name, package) for package in self.packages):

                  # If the `packages` whitelist is set and this package isn't in the

                  # `packages` whitelist, then the policy doesn't apply to it
@@ -691,7 +637,7 @@ 

              response = rule.check(

                  self,

                  product_version,

-                 subject_identifier,

+                 subject,

                  results_retriever)

              if isinstance(response, list):

                  answers.extend(response)
@@ -756,8 +702,7 @@ 

      safe_yaml_attributes = {

          'id': SafeYAMLString(optional=True),

          'product_versions': SafeYAMLList(str),

-         'subject_type': SafeYAMLChoice(

-             'koji_build', 'redhat-module', 'redhat-container-image', optional=True),

+         'subject_type': SafeYAMLString(optional=True, default='koji_build'),

          'decision_context': SafeYAMLString(),

          'rules': SafeYAMLList(Rule),

          'blacklist': SafeYAMLList(str, optional=True),

file modified
+10 -16
@@ -56,24 +56,18 @@ 

  

  

  def subject_product_version(

-         subject_identifier,

-         subject_type,

+         subject,

          koji_proxy=None,

          koji_task_id=None):

-     if subject_type == 'koji_build':

-         try:

-             _, _, release = subject_identifier.rsplit('-', 2)

-             _, short_prod_version = release.rsplit('.', 1)

-             return _guess_product_version(short_prod_version, koji_build=True)

-         except (KeyError, ValueError):

-             pass

+     if subject.product_version:

+         return subject.product_version

  

-     if subject_type == "compose":

-         return _guess_product_version(subject_identifier)

+     if subject.short_product_version:

+         product_version = _guess_product_version(

+             subject.short_product_version, koji_build=subject.is_koji_build)

+         if product_version:

+             return product_version

  

-     if subject_type in ("redhat-module", "redhat-container-image"):

-         return "rhel-8"

- 

-     if koji_proxy:

+     if koji_proxy and subject.is_koji_build:

          return _guess_koji_build_product_version(

-             subject_identifier, koji_proxy, koji_task_id)

+             subject.identifier, koji_proxy, koji_task_id)

file modified
+6 -19
@@ -54,10 +54,10 @@ 

          super().__init__(**args)

          self.cache = {}

  

-     def _retrieve_all(self, subject_type, subject_identifier, testcase=None, scenarios=None):

+     def _retrieve_all(self, subject, testcase=None, scenarios=None):

          # Get test case result from cache if all test case results were already

-         # retrieved for given subject type/ID.

-         cache_key = (subject_type, subject_identifier, scenarios)

+         # retrieved for given Subject.

+         cache_key = (subject.type, subject.identifier, scenarios)

          if testcase and cache_key in self.cache:

              for result in self.cache[cache_key]:

                  if result['testcase']['name'] == testcase:
@@ -75,22 +75,9 @@ 

              params.update({'scenario': ','.join(scenarios)})

  

          results = []

-         if subject_type == 'koji_build':

-             params['type'] = 'koji_build,brew-build'

-             params['item'] = subject_identifier

-             results = self._retrieve_data(params)

- 

-             del params['type']

-             del params['item']

-             params['original_spec_nvr'] = subject_identifier

-             results.extend(self._retrieve_data(params))

-         elif subject_type == 'compose':

-             params['productmd.compose.id'] = subject_identifier

-             results = self._retrieve_data(params)

-         else:

-             params['type'] = subject_type

-             params['item'] = subject_identifier

-             results = self._retrieve_data(params)

+         for query in subject.result_queries():

+             query.update(params)

+             results.extend(self._retrieve_data(query))

  

          if not testcase:

              self.cache[cache_key] = results

file modified
+25 -22
@@ -30,59 +30,62 @@ 

          raise NotImplementedError()

  

  

- class SafeYAMLBool(SafeYAMLAttribute):

+ class SafeYAMLDict(SafeYAMLAttribute):

      """

-     YAML object attribute representing a boolean value.

+     YAML object attribute representing a dict.

      """

-     def __init__(self, default=False, **args):

+     def __init__(self, **args):

          super().__init__(**args)

-         self.default = default

  

      def from_yaml(self, loader, node):

-         value = loader.construct_scalar(node)

-         value = yaml.safe_load(value)

-         if isinstance(value, bool):

+         value = loader.construct_mapping(node)

+         if isinstance(value, dict):

              return value

  

-         raise SafeYAMLError('Expected a boolean value, got: {}'.format(value))

+         raise SafeYAMLError('Expected a dict value, got: {}'.format(value))

  

      def to_json(self, value):

          return value

  

      @property

      def default_value(self):

-         return self.default

+         return {}

  

  

- class SafeYAMLString(SafeYAMLAttribute):

+ class SafeYAMLBool(SafeYAMLAttribute):

      """

-     YAML object attribute representing a string value.

+     YAML object attribute representing a boolean value.

      """

+     def __init__(self, default=False, **args):

+         super().__init__(**args)

+         self.default = default

+ 

      def from_yaml(self, loader, node):

          value = loader.construct_scalar(node)

-         return str(value)

+         value = yaml.safe_load(value)

+         if isinstance(value, bool):

+             return value

+ 

+         raise SafeYAMLError('Expected a boolean value, got: {}'.format(value))

  

      def to_json(self, value):

          return value

  

      @property

      def default_value(self):

-         return None

+         return self.default

  

  

- class SafeYAMLChoice(SafeYAMLAttribute):

+ class SafeYAMLString(SafeYAMLAttribute):

      """

-     YAML object attribute with only specific values allowed.

+     YAML object attribute representing a string value.

      """

-     def __init__(self, *values, **kwargs):

-         super().__init__(**kwargs)

-         self.values = values

+     def __init__(self, default=None, **args):

+         super().__init__(**args)

+         self.default = default

  

      def from_yaml(self, loader, node):

          value = loader.construct_scalar(node)

-         if value not in self.values:

-             raise SafeYAMLError(

-                 'Value must be one of: {}'.format(', '.join(self.values)))

          return str(value)

  

      def to_json(self, value):
@@ -90,7 +93,7 @@ 

  

      @property

      def default_value(self):

-         return self.values[0]

+         return self.default

  

  

  class SafeYAMLList(SafeYAMLAttribute):

@@ -0,0 +1,48 @@ 

+ # SPDX-License-Identifier: GPL-2.0+

+ """

+ Factory functions and helper functions to create subject object.

+ """

+ 

+ from flask import current_app

+ 

+ from .subject import Subject

+ from .subject_type import create_subject_type

+ 

+ 

+ class UnknownSubjectDataError(RuntimeError):

+     pass

+ 

+ 

+ def subject_types():

+     return current_app.config['subject_types']

+ 

+ 

+ def create_subject_from_data(data):

+     """

+     Returns instance of greenwave.subject.Subject created from data dict.

+ 

+     :raises: UnknownSubjectDataError: if subject instance cannot be created

+             from data (in case data are invalid or subject configuration is

+             missing)

+     """

+     for type_ in subject_types():

+         if not type_.item_key:

+             continue

+ 

+         item = data.get(type_.item_key)

+         if not item:

+             continue

+ 

+         return Subject(type_, item)

+ 

+     type_id = data.get("type")

+     item = data.get("item")

+     if type_id and item:

+         return create_subject(type_id, item)

+ 

+     raise UnknownSubjectDataError()

+ 

+ 

+ def create_subject(type_id, item):

+     type_ = create_subject_type(type_id, subject_types())

+     return Subject(type_, item)

@@ -0,0 +1,133 @@ 

+ # SPDX-License-Identifier: GPL-2.0+

+ 

+ 

+ def _get_latest_results(results, unique_keys):

+     """

+     Yields only the latest results for unique architecture and variant pairs.

+ 

+     The input results are sorted from latest to oldest.

+     """

+     visited_arch_variants = set()

+     for result in results:

+         result_data = result["data"]

+ 

+         # Items under test result "data" are lists which are unhashable

+         # types in Python. This converts anything that is stored there

+         # to a string so we don't have to care about the stored value.

+         arch_variant = tuple(

+             str(result_data.get(key))

+             for key in unique_keys

+         )

+ 

+         if arch_variant not in visited_arch_variants:

+             visited_arch_variants.add(arch_variant)

+             yield result

+ 

+ 

+ def _to_dict(format_dict, type_, item):

+     result = {}

+ 

+     item_key = format_dict.get('item_key')

+     if item_key:

+         result[item_key] = item

+ 

+     keys = format_dict.get('keys', {})

+     for key, value in keys.items():

+         result[key] = value

+ 

+     return result

+ 

+ 

+ class Subject:

+     """

+     Decision subject.

+ 

+     Main properties are type and item/identifier.

+ 

+     Item or identifier should uniquely identify the artefact (test subject).

+     """

+ 

+     def __init__(self, type_, item):

+         self.item = item

+         self._type = type_

+ 

+     @property

+     def type(self):

+         """Subject type string."""

+         return self._type.id

+ 

+     @property

+     def identifier(self):

+         """Alias for item property."""

+         return self.item

+ 

+     @property

+     def package_name(self):

+         """Package name of the subject or None."""

+         if self._type.is_nvr:

+             return self.item.rsplit("-", 2)[0]

+ 

+         return None

+ 

+     @property

+     def short_product_version(self):

+         """Get short product version of the subject (guess from identifier) or None."""

+         if self._type.is_nvr:

+             try:

+                 _, _, release = self.identifier.rsplit("-", 2)

+                 _, short_prod_version = release.rsplit(".", 1)

+                 return short_prod_version

+             except (KeyError, ValueError):

+                 pass

+ 

+         return None

+ 

+     @property

+     def product_version(self):

+         return self._type.product_version

+ 

+     @property

+     def is_koji_build(self):

+         return self._type.is_koji_build

+ 

+     @property

+     def supports_remote_rule(self):

+         return self._type.supports_remote_rule

+ 

+     @property

+     def ignore_missing_policy(self):

+         return self._type.ignore_missing_policy

+ 

+     def to_dict(self):

+         if self._type.item_dict:

+             return _to_dict(self._type.item_dict, self.type, self.item)

+ 

+         return {"type": self.type, "item": self.item}

+ 

+     def result_queries(self):

+         """

+         Yields parameters for RestulsDB queries.

+ 

+         For example, one set of parameters could have "type=koji_build" for one

+         query and "type=brew-build" for another.

+         """

+         if self._type.result_queries:

+             for query_dict in self._type.result_queries:

+                 yield _to_dict(query_dict, self.type, self.item)

+         else:

+             yield self.to_dict()

+ 

+     def get_latest_results(self, results):

+         """

+         Filters out results to get only the latest relevant ones.

+ 

+         The input results are sorted from latest to oldest.

+         """

+         if self._type.latest_result_unique_keys:

+             return list(_get_latest_results(results, self._type.latest_result_unique_keys))

+         return results

+ 

+     def __str__(self):

+         return "subject_type {!r}, subject_identifier {!r}".format(

+             self.type_, self.item

+         )

@@ -0,0 +1,100 @@ 

+ # SPDX-License-Identifier: GPL-2.0+

+ """

+ Subject type helper functions.

+ """

+ 

+ import glob

+ import os

+ 

+ from greenwave.safe_yaml import (

+     SafeYAMLBool,

+     SafeYAMLDict,

+     SafeYAMLList,

+     SafeYAMLObject,

+     SafeYAMLString,

+ )

+ 

+ 

+ class GenericSubjectType:

+     def __init__(self, id_):

+         self.id = id_

+         self.aliases = []

+         self.is_koji_build = True

+         self.is_nvr = False

+         self.supports_remote_rule = False

+         self.product_version = None

+         self.item_dict = {}

+         self.result_queries = []

+         self.latest_result_unique_keys = []

+ 

+ 

+ class SubjectType(SafeYAMLObject):

+     root_yaml_tag = '!SubjectType'

+ 

+     safe_yaml_attributes = {

+         'id': SafeYAMLString(),

+ 

+         'aliases': SafeYAMLList(item_type=str, optional=True),

+ 

+         # Key name to load from decision 'subject' list or ResultsDB result

+         # data ("item" will be used if the key is empty or not found).

+         'item_key': SafeYAMLString(optional=True),

+ 

+         # A build for subject identifier can be found on Koji/Brew.

+         'is_koji_build': SafeYAMLBool(optional=True, default=False),

+ 

+         # Is identifier in NVR format? If true, package name and short product

+         # version can be parsed for identifier.

+         'is_nvr': SafeYAMLBool(optional=True),

+ 

+         # Subject type can be used with RemoteRule only if value is True.

+         'supports_remote_rule': SafeYAMLBool(optional=True, default=False),

+ 

+         # Omit responding with HTTP 404 if there is no applicable policy.

+         'ignore_missing_policy': SafeYAMLBool(optional=True, default=False),

+ 

+         'product_version': SafeYAMLString(optional=True),

+ 

+         # Serialization dict for decision.

+         'item_dict': SafeYAMLDict(optional=True),

+ 

+         # List of serialization dicts for ResultsDB requests.

+         # If not defined, defaults to single request with item_dict value.

+         'result_queries': SafeYAMLList(item_type=dict, optional=True),

+ 

+         # List of keys in ResultsDB data for checking obsolete items.

+         # E.g. [a, b] means that only the first item with unique values of keys

+         # 'a', 'b' in 'data' of each result is valid.

+         'latest_result_unique_keys': SafeYAMLList(item_type=str, optional=True),

+     }

+ 

+     def matches(self, id_):

+         return id_ == self.id or id_ in self.aliases

+ 

+     @property

+     def safe_yaml_label(self):

+         return 'SubjectType {!r}'.format(self.id)

+ 

+ 

+ def load_subject_types(subject_types_dir):

+     """

+     Load Greenwave subject types from the given directory.

+ 

+     :param str subject_types_dir: A path points to the policies directory.

+     :return: A list of subject_types.

+     """

+     paths = glob.glob(os.path.join(subject_types_dir, '*.yaml'))

+     subject_types = []

+     for path in paths:

+         with open(path, 'r') as f:

+             subject_types.extend(SubjectType.safe_load_all(f))

+ 

+     return subject_types

+ 

+ 

+ def create_subject_type(id_, subject_types):

+     for type_ in subject_types:

+         if type_.matches(id_):

+             return type_

+ 

+     return GenericSubjectType(id_)

@@ -0,0 +1,15 @@ 

+ import pytest

+ 

+ from greenwave.app_factory import create_app

+ 

+ 

+ @pytest.fixture

+ def app():

+     app = create_app(config_obj='greenwave.config.TestingConfig')

+     with app.app_context():

+         yield app

+ 

+ 

+ @pytest.fixture

+ def client(app):

+     yield app.test_client()

@@ -163,12 +163,24 @@ 

              mock_waivers.assert_called_once()

  

  

- def test_life_decision():

-     app = create_app('greenwave.config.TestingConfig')

-     client = app.test_client()

+ def test_life_decision(client):

      data = {

          'question': 'Where am I going to be in 5 years?'

      }

      response = client.get('/api/v1.0/life-decision', json=data)

      assert response.status_code == 200

      assert type(response.data.decode("utf-8")) == str

+ 

+ 

+ def test_subject_types(client):

+     response = client.get('/api/v1.0/subject_types')

+     assert response.status_code == 200

+     data = response.json

+     assert len(data['subject_types'])

+     assert [x['id'] for x in data['subject_types']] == [

+         'bodhi_update',

+         'compose',

+         'koji_build',

+         'redhat-container-image',

+         'redhat-module',

+     ]

@@ -22,33 +22,37 @@ 

  )

  from greenwave.resources import ResultsRetriever

  from greenwave.safe_yaml import SafeYAMLError

+ from greenwave.subjects.factory import create_subject

  from greenwave.waivers import waive_answers

  

  

+ def create_test_subject(type_id, item):

+     app = create_app('greenwave.config.TestingConfig')

+     with app.app_context():

+         return create_subject(type_id, item)

+ 

+ 

  class DummyResultsRetriever(ResultsRetriever):

-     def __init__(

-             self, subject_identifier=None, testcase=None, outcome='PASSED',

-             subject_type='koji_build'):

+     def __init__(self, subject=None, testcase=None, outcome='PASSED'):

          super(DummyResultsRetriever, self).__init__(

              ignore_ids=[],

              when='',

              timeout=0,

              verify=False,

              url='')

-         self.subject_identifier = subject_identifier

-         self.subject_type = subject_type

+         self.subject = subject

          self.testcase = testcase

          self.outcome = outcome

  

      def _retrieve_data(self, params):

-         if (params.get('item') == self.subject_identifier and

-                 ('type' not in params or self.subject_type in params['type'].split(',')) and

+         if (self.subject and params.get('item') == self.subject.identifier and

+                 ('type' not in params or self.subject.type in params['type'].split(',')) and

                  params.get('testcases') == self.testcase):

              return [{

                  'id': 123,

                  'data': {

-                     'item': [self.subject_identifier],

-                     'type': [self.subject_type],

+                     'item': [self.subject.identifier],

+                     'type': [self.subject.type],

                  },

                  'testcase': {'name': self.testcase},

                  'outcome': self.outcome,
@@ -57,21 +61,22 @@ 

  

  

  def test_summarize_answers():

+     subject = create_test_subject('koji_build', 'nvr')

      assert summarize_answers([RuleSatisfied()]) == \

          'All required tests passed'

-     assert summarize_answers([TestResultFailed('koji_build', 'nvr', 'test', None, 'id'),

+     assert summarize_answers([TestResultFailed(subject, 'test', None, 'id'),

                                RuleSatisfied()]) == \

          '1 of 2 required tests failed'

-     assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'test', None)]) == \

+     assert summarize_answers([TestResultMissing(subject, 'test', None)]) == \

          '1 of 1 required test results missing'

-     assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'test', None),

-                               TestResultFailed('koji_build', 'nvr', 'test', None, 'id')]) == \

+     assert summarize_answers([TestResultMissing(subject, 'test', None),

+                               TestResultFailed(subject, 'test', None, 'id')]) == \

          '1 of 2 required tests failed, 1 result missing'

-     assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'testa', None),

-                               TestResultMissing('koji_build', 'nvr', 'testb', None),

-                               TestResultFailed('koji_build', 'nvr', 'test', None, 'id')]) == \

+     assert summarize_answers([TestResultMissing(subject, 'testa', None),

+                               TestResultMissing(subject, 'testb', None),

+                               TestResultFailed(subject, 'test', None, 'id')]) == \

          '1 of 3 required tests failed, 2 results missing'

-     assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'test', None),

+     assert summarize_answers([TestResultMissing(subject, 'test', None),

                               RuleSatisfied()]) == \

          '1 of 2 required test results missing'

  
@@ -92,10 +97,10 @@ 

      policy = policies[0]

  

      results = DummyResultsRetriever()

-     subject_identifier = 'some_nevr'

+     subject = create_test_subject('koji_build', 'some_nevr')

  

      # Ensure that absence of a result is failure.

-     decision = policy.check('fedora-rawhide', subject_identifier, results)

+     decision = policy.check('fedora-rawhide', subject, results)

      assert len(decision) == 1

      assert isinstance(decision[0], TestResultMissing)

  
@@ -122,9 +127,10 @@ 

      policy = policies[0]

  

      item = 'some_nevr'

-     results = DummyResultsRetriever(item, 'sometest', 'FAILED', 'brew-build')

+     subject = create_test_subject('koji_build', item)

+     results = DummyResultsRetriever(subject, 'sometest', 'FAILED')

  

-     decision = policy.check('fedora-rawhide', item, results)

+     decision = policy.check('fedora-rawhide', subject, results)

      assert len(decision) == 1

      assert isinstance(decision[0], TestResultFailed)

  
@@ -136,7 +142,7 @@ 

          'product_version': 'fedora-rawhide',

          'waived': True,

      }]

-     decision = policy.check('fedora-rawhide', item, results)

+     decision = policy.check('fedora-rawhide', subject, results)

      decision = waive_answers(decision, waivers)

      assert len(decision) == 1

      assert isinstance(decision[0], RuleSatisfied)
@@ -164,9 +170,10 @@ 

      policy = policies[0]

  

      item = 'some_bodhi_update'

-     results = DummyResultsRetriever(item, 'sometest', 'FAILED', 'bodhi_update')

+     subject = create_test_subject('bodhi_update', item)

+     results = DummyResultsRetriever(subject, 'sometest', 'FAILED')

  

-     decision = policy.check('fedora-rawhide', item, results)

+     decision = policy.check('fedora-rawhide', subject, results)

      assert len(decision) == 1

      assert isinstance(decision[0], TestResultFailed)

  
@@ -178,7 +185,7 @@ 

          'product_version': 'fedora-rawhide',

          'waived': True,

      }]

-     decision = policy.check('fedora-rawhide', item, results)

+     decision = policy.check('fedora-rawhide', subject, results)

      decision = waive_answers(decision, waivers)

      assert len(decision) == 1

      assert isinstance(decision[0], RuleSatisfied)
@@ -280,7 +287,7 @@ 

      """ Testing the RemoteRule with the koji interaction.

      In this case we are just mocking koji """

  

-     nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', 'nethack-1.2.3-1.el9000')

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -315,20 +322,20 @@ 

                  policy = policies[0]

  

                  # Ensure that presence of a result is success.

-                 results = DummyResultsRetriever(nvr, 'dist.upgradepath')

-                 decision = policy.check('fedora-26', nvr, results)

+                 results = DummyResultsRetriever(subject, 'dist.upgradepath')

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], RuleSatisfied)

  

                  # Ensure that absence of a result is failure.

                  results = DummyResultsRetriever()

-                 decision = policy.check('fedora-26', nvr, results)

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultMissing)

  

                  # And that a result with a failure, is a failure.

-                 results = DummyResultsRetriever(nvr, 'dist.upgradepath', 'FAILED')

-                 decision = policy.check('fedora-26', nvr, results)

+                 results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED')

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultFailed)

  
@@ -339,6 +346,7 @@ 

      In this case we are just mocking koji """

  

      nvr = '389-ds-1.4-820181127205924.9edba152'

+     subject = create_test_subject('redhat-module', nvr)

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -374,22 +382,21 @@ 

                  policy = policies[0]

  

                  # Ensure that presence of a result is success.

-                 results = DummyResultsRetriever(nvr, 'baseos-ci.redhat-module.tier0.functional',

-                                                 subject_type='redhat-module')

-                 decision = policy.check('rhel-8', nvr, results)

+                 results = DummyResultsRetriever(subject, 'baseos-ci.redhat-module.tier0.functional')

+                 decision = policy.check('rhel-8', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], RuleSatisfied)

  

                  # Ensure that absence of a result is failure.

-                 results = DummyResultsRetriever(subject_type='redhat-module')

-                 decision = policy.check('rhel-8', nvr, results)

+                 results = DummyResultsRetriever(subject)

+                 decision = policy.check('rhel-8', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultMissing)

  

                  # And that a result with a failure, is a failure.

-                 results = DummyResultsRetriever(nvr, 'baseos-ci.redhat-module.tier0.functional',

-                                                 'FAILED', subject_type='redhat-module')

-                 decision = policy.check('rhel-8', nvr, results)

+                 results = DummyResultsRetriever(

+                     subject, 'baseos-ci.redhat-module.tier0.functional', 'FAILED')

+                 decision = policy.check('rhel-8', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultFailed)

  
@@ -399,6 +406,7 @@ 

      In this case we are just mocking koji """

  

      nvr = '389-ds-1.4-820181127205924.9edba152'

+     subject = create_test_subject('redhat-container-image', nvr)

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -435,29 +443,27 @@ 

  

                  # Ensure that presence of a result is success.

                  results = DummyResultsRetriever(

-                     nvr, 'baseos-ci.redhat-container-image.tier0.functional',

-                     subject_type='redhat-container-image')

-                 decision = policy.check('rhel-8', nvr, results)

+                     subject, 'baseos-ci.redhat-container-image.tier0.functional')

+                 decision = policy.check('rhel-8', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], RuleSatisfied)

  

                  # Ensure that absence of a result is failure.

-                 results = DummyResultsRetriever(subject_type='redhat-container-image')

-                 decision = policy.check('rhel-8', nvr, results)

+                 results = DummyResultsRetriever(subject)

+                 decision = policy.check('rhel-8', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultMissing)

  

                  # And that a result with a failure, is a failure.

                  results = DummyResultsRetriever(

-                     nvr, 'baseos-ci.redhat-container-image.tier0.functional', 'FAILED',

-                     subject_type='redhat-container-image')

-                 decision = policy.check('rhel-8', nvr, results)

+                     subject, 'baseos-ci.redhat-container-image.tier0.functional', 'FAILED')

+                 decision = policy.check('rhel-8', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultFailed)

  

  

  def test_remote_rule_policy_optional_id(tmpdir):

-     nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', 'nethack-1.2.3-1.el9000')

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -490,7 +496,7 @@ 

  

                  results = DummyResultsRetriever()

                  expected_details = "Policy 'untitled': Attribute 'product_versions' is required"

-                 decision = policy.check('fedora-26', nvr, results)

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], InvalidGatingYaml)

                  assert decision[0].is_satisfied is False
@@ -500,7 +506,7 @@ 

  def test_remote_rule_malformed_yaml(tmpdir):

      """ Testing the RemoteRule with a malformed gating.yaml file """

  

-     nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', 'nethack-1.2.3-1.el9000')

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -545,7 +551,7 @@ 

                      policy = policies[0]

  

                      results = DummyResultsRetriever()

-                     decision = policy.check('fedora-26', nvr, results)

+                     decision = policy.check('fedora-26', subject, results)

                      assert len(decision) == 1

                      assert isinstance(decision[0], InvalidGatingYaml)

                      assert decision[0].is_satisfied is False
@@ -555,7 +561,7 @@ 

      """ Testing the RemoteRule with a malformed gating.yaml file

      But this time waiving the error """

  

-     nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', 'nethack-1.2.3-1.el9000')

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -610,14 +616,14 @@ 

                          'comment': 'Waiving the invalid gating.yaml file',

                          'waived': True,

                      }]

-                     decision = policy.check('fedora-26', nvr, results)

+                     decision = policy.check('fedora-26', subject, results)

                      decision = waive_answers(decision, waivers)

                      assert len(decision) == 0

  

  

  def test_remote_rule_required():

      """ Testing the RemoteRule with required flag set """

-     nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', 'nethack-1.2.3-1.el9000')

      app = create_app('greenwave.config.TestingConfig')

      with app.app_context():

          with mock.patch('greenwave.resources.retrieve_scm_from_koji') as scm:
@@ -635,11 +641,11 @@ 

                  """))

                  policy = policies[0]

                  results = DummyResultsRetriever()

-                 decision = policy.check('fedora-rawhide', nvr, results)

+                 decision = policy.check('fedora-rawhide', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], MissingGatingYaml)

                  assert not decision[0].is_satisfied

-                 assert decision[0].subject_identifier == nvr

+                 assert decision[0].subject.identifier == subject.identifier

  

  

  def test_parse_policies_missing_tag():
@@ -715,8 +721,9 @@ 

      policies = load_policies(tmpdir.strpath)

      policy = policies[0]

  

-     results = DummyResultsRetriever('nethack-1.2.3-1.el9000', 'sometest', 'PASSED', 'kind-of-magic')

-     decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results)

+     subject = create_test_subject('kind-of-magic', 'nethack-1.2.3-1.el9000')

+     results = DummyResultsRetriever(subject, 'sometest', 'PASSED')

+     decision = policy.check('rhel-9000', subject, results)

      assert len(decision) == 1

      assert isinstance(decision[0], TestResultPassed)

  
@@ -743,8 +750,9 @@ 

      policies = load_policies(tmpdir.strpath)

      policy = policies[0]

  

-     results = DummyResultsRetriever('nethack-1.2.3-1.el9000', 'sometest', 'PASSED', 'koji_build')

-     decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results)

+     subject = create_test_subject('koji_build', 'nethack-1.2.3-1.el9000')

+     results = DummyResultsRetriever(subject, 'sometest', 'PASSED')

+     decision = policy.check('rhel-9000', subject, results)

      assert len(decision) == num_decisions

      if num_decisions:

          assert isinstance(decision[0], TestResultPassed)
@@ -923,6 +931,7 @@ 

  

  def test_policy_with_subject_type_component_version(tmpdir):

      nv = '389-ds-base-1.4.0.10'

+     subject = create_test_subject('component-version', nv)

      p = tmpdir.join('fedora.yaml')

      p.write(dedent("""

          --- !Policy
@@ -937,9 +946,8 @@ 

          """))

      policies = load_policies(tmpdir.strpath)

      policy = policies[0]

-     results = DummyResultsRetriever(nv, 'test_for_new_type', 'PASSED',

-                                     'component-version')

-     decision = policy.check('fedora-29', nv, results)

+     results = DummyResultsRetriever(subject, 'test_for_new_type', 'PASSED')

+     decision = policy.check('fedora-29', subject, results)

      assert len(decision) == 1

      assert isinstance(decision[0], RuleSatisfied)

  
@@ -947,6 +955,7 @@ 

  @pytest.mark.parametrize('subject_type', ["redhat-module", "redhat-container-image"])

  def test_policy_with_subject_type_redhat_module(tmpdir, subject_type):

      nsvc = 'httpd:2.4:20181018085700:9edba152'

+     subject = create_test_subject(subject_type, nsvc)

      p = tmpdir.join('fedora.yaml')

      p.write(dedent("""

          --- !Policy
@@ -961,9 +970,8 @@ 

          """ % subject_type))

      policies = load_policies(tmpdir.strpath)

      policy = policies[0]

-     results = DummyResultsRetriever(nsvc, 'test_for_redhat_module_type', 'PASSED',

-                                     subject_type)

-     decision = policy.check('fedora-29', nsvc, results)

+     results = DummyResultsRetriever(subject, 'test_for_redhat_module_type', 'PASSED')

+     decision = policy.check('fedora-29', subject, results)

      assert len(decision) == 1

      assert isinstance(decision[0], RuleSatisfied)

  
@@ -974,6 +982,7 @@ 

      In this case we are just mocking koji """

  

      nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', nvr)

  

      serverside_json = {

          'product_version': 'fedora-26',
@@ -1006,30 +1015,30 @@ 

                  policy = OnDemandPolicy.create_from_json(serverside_json)

  

                  # Ensure that presence of a result is success.

-                 results = DummyResultsRetriever(nvr, 'dist.upgradepath')

-                 decision = policy.check('fedora-26', nvr, results)

+                 results = DummyResultsRetriever(subject, 'dist.upgradepath')

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], RuleSatisfied)

  

                  # Ensure that absence of a result is failure.

                  results = DummyResultsRetriever()

-                 decision = policy.check('fedora-26', nvr, results)

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultMissing)

  

                  # And that a result with a failure, is a failure.

-                 results = DummyResultsRetriever(nvr, 'dist.upgradepath', 'FAILED')

-                 decision = policy.check('fedora-26', nvr, results)

+                 results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED')

+                 decision = policy.check('fedora-26', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultFailed)

  

  

  @pytest.mark.parametrize('two_rules', (True, False))

  def test_on_demand_policy_match(two_rules):

-     """ Testing the RemoteRule with the koji interaction when on_demand policy is given.

-     In this case we are just mocking koji """

+     """ Proceed other rules when there's no source URL in Koji build """

  

      nvr = 'httpd-2.4.el9000'

+     subject = create_test_subject('koji_build', nvr)

  

      serverside_json = {

          'product_version': 'fedora-30',
@@ -1057,15 +1066,15 @@ 

              koji_server.return_value = koji_server_instance

              policy = OnDemandPolicy.create_from_json(serverside_json)

  

-             rv = policy.matches(subject_identifier=nvr)

+             policy_matches = policy.matches(subject=subject)

  

              koji_server_instance.getBuild.assert_called_once()

-             assert rv is two_rules

+             assert policy_matches

  

              results = DummyResultsRetriever(

-                 nvr, 'fake.testcase.tier0.validation', 'PASSED', 'koji_build'

+                 subject, 'fake.testcase.tier0.validation', 'PASSED'

              )

-             decision = policy.check('fedora-30', nvr, results)

+             decision = policy.check('fedora-30', subject, results)

              if two_rules:

                  assert len(decision) == 1

                  assert isinstance(decision[0], RuleSatisfied)
@@ -1073,6 +1082,7 @@ 

  

  def test_two_rules_no_duplicate(tmpdir):

      nvr = 'nethack-1.2.3-1.el9000'

+     subject = create_test_subject('koji_build', nvr)

  

      serverside_fragment = dedent("""

          --- !Policy
@@ -1108,19 +1118,19 @@ 

                  policy = policies[0]

  

                  # Ensure that presence of a result is success.

-                 results = DummyResultsRetriever(nvr, 'dist.upgradepath')

-                 decision = policy.check('fedora-31', nvr, results)

+                 results = DummyResultsRetriever(subject, 'dist.upgradepath')

+                 decision = policy.check('fedora-31', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], RuleSatisfied)

  

                  # Ensure that absence of a result is failure.

                  results = DummyResultsRetriever()

-                 decision = policy.check('fedora-31', nvr, results)

+                 decision = policy.check('fedora-31', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultMissing)

  

                  # And that a result with a failure, is a failure.

-                 results = DummyResultsRetriever(nvr, 'dist.upgradepath', 'FAILED')

-                 decision = policy.check('fedora-31', nvr, results)

+                 results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED')

+                 decision = policy.check('fedora-31', subject, results)

                  assert len(decision) == 1

                  assert isinstance(decision[0], TestResultFailed)

@@ -5,20 +5,28 @@ 

  

  from textwrap import dedent

  

- import greenwave.app_factory

  import greenwave.consumers.resultsdb

+ from greenwave.app_factory import create_app

  from greenwave.product_versions import subject_product_version

  from greenwave.policies import Policy

+ from greenwave.subjects.factory import create_subject

  

  

- def test_announcement_keys_decode_with_list():

+ def announcement_subject(message):

      cls = greenwave.consumers.resultsdb.ResultsDBHandler

+ 

+     app = create_app()

+     with app.app_context():

+         subject = cls.announcement_subject(message)

+         if subject:

+             return subject.to_dict()

+ 

+ 

+ def test_announcement_keys_decode_with_list():

      message = {'msg': {'data': {

          'original_spec_nvr': ['glibc-1.0-1.fc27'],

      }}}

-     subjects = list(cls.announcement_subjects(message))

- 

-     assert subjects == [('koji_build', 'glibc-1.0-1.fc27')]

+     assert announcement_subject(message) == {'type': 'koji_build', 'item': 'glibc-1.0-1.fc27'}

  

  

  def test_no_announcement_subjects_for_empty_nvr():
@@ -28,26 +36,22 @@ 

      empty string. To avoid unpredictable consequences, we should not

      return any announcement subjects for such a message.

      """

-     cls = greenwave.consumers.resultsdb.ResultsDBHandler

      message = {'msg': {'data': {

          'original_spec_nvr': [""],

      }}}

-     subjects = list(cls.announcement_subjects(message))

  

-     assert subjects == []

+     assert announcement_subject(message) is None

  

  

  def test_announcement_subjects_for_brew_build():

      # The 'brew-build' type appears internally within Red Hat. We treat it as an

      # alias of 'koji_build'.

-     cls = greenwave.consumers.resultsdb.ResultsDBHandler

      message = {'msg': {'data': {

          'type': 'brew-build',

          'item': ['glibc-1.0-3.fc27'],

      }}}

-     subjects = list(cls.announcement_subjects(message))

  

-     assert subjects == [('koji_build', 'glibc-1.0-3.fc27')]

+     assert announcement_subject(message) == {'type': 'koji_build', 'item': 'glibc-1.0-3.fc27'}

  

  

  def test_announcement_subjects_for_new_compose_message():
@@ -57,7 +61,6 @@ 

      productmd.compose.id with value of the compose ID. This is only

      possible with new-style 'resultsdb' fedmsgs, like this one.

      """

-     cls = greenwave.consumers.resultsdb.ResultsDBHandler

      message = {

          'msg': {

              'data': {
@@ -78,9 +81,9 @@ 

              }

          }

      }

-     subjects = list(cls.announcement_subjects(message))

  

-     assert subjects == [('compose', 'Fedora-Rawhide-20181205.n.0')]

+     assert announcement_subject(message) == \

+         {'productmd.compose.id': 'Fedora-Rawhide-20181205.n.0'}

  

  

  def test_no_announcement_subjects_for_old_compose_message():
@@ -89,7 +92,6 @@ 

      https://pagure.io/greenwave/issue/122 etc. So we should NOT

      produce any subjects for this kind of message.

      """

-     cls = greenwave.consumers.resultsdb.ResultsDBHandler

      message = {

          'msg': {

              'task': {
@@ -106,9 +108,8 @@ 

              }

          }

      }

-     subjects = list(cls.announcement_subjects(message))

  

-     assert subjects == []

+     assert announcement_subject(message) is None

  

  

  parameters = [
@@ -321,12 +322,12 @@ 

      }

      handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub)

      with handler.flask_app.app_context():

-         product_version = subject_product_version(

-             'release-e2e-test-1.0.1685-1.el5', 'koji_build')

+         subject = create_subject('koji_build', 'release-e2e-test-1.0.1685-1.el5')

+         product_version = subject_product_version(subject)

          assert product_version == 'rhel-5'

  

-         product_version = subject_product_version(

-             'rust-toolset-rhel8-20181010170614.b09eea91', 'redhat-module')

+         subject = create_subject('redhat-module', 'rust-toolset-rhel8-20181010170614.b09eea91')

+         product_version = subject_product_version(subject)

          assert product_version == 'rhel-8'

  

  
@@ -335,8 +336,10 @@ 

      koji_proxy.getBuild.return_value = {'task_id': 666}

      koji_proxy.getTaskRequest.return_value = ['git://example.com/project', 'rawhide', {}]

  

-     product_version = subject_product_version(

-         'fake_koji_build', 'container-build', koji_proxy)

+     app = create_app()

+     with app.app_context():

+         subject = create_subject('container-build', 'fake_koji_build')

+     product_version = subject_product_version(subject, koji_proxy)

      koji_proxy.getBuild.assert_called_once_with('fake_koji_build')

      koji_proxy.getTaskRequest.assert_called_once_with(666)

      assert product_version == 'fedora-rawhide'
@@ -346,8 +349,10 @@ 

      koji_proxy = mock.MagicMock()

      koji_proxy.getBuild.return_value = {'task_id': None}

  

-     product_version = subject_product_version(

-         'fake_koji_build', 'container-build', koji_proxy)

+     app = create_app()

+     with app.app_context():

+         subject = create_subject('container-build', 'fake_koji_build')

+     product_version = subject_product_version(subject, koji_proxy)

  

      koji_proxy.getBuild.assert_called_once_with('fake_koji_build')

      koji_proxy.getTaskRequest.assert_not_called()
@@ -361,7 +366,10 @@ 

      'badnvr-1.2.f30',

  ))

  def test_guess_product_version_failure(nvr):

-     product_version = subject_product_version(nvr, 'koji_build')

+     app = create_app()

+     with app.app_context():

+         subject = create_subject('koji_build', nvr)

+     product_version = subject_product_version(subject)

      assert product_version is None

  

  

@@ -6,6 +6,7 @@ 

  from greenwave.app_factory import create_app

  from greenwave.policies import Policy, RemoteRule

  from greenwave.safe_yaml import SafeYAMLError

+ from greenwave.subjects.factory import create_subject

  

  

  def test_match_passing_test_case_rule():
@@ -54,6 +55,7 @@ 

  

      app = create_app('greenwave.config.TestingConfig')

      with app.app_context():

+         subject = create_subject('koji_build', nvr)

          policies = Policy.safe_load_all(policy_yaml)

          assert len(policies) == 1

  
@@ -62,9 +64,9 @@ 

  

          rule = policy.rules[0]

          assert rule.matches(policy)

-         assert rule.matches(policy, subject_identifier=nvr)

-         assert rule.matches(policy, subject_identifier=nvr, testcase='some_test_case')

-         assert not rule.matches(policy, subject_identifier=nvr, testcase='other_test_case')

+         assert rule.matches(policy, subject=subject)

+         assert rule.matches(policy, subject=subject, testcase='some_test_case')

+         assert not rule.matches(policy, subject=subject, testcase='other_test_case')

  

          # Simulate invalid gating.yaml file.

          def raiseYamlError(*args):
@@ -73,9 +75,9 @@ 

          mock_retrieve_yaml_remote_rule.side_effect = raiseYamlError

  

          assert rule.matches(policy)

-         assert not rule.matches(policy, subject_identifier=nvr)

-         assert not rule.matches(policy, subject_identifier=nvr, testcase='some_test_case')

-         assert not rule.matches(policy, subject_identifier=nvr, testcase='other_test_case')

+         assert not rule.matches(policy, subject=subject)

+         assert not rule.matches(policy, subject=subject, testcase='some_test_case')

+         assert not rule.matches(policy, subject=subject, testcase='other_test_case')

  

  

  @pytest.mark.parametrize(('required_flag', 'required_value'), (

@@ -0,0 +1,92 @@ 

+ import pytest

+ 

+ from greenwave.subjects.factory import (

+     create_subject,

+     create_subject_from_data,

+     UnknownSubjectDataError,

+ )

+ 

+ 

+ def test_subject_create_from_data(app):

+     data = {

+         'item': 'some_item',

+         'type': 'some_type',

+     }

+     subject = create_subject_from_data(data)

+ 

+     assert subject.item == 'some_item'

+     assert subject.type == 'some_type'

+     assert subject.to_dict() == data

+     assert list(subject.result_queries()) == [data]

+ 

+ 

+ def test_subject_create_from_data_bad(app):

+     with pytest.raises(UnknownSubjectDataError):

+         create_subject_from_data({})

+ 

+ 

+ def test_subject_create_generic(app):

+     subject = create_subject('some_type', 'some_item')

+     assert subject.item == 'some_item'

+     assert subject.type == 'some_type'

+     assert subject.identifier == subject.item

+     assert subject.package_name is None

+     assert subject.short_product_version is None

+     assert subject.product_version is None

+     assert subject.is_koji_build

+     assert not subject.supports_remote_rule

+ 

+ 

+ def test_subject_koji_build_result_queries(app):

+     subject = create_subject('koji_build', 'some_nvr')

+     assert list(subject.result_queries()) == [

+         {'type': 'koji_build,brew-build', 'item': 'some_nvr'},

+         {'original_spec_nvr': 'some_nvr'},

+     ]

+ 

+ 

+ def test_subject_ignore_missing_policy(app):

+     subject = create_subject('bodhi_update', 'some_item')

+     assert subject.ignore_missing_policy

+ 

+ 

+ def test_subject_get_latest_results(app):

+     compose_id = 'some_compose'

+     variant_arch_outcome = (

+         ('BaseOS', 'ppc64', 'PASSED'),

+         ('BaseOS', 'ppc64', 'FAILED'),

+         ('BaseOS', 'x86_64', 'PASSED'),

+         ('BaseOS', 'ppc64', 'FAILED'),

+         ('BaseOS', 'x86_64', 'FAILED'),

+     )

+ 

+     results = [

+         {

+             'testcase': {'name': 'rtt.acceptance.validation'},

+             'outcome': outcome,

+             'data': {

+                 'productmd.compose.id': [compose_id],

+                 'system_variant': [variant],

+                 'system_architecture': [arch],

+             }

+         }

+         for variant, arch, outcome in variant_arch_outcome

+     ]

+ 

+     subject = create_subject('compose', compose_id)

+ 

+     assert len(subject.get_latest_results(results)) == 2

+ 

+     assert subject.get_latest_results(results)[0]['data'] == {

+         'productmd.compose.id': [compose_id],

+         'system_variant': ['BaseOS'],

+         'system_architecture': ['ppc64'],

+     }

+     assert subject.get_latest_results(results)[0]['outcome'] == 'PASSED'

+ 

+     assert subject.get_latest_results(results)[1]['data'] == {

+         'productmd.compose.id': [compose_id],

+         'system_variant': ['BaseOS'],

+         'system_architecture': ['x86_64'],

+     }

+     assert subject.get_latest_results(results)[1]['outcome'] == 'PASSED'

@@ -0,0 +1,39 @@ 

+ import pytest

+ 

+ from greenwave.config import TestingConfig

+ from greenwave.subjects.subject_type import (

+     create_subject_type,

+     load_subject_types,

+ )

+ 

+ 

+ @pytest.yield_fixture(scope='session')

+ def subject_types():

+     yield load_subject_types(TestingConfig.SUBJECT_TYPES_DIR)

+ 

+ 

+ def test_subject_type_create(subject_types):

+     subject_type = create_subject_type('koji_build', subject_types)

+     assert subject_type.id == 'koji_build'

+     assert subject_type.aliases == ['brew-build']

+     assert subject_type.is_koji_build

+     assert subject_type.is_nvr

+     assert subject_type.supports_remote_rule

+     assert subject_type.item_key == 'original_spec_nvr'

+ 

+ 

+ def test_subject_type_matches_type_id(subject_types):

+     subject_type = create_subject_type('koji_build', subject_types)

+     assert subject_type.id == 'koji_build'

+     assert subject_type.matches('koji_build')

+ 

+ 

+ def test_subject_type_matches_alias(subject_types):

+     subject_type = create_subject_type('koji_build', subject_types)

+     assert subject_type.aliases == ['brew-build']

+     assert subject_type.matches('brew-build')

+ 

+ 

+ def test_subject_type_safe_yaml_label(subject_types):

+     subject_type = create_subject_type('koji_build', subject_types)

+     assert subject_type.safe_yaml_label == "SubjectType 'koji_build'"

@@ -8,16 +8,20 @@ 

      InvalidGatingYaml,

  )

  

+ from greenwave.subjects.subject import Subject

+ from greenwave.subjects.subject_type import GenericSubjectType

  

+ 

+ testSubject = Subject(GenericSubjectType('koji_build'), 'nethack-1.2.3-1.el9000')

  testResultPassed = RuleSatisfied()

  testResultFailed = TestResultFailed(

-     'koji_build', 'nethack-1.2.3-1.el9000', 'test', None, 1)

+     testSubject, 'test', None, 1)

  testResultMissing = TestResultMissing(

-     'koji_build', 'nethack-1.2.3-1.el9000', 'test', None)

+     testSubject, 'test', None)

  testResultMissingWaived = TestResultMissingWaived(

-     'koji_build', 'nethack-1.2.3-1.el9000', 'test', None)

+     testSubject, 'test', None)

  testInvalidGatingYaml = InvalidGatingYaml(

-     'koji_build', 'nethack-1.2.3-1.el9000', 'test', 'Missing !Policy tag')

+     testSubject, 'test', 'Missing !Policy tag')

  

  

  def test_summary_passed():

file modified
+13 -10
@@ -5,14 +5,19 @@ 

      TestResultMissing,

      TestResultFailed,

  )

+ from greenwave.subjects.subject import Subject

+ from greenwave.subjects.subject_type import GenericSubjectType

  from greenwave.waivers import waive_answers

  

  

+ def test_subject():

+     return Subject(GenericSubjectType('koji_build'), 'nethack-1.2.3-1.rawhide')

+ 

+ 

  def test_waive_failed_result():

      answers = [

          TestResultFailed(

-             subject_type='koji-build',

-             subject_identifier='nethack-1.2.3-1.rawhide',

+             subject=test_subject(),

              test_case_name='test1',

              scenario='scenario1',

              result_id=99,
@@ -24,7 +29,7 @@ 

  

      waivers = [

          dict(

-             subject_type='koji-build',

+             subject_type='koji_build',

              subject_identifier='nethack-1.2.3-1.rawhide',

              product_version='rawhide',

              testcase='test1',
@@ -43,8 +48,7 @@ 

  def test_waive_missing_result():

      answers = [

          TestResultMissing(

-             subject_type='koji-build',

-             subject_identifier='nethack-1.2.3-1.rawhide',

+             subject=test_subject(),

              test_case_name='test1',

              scenario='scenario1',

          )
@@ -55,7 +59,7 @@ 

  

      waivers = [

          dict(

-             subject_type='koji-build',

+             subject_type='koji_build',

              subject_identifier='nethack-1.2.3-1.rawhide',

              product_version='rawhide',

              testcase='test1',
@@ -65,7 +69,7 @@ 

      expected_json = dict(

          type='test-result-missing-waived',

          testcase='test1',

-         subject_type='koji-build',

+         subject_type='koji_build',

          subject_identifier='nethack-1.2.3-1.rawhide',

          scenario='scenario1',

      )
@@ -76,8 +80,7 @@ 

  def test_waive_invalid_gatin_yaml():

      answers = [

          InvalidGatingYaml(

-             subject_type='koji-build',

-             subject_identifier='nethack-1.2.3-1.rawhide',

+             subject=test_subject(),

              test_case_name='invalid-gating-yaml',

              details='',

          )
@@ -88,7 +91,7 @@ 

  

      waivers = [

          dict(

-             subject_type='koji-build',

+             subject_type='koji_build',

              subject_identifier='nethack-1.2.3-1.rawhide',

              product_version='rawhide',

              testcase='invalid-gating-yaml',

@@ -21,7 +21,7 @@ 

      retriever.ignore_ids = [100]

      waiver = dict(

          id=99,

-         subject_type='koji-build',

+         subject_type='koji_build',

          subject_identifier='nethack-1.2.3-1.rawhide',

          product_version='rawhide',

          testcase='test1',
@@ -38,7 +38,7 @@ 

      retriever.ignore_ids = [99]

      waiver = dict(

          id=99,

-         subject_type='koji-build',

+         subject_type='koji_build',

          subject_identifier='nethack-1.2.3-1.rawhide',

          product_version='rawhide',

          testcase='test1',
@@ -54,7 +54,7 @@ 

      retriever = WaiversRetriever(**_DUMMY_RETRIEVER_ARGUMENTS)

      waiver = dict(

          id=99,

-         subject_type='koji-build',

+         subject_type='koji_build',

          subject_identifier='nethack-1.2.3-1.rawhide',

          product_version='rawhide',

          testcase='test1',

file modified
+2 -2
@@ -6,8 +6,8 @@ 

      Returns true only if there is a matching waiver for given answer.

      """

      return any(

-         waiver['subject_type'] == answer.subject_type and

-         waiver['subject_identifier'] == answer.subject_identifier and

+         waiver['subject_type'] == answer.subject.type and

+         waiver['subject_identifier'] == answer.subject.identifier and

          waiver['testcase'] == answer.test_case_name

          for waiver in waivers)

  

Adds classes for decision subject item and type.

This makes the code more modular and allows to easily enable or disable
subject type specializations. E.g. Fedora instances probably don't need
to include /etc/greenwave/subject_types/redhat-module.yaml. Also
brew-build can be removed as alias in koji_build.yaml.

Fixes "koji-build" in tests to "koji_build".

TODO:

  • Add documentation for the new configuration.

Signed-off-by: Lukas Holecek hluk@email.cz

rebased onto 01a4deb81f3910d90a51709044d31778e9da7a89

4 years ago

rebased onto 2d97989bc06f7f5a516892cd8c102365b485113f

4 years ago

rebased onto 36bb8caac01c1ff19c54cd9bd8053844fe47ea19

4 years ago

rebased onto 8389a944ba3a97213c3fb7e74c58ef58a4cb2884

4 years ago

Well of course the subject type koji_build is koji_build! :smirk:

Can we turn this into a meaningful property instead? For instance: has_guessable_product_version.

Can we just have create_subject_from_data raise the exception instead? Maybe we can have a custom exception, UnknownSubject which gets mapped to BadRequest automatically?

Maybe add some kind of is_deprecated field?

IMO has_guessable_product_version is less clear.

What I want to communicate with the attribute name is that whether it's possible to get the product version from Koji/Brew for the build ID (which is the subject item).

Why are we checking the secondary_item_key first? Its name implies that it should only be used if there's no match for the "primary".

IMO has_guessable_product_version is less clear.
What I want to communicate with the attribute name is that whether it's possible to get the product version from Koji/Brew for the build ID (which is the subject item).

Yeah... I don't have a better suggestion for it, but whenever we add a conditional in the code that checks the subject type, it's a warning sign of a gap in the abstraction layer.

Overall, I like where this is going. My main concern is being able to ensure that existing behavior is not broken.

rebased onto aee0038cb86b0fd25922f2cecc5c1360b1849ff2

4 years ago

Renamed secondary_item_key to item_key.

to_json ?

Renamed to_item_dict to to_dict (the value is not JSON yet).

Ah, is_deprecated is a good idea - I will add this.

rebased onto 322acd4fc7ea5011f0e8c7ec3cbf41a3a6ef1fe3

4 years ago

Updated.

BTW, I used ignore_missing_policy instead of is_deprecated in bodhi_update subject type.

@lucarval @gnaponie Can we merge this before I start adding a code for new subject types? This would help me a lot.

"Could not" or "Couldn't"

It is unclear to me: are we still allowing with this change arbitrary subject_type? Because we should.

I still think subject types shouldn't be fixed, but the user should be able to ask for whatever, without asking us to introduce it, putting overload in the process. We made an effort in the past to do that, and we are making a step back with this. see: FACTORY-3583
I think that if we want to revert this we should talk about it first.

It is unclear to me: are we still allowing with this change arbitrary subject_type? Because we should.

Yes, greenwave.subjects.subject_type.create_subject_type() creates GenericSubjectType object if no configuration matches.

rebased onto f0adc44132edae6139a4b011eafc2ca9bb3e7d9f

4 years ago

I still think subject types shouldn't be fixed, but the user should be able to ask for whatever, without asking us to introduce it, putting overload in the process. We made an effort in the past to do that, and we are making a step back with this. see: FACTORY-3583
I think that if we want to revert this we should talk about it first.

I'm still not sure what are your concerns.

This patch allows customizing behavior for any subject type using a yaml file. With this, Fedora and internal Greenwave instance can be customized separately, adding config files for other subject types or removing some of the default ones.

There are no fixed subject types.

Use can still ask for any subject type. If there is a yaml file for the subject type, it's behavior will depend on the configuration in the file.

Handling subjects/subject_types in the new submodules cleans up the code since there shouldn't be any checks for subject_type == 'koji_build' and similar.

This makes it easier and less error-prone to add special behavior for subject types.

Is there any downside to this? Is there other solution you have in mind?

I'm still not sure what are your concerns.
This patch allows customizing behavior for any subject type using a yaml file. With this, Fedora and internal Greenwave instance can be customized separately, adding config files for other subject types or removing some of the default ones.
There are no fixed subject types.
Use can still ask for any subject type. If there is a yaml file for the subject type, it's behavior will depend on the configuration in the file.
Handling subjects/subject_types in the new submodules cleans up the code since there shouldn't be any checks for subject_type == 'koji_build' and similar.
This makes it easier and less error-prone to add special behavior for subject types.
Is there any downside to this? Is there other solution you have in mind?

Alright then, I guess I misunderstood the change. It seems fine this approach.

This looks good... I guess. Please try to make more than one commit next time :)
it's really big and it's hard to review (to me).

+1

+1 (let's make sure we thoroughly test this in stage.)

rebased onto acf5d56

4 years ago

Pull-Request has been merged by lholecek

4 years ago
Metadata