environ config changes are never validated
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
juju-core |
Fix Released
|
High
|
Jesse Meek |
Bug Description
There's a Validate method on EnvironProvider for exactly this purpose, but it's never called when it needs to be (on SetConfig, for every provider).
This is really scary, and may allow users to irreparably break their environments (consider changes to name, type, firewall-mode, ...).
"Fixing" it for the dummy provider breaks a few tests, but probably not as many as it should; we should find out why.
*Really* fixing it could be rather tedious, because the state package is ignorant of environs and has no option but to accept whatever *config.Config is passed into SetEnvironConfig, regardless of compatibility or sanity. This could be resolved by registering config providers in the environs/config package, just as environ providers are registered in the environs package: we'd thereby actually be able to use and produce configs in a non-crippled way from state. But we'd also break a bunch more tests, which would all need to be understood and fixed.
So, probably not a 13.04 goal. But... this is a pretty critical chunk of data, and we are more than somewhat cavalier with it, and we should make an effort to .
Related branches
- Juju Engineering: Pending requested
-
Diff: 1846 lines (+475/-413)46 files modifiedcmd/juju/authorisedkeys_test.go (+1/-2)
cmd/juju/environment.go (+2/-20)
cmd/juju/environment_test.go (+2/-1)
cmd/juju/upgradejuju_test.go (+7/-14)
cmd/jujud/machine_test.go (+3/-8)
environs/config.go (+5/-5)
environs/config/config.go (+9/-0)
environs/statepolicy.go (+10/-0)
environs/testing/tools.go (+1/-7)
juju/conn.go (+3/-7)
juju/conn_test.go (+4/-10)
juju/testing/conn.go (+5/-1)
juju/testing/repo.go (+2/-5)
juju/testing/utils.go (+0/-14)
state/api/common/testing/environwatcher.go (+16/-7)
state/api/firewaller/state_test.go (+4/-7)
state/api/keymanager/client_test.go (+1/-2)
state/api/keyupdater/authorisedkeys_test.go (+1/-1)
state/api/logger/logger_test.go (+1/-1)
state/apiserver/client/client.go (+15/-30)
state/apiserver/client/client_test.go (+3/-12)
state/apiserver/keymanager/keymanager.go (+21/-21)
state/apiserver/keymanager/keymanager_test.go (+1/-2)
state/apiserver/keyupdater/authorisedkeys_test.go (+1/-1)
state/apiserver/logger/logger_test.go (+1/-1)
state/apiserver/provisioner/provisioner_test.go (+3/-6)
state/apiserver/rsyslog/rsyslog.go (+2/-10)
state/configvalidator_test.go (+114/-0)
state/conn_test.go (+9/-1)
state/initialize_test.go (+5/-5)
state/machine_test.go (+2/-3)
state/policy.go (+28/-0)
state/state.go (+48/-10)
state/state_test.go (+50/-19)
state/testing/agent.go (+1/-1)
state/testing/config.go (+0/-22)
upgrades/deprecatedattributes.go (+8/-21)
upgrades/deprecatedattributes_test.go (+3/-6)
upgrades/rsyslogport.go (+2/-9)
worker/authenticationworker/worker_test.go (+1/-2)
worker/environ_test.go (+17/-13)
worker/firewaller/firewaller_test.go (+17/-38)
worker/instancepoller/observer_test.go (+12/-15)
worker/machineenvironmentworker/machineenvironmentworker_test.go (+8/-8)
worker/provisioner/provisioner_test.go (+23/-39)
worker/uniter/uniter_test.go (+3/-6)
tags: | added: tech-debt weeks |
Changed in juju-core: | |
assignee: | nobody → Jesse Meek (waigani) |
Changed in juju-core: | |
status: | Triaged → Fix Committed |
Changed in juju-core: | |
status: | Fix Committed → Fix Released |
milestone: | none → 1.17.6 |