bsdci / libioc

A Python library to manage jails with ioc{age,ell}
https://bsd.ci/libioc
Other
38 stars 11 forks source link

add config with no exists config #630

Closed himrock922 closed 5 years ago

himrock922 commented 5 years ago

I commit of parameter that was said "unknown" with config. For example, The config property 'vnet_default_interface' is unknown Because I don't know very well about config detail, I need refactoring it.

This PR addresses #626

igalic commented 5 years ago

hrm… with the amount of config options we have to add, i think we should hurry up and make the config better structured.

as far as i can see it, there are jail(8) options, libioc (config) options, iocell and iocage compat options. We have a json like structure but, but currently, except for provisioning it's entirely flat.

igalic commented 5 years ago

@himrock922 may i recommend either adding the email address from this commit to your github email addresses — or else change it, so you're correctly credited!

himrock922 commented 5 years ago

Hi @igalic.

may i recommend either adding the email address from this commit to your github email addresses — or else change it, so you're correctly credited!

I'm sorry, after reset HEAD~, I did forcely commit that changes of email address.

himrock922 commented 5 years ago

Hi @gronke Thanks for reply.

thanks for your contribution. You have caused a whole bunch of new Issues that reflect new and missing features 😍Do you agree to deal with all the enhancements in separate PRs?

All right!, I don't know how much of can enhancements. But, I want cooperate of that enhancements.

Regards.

himrock922 commented 5 years ago

I beginning refactoring it. Question, Should I use not 0, but False? Also, Should I use not 1, but True?

gronke commented 5 years ago

@himrock922 you should use boolean types. The jail parameters are translated for the jail command in the JailGenerator._get_value method https://github.com/bsdci/libioc/blob/72cbed751e395609c915e529423b7eae91321a39/libioc/Jail.py#L1804-L1811

@fabianfreyer brought up the idea to read valid jail parameters from syctl, as seen here on a FreeBSD 12.0-RELEASE:

$ sysctl -t security.jail
security.jail.param.sysvshm.: integer
security.jail.param.sysvsem.: integer
security.jail.param.sysvmsg.: integer
security.jail.param.allow.mount.zfs: integer
security.jail.param.allow.mount.tmpfs: integer
security.jail.param.allow.mount.linsysfs: integer
security.jail.param.allow.mount.linprocfs: integer
security.jail.param.allow.mount.procfs: integer
security.jail.param.allow.mount.nullfs: integer
security.jail.param.allow.mount.fdescfs: integer
security.jail.param.allow.mount.devfs: integer
security.jail.param.allow.mount.: integer
security.jail.param.allow.socket_af: integer
security.jail.param.allow.quotas: integer
security.jail.param.allow.chflags: integer
security.jail.param.allow.raw_sockets: integer
security.jail.param.allow.sysvipc: integer
security.jail.param.allow.set_hostname: integer
security.jail.param.ip6.saddrsel: integer
security.jail.param.ip6.addr: opaque
security.jail.param.ip6.: integer
security.jail.param.ip4.saddrsel: integer
security.jail.param.ip4.addr: opaque
security.jail.param.ip4.: integer
security.jail.param.cpuset.id: integer
security.jail.param.host.hostid: unsigned long
security.jail.param.host.hostuuid: string
security.jail.param.host.domainname: string
security.jail.param.host.hostname: string
security.jail.param.host.: integer
security.jail.param.children.max: integer
security.jail.param.children.cur: integer
security.jail.param.dying: integer
security.jail.param.vnet: integer
security.jail.param.persist: integer
security.jail.param.devfs_ruleset: integer
security.jail.param.enforce_statfs: integer
security.jail.param.osrelease: string
security.jail.param.osreldate: integer
security.jail.param.securelevel: integer
security.jail.param.path: string
security.jail.param.name: string
security.jail.param.parent: integer
security.jail.param.jid: integer
security.jail.devfs_ruleset: integer
security.jail.enforce_statfs: integer
security.jail.mount_zfs_allowed: integer
security.jail.mount_tmpfs_allowed: integer
security.jail.mount_linsysfs_allowed: integer
security.jail.mount_linprocfs_allowed: integer
security.jail.mount_procfs_allowed: integer
security.jail.mount_nullfs_allowed: integer
security.jail.mount_fdescfs_allowed: integer
security.jail.mount_devfs_allowed: integer
security.jail.mount_allowed: integer
security.jail.chflags_allowed: integer
security.jail.allow_raw_sockets: integer
security.jail.sysvipc_allowed: integer
security.jail.socket_unixiproute_only: integer
security.jail.set_hostname_allowed: integer
security.jail.jail_max_af_ips: unsigned integer
security.jail.vnet: integer
security.jail.jailed: integer
security.jail.list: opaque
$ sysctl security.jail 
security.jail.param.sysvshm.: 0
security.jail.param.sysvsem.: 0
security.jail.param.sysvmsg.: 0
security.jail.param.allow.mount.zfs: 0
security.jail.param.allow.mount.tmpfs: 0
security.jail.param.allow.mount.linsysfs: 0
security.jail.param.allow.mount.linprocfs: 0
security.jail.param.allow.mount.procfs: 0
security.jail.param.allow.mount.nullfs: 0
security.jail.param.allow.mount.fdescfs: 0
security.jail.param.allow.mount.devfs: 0
security.jail.param.allow.mount.: 0
security.jail.param.allow.socket_af: 0
security.jail.param.allow.quotas: 0
security.jail.param.allow.chflags: 0
security.jail.param.allow.raw_sockets: 0
security.jail.param.allow.sysvipc: 0
security.jail.param.allow.set_hostname: 0
security.jail.param.ip6.saddrsel: 0
security.jail.param.ip6.: 0
security.jail.param.ip4.saddrsel: 0
security.jail.param.ip4.: 0
security.jail.param.cpuset.id: 0
security.jail.param.host.hostid: 0
security.jail.param.host.hostuuid: 64
security.jail.param.host.domainname: 256
security.jail.param.host.hostname: 256
security.jail.param.host.: 0
security.jail.param.children.max: 0
security.jail.param.children.cur: 0
security.jail.param.dying: 0
security.jail.param.vnet: 0
security.jail.param.persist: 0
security.jail.param.devfs_ruleset: 0
security.jail.param.enforce_statfs: 0
security.jail.param.osrelease: 32
security.jail.param.osreldate: 0
security.jail.param.securelevel: 0
security.jail.param.path: 1024
security.jail.param.name: 256
security.jail.param.parent: 0
security.jail.param.jid: 0
security.jail.devfs_ruleset: 0
security.jail.enforce_statfs: 2
security.jail.mount_zfs_allowed: 0
security.jail.mount_tmpfs_allowed: 0
security.jail.mount_linsysfs_allowed: 0
security.jail.mount_linprocfs_allowed: 0
security.jail.mount_procfs_allowed: 0
security.jail.mount_nullfs_allowed: 0
security.jail.mount_fdescfs_allowed: 0
security.jail.mount_devfs_allowed: 0
security.jail.mount_allowed: 0
security.jail.chflags_allowed: 0
security.jail.allow_raw_sockets: 0
security.jail.sysvipc_allowed: 0
security.jail.socket_unixiproute_only: 1
security.jail.set_hostname_allowed: 1
security.jail.jail_max_af_ips: 255
security.jail.vnet: 0
security.jail.jailed: 0

