canonical / cloud-init

Official upstream for the cloud-init: cloud instance initialization
https://cloud-init.io/
Other
2.99k stars 883 forks source link

#cloud-config-jsonp is almost completely useless #5549

Open TheRealFalcon opened 3 months ago

TheRealFalcon commented 3 months ago

#cloud-config-jsonp is a way to specify JSON Patch operations to apply to cloud-init's existing configuration. The problem is, in its current form, it is almost completely useless.

I think that the idea is to have a convenient way (even though we already have merge configuration?) to merge user data with previous config. The problem is due to the ordering of how we process cloud-config, it will often attempt to be applied before it ever has a chance to patch anything.

Our docs call out using it as a way for user data to override vendor data, but this does not work, because the jsonp part gets processed before vendor data.

The only way this can work as expected is if you have a MIME multipart user data in which the jsonp attachment is ordered after an earlier attachment that you want to patch. This seems like an extreme corner case because:

  1. Merge strategy is available
  2. If you control your user data parts, why would structure them such that overlap in a way that needs to be merged this way?
  3. If you don't control your user data parts, how can you ensure that they get ordered correctly?

Given that feature also pulls in the jsonpatch dependency, I think we should remove this feature soon in a upcoming release.

holmanb commented 3 months ago

Our docs call out using it as a way for user data to override vendor data, but this does not work, because the jsonp part gets processed before vendor data.

Thanks for dropping this incorrect documentation in #5551.

Given that feature also pulls in the jsonpatch dependency, I think we should remove this feature soon in a upcoming release.

It sounds like it is error prone/broken and doesn't add value over other types. Agreed - lets ditch it.