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

Update apparmor profiles #765

Merged
merged 1 commit into from
Sep 17, 2018
Merged

Conversation

skolekonov
Copy link
Contributor

@skolekonov skolekonov commented Sep 14, 2018

This change is Reviewable

@@ -29,9 +33,12 @@ profile virtlet flags=(attach_disconnected) {
/{usr/,}sbin/ebtables rix,
/{usr/,}sbin/brctl rix,
/opt/cni/bin/calico* rix,
/opt/cni/bin/genie rix,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about other plugins, like flannel?
Same with ipam plugins (host-local, dhcp).
Maybe it needs a note in readme how to enable/disable particular plugins?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For flannel it works, can you please provide a list of binaries specific to ipam plugins?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So /opt/cni/bin/flannel (and probably the same will be with /opt/cni/bin/bridge) does not require such entry with rix flag, right?
It will have read right to /run/flannel/subnet.env?

Mainly used ipam plugin will be probably /opt/cni/bin/host-local, which stores own data files in /var/lib/cni/networks/$NETWORK_NAME - with network name defined as in plugin config file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to provide access to the main binaries anyway, if there's a possibility that they will be executed. Will update rules

Copy link
Contributor

@jellonek jellonek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: but lets wait for the tests

Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: 0 of 2 approvals obtained

Copy link
Contributor

@ivan4th ivan4th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: 1 of 2 approvals obtained

@ivan4th ivan4th merged commit 6205387 into Mirantis:master Sep 17, 2018
@skolekonov skolekonov deleted the update-apparmor branch September 17, 2018 12:11
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 this pull request may close these issues.

3 participants