The first command prints each supported jail parameter type, the second one their length. To efficiently read those values, we'd need a native Python sysctl module - preferably without using Cython.

After reading those values and structuring the jail params in the config files properly (nesting with . notation), we can transparently pass through all valid jail parameters. When a config file defines one that is unknown, an according error can be triggered. That way we do not need to "support" new jail parameters, but always match with the host OS capabilities.

I left a lot comments on the added lines that, whenever necessary, resulted in a new issue. We should target each new issue in a separate PRs. @himrock922 please file a new Issue in case I forgot anything. In summary that is:

himrock922 commented 5 years ago

Hi @gronke.

I left a lot comments on the added lines that, whenever necessary, resulted in a new issue. We should target each new issue in a separate PRs. @himrock922 please file a new Issue in case I forgot anything. In summary that is:
Some config parameters are state, that should not be stored in a config, but read from the jail itself including ZFS properties (compressratio, ...)
Read (and write) support for ZFS properties on creation #633
Mount linprocfs #637
add cpuset to resource limits #636
Inheritance of host configuration (locale, timezone) #635
Host ID information stored in jails #634
last_started date of Jails #632
opt out from rtsold when accept_rtadv in an ip6_addr #631

All right! First, I do commit PR related to above issue. By the way, May I create PR not master branch but this PR as a target?

If I don't do that, I think hard to understand what I do fix.

Regards.

gronke commented 5 years ago

All right! First, I do commit PR related to above issue. By the way, May I create PR not master branch but this PR as a target?

If I don't do that, I think hard to understand what I do fix.

Of course you can do that. I'm unsure how GitHub handles it when we later merge to master instead of this branch, but let's find out!

Still I believe what you want to do is to cherry-pick your commit in your other PR branches and remove it before pushing your changes. That way you can point to master, but still have your ideas from this PR in place. Another supportive idea is that we can tick off lines in this PR as soon as we merge related changes elsewhere. However you wish :)

igalic commented 5 years ago

@himrock922 may i suggest you rebase instead of merging master into your branch?

himrock922 commented 5 years ago

Hi @igalic.

may i suggest you rebase instead of merging master into your branch?

right. Is my understanding of correct? After from my branch reverting merge master branch, that does rebase.

igalic commented 5 years ago

a rebase "simply" puts your changes on top of the latest master

himrock922 commented 5 years ago

Hi @igalic. Now I've rebased this branch on top latest branch. I'm sorry for the mistake.

igalic commented 5 years ago

no need to apologise

i am always happy to help people — with git or anything else!

himrock922 commented 5 years ago

ref #668 This PR related each parameters does migrate that it's from Defaults.py to Globals.py.

gronke commented 5 years ago

This PR related each parameters does migrate that it's from Defaults.py to Globals.py.

Yes, indeed the hardcoded defaults were moved to another file that stores the hardcoded defaults. To keep in mind we have three different places where a configuration is derived from:

  1. Global Hard-Coded defaults
  2. A defaults.json file in the top-level of a ioc_dataset (or activated pool)
  3. The jail dataset itself