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

Upload dockerd configuration over HTTP #212

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

killenheladagen
Copy link
Contributor

@killenheladagen killenheladagen commented Apr 16, 2024

Let admin upload daemon.json over HTTP, enabling control over those part of the dockerd configuration that are not exposed through parameters today.

Checklist before requesting a review

  • I have performed a self-review of my own code
  • I have verified that the code builds perfectly fine on my local system
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have verified that my code follows the style already available in the repository
  • I have made corresponding changes to the documentation

@killenheladagen killenheladagen requested a review from a team as a code owner April 16, 2024 11:01
@killenheladagen killenheladagen changed the base branch from upload-recovery to reject-bad-tls April 16, 2024 11:15
Copy link
Contributor

@madelen-at-work madelen-at-work left a comment

Choose a reason for hiding this comment

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

This is a nice feature but it is not needed to get rootless Docker delivered.
In addition it adds a new dependency on the fastcgi setup meaning that that would be harder to remove if we find a better solution for the TLS upload
It is also not clear how we explain to the user (or handle) if the file contains settings that we already use as inputs to dockerd.

I would prefer that you do not merge this.

Base automatically changed from reject-bad-tls to main April 16, 2024 13:29
@killenheladagen
Copy link
Contributor Author

This is a nice feature but it is not needed to get rootless Docker delivered. In addition it adds a new dependency on the fastcgi setup meaning that that would be harder to remove if we find a better solution for the TLS upload It is also not clear how we explain to the user (or handle) if the file contains settings that we already use as inputs to dockerd.

I would prefer that you do not merge this.

The main use case for this is to allow ourselves to upload proxy settings without SSH. If we would switch to a different solution for TLS upload, I assume that solution would also work for daemon.json.

If a configuration is specified both in daemon.json and on the command line, dockerd will not start. Perhaps we need consider this even for the current way we expose daemon.json. We could add a note in the documentation about which entries in this file that are not configurable.

I find it odd that we provide a mechanism for per-device configuration of dockerd but we don't want to make it as convenient as possible to use, even for ourselves.

An alternative could be to provide the proxy settings as AXParameters, or if there are already other proxy settings in the device that we can use.

@killenheladagen killenheladagen marked this pull request as draft April 19, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants