freshmaker

#147 Two fixes to whitelist
Merged by jkaluza. Opened by cqi.
cqi/freshmaker fix-whitelist  into  master

Download 147.patch

In this PR, remaining word blacklist are removed from source code. allow_build is updated to not allow rebuild if rule names passed as parameter to allow_build and configured rule names do not match. For example, handler_build_whitelist is configured as below

HANDLER_BUILD_WHITELIST = {
    'some_handler': {
        'image': [{'name': 'RHSA-0001'}]
    }
}

and if calling if self.allow_build(ArtifactType.IMAGE, advisory_name='RHSA-0001'), False should be returned.

This PR should fix #107 in a way.

2 new commits added

  • Do not allow build if configured whitelist and passed rule name does not match
  • Clean word blacklist since blacklist has been removed

ping for review.

Maybe we can change the default value of in_whitelist to False, and then requires the artifact to be in whitelist explicitly?

Have no idea at this moment. Looks in_whitelist is used to hold the final return value. But, this is out of the scope of this PR. I think it could be fixed in another PR if it's worth to fix.

If this log.debug should stay hee, log also some description saying what those logged values mean.

This only checks that the keys provided to allow_build are set in the whitelist in configuration file and if not, it returns false, right? If so, could you add comment there, it could save some time when reading this code.

I think @qwan is right that if this should fix #107, we should probably remove in_whitelist and change this condition like this:

            if whitelist and any([match_rule(kwargs, rule) for rule in whitelist]):
                log.debug('%r, type=%r is whitelisted.',
                          kwargs, artifact_type.name.lower())
                return True

And also return False in the end of allow_build method instead of return in_whitelist.

rebased onto 5c0bd2afbbed662df6ca43b9fa02220618cb720f

Ah, this log should not present here. I added it for debug purpose. Let me remove it.

2 new commits added

  • Do not allow build if configured whitelist and passed rule name does not match
  • Clean word blacklist since blacklist has been removed

Fixed issues mentioned in comments. PTAL.

1 new commit added

  • Fix flake8 errors

+1, this looks good now :)

This one now conflicts with your previous PR I've just merged... Could you rebase?

rebased onto 7525299c07d5a78104dd47ff1fac255572b4eee7

Pull-Request has been merged by jkaluza

\ó/
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.