Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of check that check presence of sections #105

Closed
WellIDKRealy opened this issue Nov 5, 2021 · 2 comments
Closed

Lack of check that check presence of sections #105

WellIDKRealy opened this issue Nov 5, 2021 · 2 comments

Comments

@WellIDKRealy
Copy link

Describe the bug
If some sections are not present for example Mounts udcia will crash with Key error as cause etc

Even if i did something completely wrong it should be indicated Key Error error does not indicate it well IMO

To Reproduce
Make any container that does not have Mounts. NetworkSettings or Host config
run idcia on it
Example steps
podman pod create --name a
podman run -it --rm --pod a fedora /bin/bash
podman inspect k8s.gcr.io/pause:3.5

Expected behavior
Policy for mounts shoudl't be added if mounts are not present etc

Proposed Fix
https://github.com/WellIDKRealy/udica

i didn't test it extensively so i have no idea if i broke something

@vmojzis
Copy link
Collaborator

vmojzis commented Nov 9, 2021

Thanks for the suggestion. I'll definitely add some sort of checking.
Your approach is interesting and I actually learned a lot trying to understand it.

I'm just wondering if there was any reason to use decorator factory instead of a simple decorator that would catch KeyError. e.g.

def getter_decorator(function):
    def wrapper(self, data, *args):
        try:
            value = function(self, data, *args)
            return value if value else []
        except KeyError,TypeError:
            return []

    return wrapper

Also, returning [] instead of None saves us from editing more code (since all the values are later processed as iterables).

vmojzis added a commit to vmojzis/udica that referenced this issue Nov 9, 2021
Fixes: containers#105

Inspired by:
WellIDKRealy@0c56d98

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
vmojzis added a commit to vmojzis/udica that referenced this issue Nov 9, 2021
@WellIDKRealy
Copy link
Author

I used decorator factory because if there was any error unrelated to sections not existing and of the same type as error caused by nonexistence of them it still would be caught.

Anyways thanks for adding those checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants