From f5222ac626cf2a7f6f7659ef8e7777afd8626527 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Sep 22 2020 10:27:36 +0000 Subject: block inherited extra tag info Fixes: https://pagure.io/koji/issue/2283 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 0ebd274..fca991b 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -4987,6 +4987,8 @@ def handle_edit_tag(goptions, session, args): help=_("Set tag extra option")) parser.add_option("-r", "--remove-extra", action="append", default=[], metavar="key", help=_("Remove tag extra option")) + parser.add_option("-b", "--block-extra", action="append", default=[], metavar="key", + help=_("Block inherited tag extra option")) (options, args) = parser.parse_args(args) if len(args) != 1: parser.error(_("Please specify a name for the tag")) @@ -5020,8 +5022,8 @@ def handle_edit_tag(goptions, session, args): value = arg_filter(value) extra[key] = value opts['extra'] = extra - if options.remove_extra: - opts['remove_extra'] = options.remove_extra + opts['remove_extra'] = options.remove_extra + opts['block_extra'] = options.block_extra # XXX change callname session.editTag2(tag, **opts) diff --git a/docs/schema-upgrade-1.23-1.24.sql b/docs/schema-upgrade-1.23-1.24.sql new file mode 100644 index 0000000..32fcdc0 --- /dev/null +++ b/docs/schema-upgrade-1.23-1.24.sql @@ -0,0 +1,9 @@ +-- upgrade script to migrate the Koji database schema +-- from version 1.23 to 1.24 + + +BEGIN; + +ALTER TABLE tag_extra ALTER COLUMN value DROP NOT NULL; + +COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index 74d38d9..ac3b374 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -394,7 +394,7 @@ CREATE TABLE tag_config ( CREATE TABLE tag_extra ( tag_id INTEGER NOT NULL REFERENCES tag(id), key TEXT NOT NULL, - value TEXT NOT NULL, -- TODO - move this to jsonb when we can + value TEXT, -- versioned - see desc above create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(), revoke_event INTEGER REFERENCES events(id), diff --git a/hub/kojihub.py b/hub/kojihub.py index 8537990..47934fb 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -3266,7 +3266,7 @@ def _create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_s return tag_id -def get_tag(tagInfo, strict=False, event=None): +def get_tag(tagInfo, strict=False, event=None, blocked=False): """Get tag information based on the tagInfo. tagInfo may be either a string (the tag name) or an int (the tag ID). Returns a map containing the following keys: @@ -3318,21 +3318,28 @@ def get_tag(tagInfo, strict=False, event=None): if strict: raise koji.GenericError("Invalid tagInfo: %r" % tagInfo) return None - result['extra'] = get_tag_extra(result, event) + result['extra'] = get_tag_extra(result, event, blocked=blocked) return result -def get_tag_extra(tagInfo, event=None): +def get_tag_extra(tagInfo, event=None, blocked=False): """ Get tag extra info (no inheritance) """ tables = ['tag_extra'] - fields = ['key', 'value'] + fields = ['key', 'value', 'CASE WHEN value IS NULL THEN TRUE ELSE FALSE END'] + aliases = ['key', 'value', 'blocked'] clauses = [eventCondition(event, table='tag_extra'), "tag_id = %(id)i"] + if not blocked: + clauses.append("value IS NOT NULL") query = QueryProcessor(columns=fields, tables=tables, clauses=clauses, values=tagInfo, - opts={'asList': True}) + aliases=aliases) result = {} - for key, value in query.execute(): - value = parse_json(value, errstr="Invalid tag extra data: %s" % key) - result[key] = value + for h in query.execute(): + if h['value'] is not None: + h['value'] = parse_json(h['value'], errstr="Invalid tag extra data: %s" % h['key']) + if blocked: + result[h['key']] = (h['blocked'], h['value']) + else: + result[h['key']] = h['value'] return result @@ -3357,6 +3364,7 @@ def edit_tag(tagInfo, **kwargs): the Maven repo. :param dict extra: add or update extra tag parameters. :param list remove_extra: remove extra tag parameters. + :param list block_extra: block inherited extra tag parameters. """ context.session.assertPerm('tag') @@ -3425,19 +3433,23 @@ WHERE id = %(tagID)i""" # handle extra data if 'extra' in kwargs: + removed = set(kwargs.get('block_extra', [])) | set(kwargs.get('remove_extra', [])) # check whether one key is both in extra and remove_extra - if 'remove_extra' in kwargs: - for removed in kwargs['remove_extra']: - if removed in kwargs['extra']: - raise koji.GenericError("Can not both add/update and remove tag-extra: '%s'" % - removed) + conflicts = removed.intersection(set(kwargs['extra'])) + if conflicts: + raise koji.GenericError("Can not both add/update and remove tag-extra: '%s'" % + conflicts.pop()) for key in kwargs['extra']: value = kwargs['extra'][key] if key not in tag['extra'] or tag['extra'][key] != value: + if value is None: + value = 'null' + else: + value = json.dumps(value) data = { 'tag_id': tag['id'], 'key': key, - 'value': json.dumps(kwargs['extra'][key]), + 'value': value, } # revoke old entry, if any update = UpdateProcessor('tag_extra', values=data, clauses=['tag_id = %(tag_id)i', @@ -3449,6 +3461,23 @@ WHERE id = %(tagID)i""" insert.make_create() insert.execute() + if 'block_extra' in kwargs: + for key in kwargs['block_extra']: + data = { + 'tag_id': tag['id'], + 'key': key, + 'value': None, + } + # revoke old entry, if any + update = UpdateProcessor('tag_extra', values=data, clauses=['tag_id = %(tag_id)i', + 'key=%(key)s']) + update.make_revoke() + update.execute() + # add new entry + insert = InsertProcessor('tag_extra', data=data) + insert.make_create() + insert.execute() + # handle remove_extra data if 'remove_extra' in kwargs: ne = [e for e in kwargs['remove_extra'] if e not in tag['extra']] @@ -11916,18 +11945,24 @@ class RootExports(object): def getBuildConfig(self, tag, event=None): """Return build configuration associated with a tag""" - taginfo = get_tag(tag, strict=True, event=event) + taginfo = get_tag(tag, strict=True, event=event, blocked=True) order = readFullInheritance(taginfo['id'], event=event) # follow inheritance for arches and extra for link in order: if link['noconfig']: continue - ancestor = get_tag(link['parent_id'], strict=True, event=event) + ancestor = get_tag(link['parent_id'], strict=True, event=event, blocked=True) if taginfo['arches'] is None and ancestor['arches'] is not None: taginfo['arches'] = ancestor['arches'] for key in ancestor['extra']: if key not in taginfo['extra']: taginfo['extra'][key] = ancestor['extra'][key] + # cleanup extras by blocked + for k, v in list(taginfo['extra'].items()): + if v[0]: + del taginfo['extra'][k] + else: + taginfo['extra'][k] = v[1] return taginfo def getRepo(self, tag, state=None, event=None, dist=False): diff --git a/tests/test_cli/test_edit_tag.py b/tests/test_cli/test_edit_tag.py index 128bbab..41144c6 100644 --- a/tests/test_cli/test_edit_tag.py +++ b/tests/test_cli/test_edit_tag.py @@ -46,6 +46,7 @@ class TestEditTag(utils.CliTestCase): 'maven_support': maven_support, 'maven_include_all': maven_include_all, 'extra': extra, + 'block_extra': [], 'remove_extra': remove_extra} options = mock.MagicMock() @@ -78,6 +79,8 @@ class TestEditTag(utils.CliTestCase): opts = {'perm': None, 'locked': not locked, 'maven_support': not maven_support, + 'block_extra': [], + 'remove_extra': [], 'maven_include_all': not maven_include_all} # Run it and check immediate output # args: tag --no-perm --unlock --no-maven-support --no-include-all @@ -130,6 +133,8 @@ Options: Set tag extra option -r key, --remove-extra=key Remove tag extra option + -b key, --block-extra=key + Block inherited tag extra option """ % progname expected_stderr = '' self.assertMultiLineEqual(actual_stdout, expected_stdout)