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

interfaces: put base policy fragments inside each interface #3464

Merged
merged 92 commits into from Jun 29, 2017

Conversation

zyga
Copy link
Collaborator

@zyga zyga commented Jun 9, 2017

This change completes the work towards making interface modules self-contained. Now each interface
can define a fragment of the base policy that applies to its plugs and slots.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

@codecov-io
Copy link

codecov-io commented Jun 9, 2017

Codecov Report

Merging #3464 into master will decrease coverage by 0.09%.
The diff coverage is 32.04%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3464     +/-   ##
=========================================
- Coverage   76.93%   76.84%   -0.1%     
=========================================
  Files         378      378             
  Lines       26116    26217    +101     
=========================================
+ Hits        20093    20147     +54     
- Misses       4243     4290     +47     
  Partials     1780     1780
Impacted Files Coverage Δ
interfaces/policy/basedeclaration.go 40.74% <ø> (ø) ⬆️
interfaces/builtin/storage_framework_service.go 78.94% <0%> (-2.14%) ⬇️
interfaces/builtin/io_ports_control.go 77.77% <0%> (-2.23%) ⬇️
interfaces/builtin/modem_manager.go 53.48% <0%> (-1.28%) ⬇️
interfaces/builtin/ubuntu_download_manager.go 52.63% <0%> (-1.43%) ⬇️
interfaces/builtin/udisks2.go 80.48% <0%> (-2.02%) ⬇️
interfaces/builtin/time_control.go 75.75% <0%> (-2.37%) ⬇️
interfaces/builtin/ppp.go 42.85% <0%> (-2.15%) ⬇️
interfaces/builtin/pulseaudio.go 48.27% <0%> (-1.73%) ⬇️
interfaces/builtin/hardware_random_observe.go 74.19% <0%> (-2.48%) ⬇️
... and 85 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84227d0...d3f3c2c. Read the comment docs.

zyga added 29 commits June 20, 2017 10:51
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
…tself

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
…tself

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
…tself

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
zyga added 23 commits June 20, 2017 10:54
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
…self

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
… itself

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
…tself

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@zyga zyga force-pushed the feature/metadata-defines-base-policy branch from b5328c1 to 4e3a855 Compare June 20, 2017 08:55
Copy link

@jdstrand jdstrand left a comment

Choose a reason for hiding this comment

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

Looks good. In addition to the review I compared the output with 'snap debug get-base-declaration' and it all looks fine.

As an aside, I was surprised that I needed 'sudo' when using 'snap debug get-base-declaration'-- there is nothing privileged there, the info is static and public.

Copy link
Contributor

@stolowski stolowski left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for these changes! Just one question about a regex, see below.

- app
deny-connection:
slot-attributes:
name: .+
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this regex be more strict?

Choose a reason for hiding this comment

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

It could be, but all it is really saying is that 'name' must be present. The interface slot code verifies this for us.

x11:
allow-installation:
slot-snap-type:
- core
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't sure what is the real purpose of all these changes until I reached this... Very nice, it's great we get rid of this big blob :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:-)

@zyga
Copy link
Collaborator Author

zyga commented Jun 23, 2017

@jdstrand thank you for commenting, I think you are right but perhaps the nature of the debug endpoint is special enough to warrant this. All the debug interactions are done through one API endpoint. We could explore how to make specific actions require authentication but for now that is all-or-nothing for the debug endpoint.

@mvo5 mvo5 merged commit 2c7f904 into snapcore:master Jun 29, 2017
@zyga zyga deleted the feature/metadata-defines-base-policy branch June 29, 2017 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants