Add coreos-update-ca-trust.service #120
Conversation
| ConditionDirectoryNotEmpty=/etc/pki/ca-trust/source/anchors/ | ||
| # We want to run quite early, in particular before anything | ||
| # that may speak TLS to external services. In the future, | ||
| # it may make sense to do this in the initramfs too. |
There was a problem hiding this comment.
WRT the initramfs, I'm not sure it's needed. The only things in there I can think of that might need TLS are Ignition (which supports custom certs in the ignition section) and afterburn, which is talking to cloud providers.
There was a problem hiding this comment.
Right coreos/ignition#636 is about this, but I can imagine people wanting to have the ones provided to Ignition persist for the host too.
But, this doesn't conflict with that; if Ignition writes it optionally, this service could read it.
There was a problem hiding this comment.
I'm not sure I 100% follow? Ignition does not write them to the initramfs root, it adds them to its HTTP client directly from the config. I also think it would be a little misleading for Ignition to write them out to the initramfs root since the ignition section is just for configuring Ignition, not the initramfs in general. I still don't see a use case for this either.
If people want them to persist to the host they should add storage.files entries. We could also add sugar to FCCT for this that generates both.
There was a problem hiding this comment.
People have asked that Ignition persist the CA to disk, but I'm very much against this. The ignition section of the config directs Ignition; nothing more.
There was a problem hiding this comment.
Right, no one is talking about changing that default, just adding a new mechanism to do both.
|
Half of me wants this as FCCT sugar instead since that would prevent any future compatibility constraints. If we ship it in the OS we need to be very careful about how we update it to not break systems. On the other hand this is pretty simple. |
|
What updates are you thinking of that we might do that could break something? Is there anything similar in CL that was changed and broke something? |
jlebon
left a comment
jlebon
left a comment
There was a problem hiding this comment.
This looks reasonable to me. The mechanic here is pretty clean. Just two comments.
| # We want to run quite early, in particular before anything | ||
| # that may speak TLS to external services. In the future, | ||
| # it may make sense to do this in the initramfs too. | ||
| DefaultDependencies=no |
There was a problem hiding this comment.
This needs ConditionFirstBoot=yes too, right? I don't think we want users to rely on reboots magically having any new certs they added be extracted.
There was a problem hiding this comment.
If we did this, we wouldn't be able to use this service for the MCD.
There was a problem hiding this comment.
MCD doesn't run on FCOS right now. MCD could also add it's own drop in for turning ConditionFirstBoot=no if needed.
There was a problem hiding this comment.
Have we verified that systemd honors conditions in drop-ins? I seem to recall this not working in the past.
There was a problem hiding this comment.
In playing with this, it seems to work to do ConditionFirstBoot= but not ConditionFirstBoot=no. Dunno exactly why, maybe something about "reset semantics" vs override?
There was a problem hiding this comment.
When a user adds a new cert via machineconfig, wouldn't that restart the hosts anyway? So, in that case, we would actually want this to run on firstboot only.
cgwalters
force-pushed
the
ca-trust
branch
2 times, most recently
from
1839a6b to
a530a17
Compare
|
Actually the |
If...we decided to drop the condition? Spell this out more? |
jlebon
left a comment
jlebon
left a comment
There was a problem hiding this comment.
LGTM! Will let @ajeddeloh do the final merge.
Yeah, or add other conditions, dependencies, etc. Basically I'm hesitant to add configuration for things that users could write just as easily, or that can be generated by FCCT. In that case we basically make FCCs always generate the same config given that same config version and if we want to make changes we bump the config version. Basically I'm not sure if this should be our responsibility to manage/maintain. Curious if @bgilbert has thoughts. I feel like we've had a similar discussion for something in CL before but I don't remember what. |
|
In fact, if we added it to FCCT, we could make it autogenerate this unit when they want CAs in the host as well as ignition. e.g. something like: certs:
- name: foo
- contents: <foo>
- ignition: true
- host: truecould generate that unit, enable it, add the CAs to the |
|
That's presuming the only thing generating Ignition for the node is FCCT. Since part of the plan is to have e.g. OKD target FCOS, and the installer today generates Ignition too, we would need to merge them. Literally in fact this same time we're discussing this someone added a duplicate of this service to the installer, which would race. It seems more like a fundamental base OS feature - the concerns you have about compatibility seem too vague to me to outweigh the utility of having it in the base OS. |
It's presuming there is only one tool generating the config, not necessarily FCCT, just not FCCT in tandem with another tool.
I don't think that race situation is limited to just this case; any other programs a user might want to only run once on first boot would also suffer this problem. Any time you have two tools that don't know about each other both generating Ignition configs then you could hit similar problems. I think it'd be better to ensure that either there's only one thing generating the Ignition config or that all the pieces that generate configs are aware of each other. I guess this gets at the larger question of "how will OKD configuration work". side note: if they use the same unit name then the units will be merged (child's config's contents replacing the parent's). |
|
Putting the unit file in FCCT wouldn't remove compatibility constraints, just move them around. For example, |
|
Hrm that's a good point, though in general things like binary paths are also potential compat issues. I'm fine with this.
I think it's fine to have non-trivial code in FCCT as long as it's using things we know have strong compat guarantees. For example, writing out an |
|
For reference:
|
This blends well with Ignition, allowing people to just drop in certs without having to worry about injecting their own systemd service to update them. We also want this for RHCOS. See: openshift/machine-config-operator#528 For FCOS, in theory this could be a "run at most once" service. But for RHCOS, since we support dynamic changes, we need it to run on each boot too. The cost of doing so is very small.
Thanks, that's useful!
Hm...without diving in a lot there, I'm guessing what happened is that at some point the bundle started to be generated in the base image, which defeated the point of the unit. Here we're using (Though, as an aside...
Again offhand...that looks likely to be fallout from some distribution misconfiguration. The OpenSSL is definitely configured to read the bundle |
|
Rebased 🏄♂️ Though as of this moment we merged into RHCOS a variant of this unit without |
|
I don't want to derail this PR, but I do wonder if longer term we would/should have something akin to |
How would that be different from dropping the |
|
@cgwalters yes, that was implicit point in favor of dropping the first-boot condition, I guess.
|
It updates the pre-built bundle that TLS libraries like OpenSSL, gnutls, golang's, etc. know how to read.
Right, that's where the bundle lives. What issue are you thinking of? |
|
How do we drive this to consensus? Options seem to be:
Based on discussion so far...I don't have a strong opinion between current and RHCOS-identical; there are arguments both ways that just leave me feeling "eh". (Argument for retain as is is that if someone wants it to run manually they can just invoke |
|
Which method is tested with more layered product offering expectations? |
|
I'm in favor of shipping with the conditional. It matches with the "Set everything up on first boot" scheme. If users want to mask/disable it, they can. If they want to run every boot, they can disable the condition by setting it to the default or write their own unit. In fact, if they're writing files there after first boot, whatever is writing those files should probably own running update-ca-trust or setting it up to run. |
Yeah, I think that could make sense, though it seems safer to me to drive a change like this in the rest of Fedora too (and in the process shake out any other repercussions this may have for crypto libraries). For now, it's much easier to commit to this API with
Also in favour. Let's give it a bit more time and if no new objections are raised, let's merge it! |
This adds a migration note to highlight possibly different `docker` defaults as part of Fedora packaging, notably around logging and live-restore.
8 participants
This blends well with Ignition, allowing people to just drop
in certs without having to worry about injecting their own
systemd service to update them.
We also want this for RHCOS.
See: openshift/machine-config-operator#528
For FCOS, in theory this could be a "run at most once" service.
But for RHCOS, since we support dynamic changes, we need it to
run on each boot too. The cost of doing so is very small.