kubernetes
docs(release): add note to 1.22 about the CA CN rename
Summary
Add a note in 1.22 changelog about https://github.com/kubernetes/kops/pull/11921 et al's CA changes
Details
- this broke some of my teams' automation code that relied on the CN, so thought it would be good to call out in case anyone else stumbles upon this in order to not spend a few hours debugging 😅
- this change may particularly impact Prod environments, where the cluster has not been rebuilt in some time (years), and so they will have the old CN while new clusters in lower environments will have the new CN
- so code that relies on the CN may unexpectedly break Production while working fine in lower environments 😬 😬 😬
- fortunately we caught this in our QA env, but it passed Dev fine
- so code that relies on the CN may unexpectedly break Production while working fine in lower environments 😬 😬 😬
- this change may particularly impact Prod environments, where the cluster has not been rebuilt in some time (years), and so they will have the old CN while new clusters in lower environments will have the new CN
Testing Evidence
From an internal fix my team made to another team's automation:
(We've recommend they pull the CA from /etc/kubernetes/pki/ca.crt or /var/run/secrets/kubernetes.io/serviceaccount/ca.crt to be a bit more resilient to changes like this)
Review Notes
Feel free to change the language / wording as you see fit or put in a different part of the release notes (I put this under "Other changes of note" right now).
I thought specifically warning about older clusters was important, as otherwise this is particularly easy to overlook
Hi @agilgur5. Thanks for your PR.
I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
I must admit to being at a loss as to why anything would depend on the CN having a particular value.
The automation I mentioned here was for the Vault k8s integration, which requires the k8s CA.
Can see in the screenshot above that this code does TLS inspection to pull the CA, but ends up getting two certs, the CA and the kubernetes-master cert. It uses the CN to distinguish between the two.
I don't think that's the best way to get the CA, as I mentioned in the opening, but regardless, a team I work with already had code that specifically depended on the CN. I.e. the CN was part of the "public surface" that kOps exposes, and changing the CN broke existing code. Whether or not that code is optimal or why that code exists, the fact that any code exists anywhere that relies on the CN is enough to show this.
As is oft stated in Linux kernel development, anything that is exposed will be used (aka the "Workflow" xkcd).
So at the very least, I think this should be documented in the changelog, which is all that this PR does. Per the opening, I think it is doubly important to document since this has a higher likelihood to break code that depends on the CN in Prod, as those clusters are often older / rebuilt less often and so may still have the old CN. Per opening, code that worked in Dev may actually fail in Prod as a result, with potentially very significant consequences (in the case here, several Vault integrations would fail, eventually resulting in downtime).
I don't think this rises to the level of a release note. This is an implementation detail; the CN of a root CA is not something that is supposed to matter to anything.
I don't think this rises to the level of a release note.
It's a breaking change that broke actual code in production exactly because it wasn't mentioned in the release notes...
A reasonable user could suggest much more than a tiny release note when a change broke prod. This is the bare minimum, in my opinion.
the CN of a root CA is not something that is supposed to matter to anything.
It did. This is not a hypothetical... Only one counter-example is needed to show that.
kOps 1.22 has been out of support for about a year. One person making an incorrect assumption about internals does not merit a release note.
kOps 1.22 has been out of support for about a year.
I'll repeat once more that this breaking change affects clusters that upgraded to 1.22 and beyond and were not rebuilt in that time. Which is, again, much more likely for Prod clusters that have zero downtime (i.e. high criticality). This affects all clusters that existed pre-1.22 and were upgraded to 1.22+ through the standard kOps upgrade process.
A cluster that was upgraded from 1.21 to 1.26, which is current, would also have this issue. As written above, that cluster, which would be on 1.26, would have the old CN, while clusters built with 1.22+, and similarly on 1.26, would have the newer CN. Two clusters on 1.26 could have two different CNs, despite having the same configuration.
The CA has a different CN depending on what version of kOps the cluster was originally created with (not the version it's on).
One person making an incorrect assumption about internals
I'm not the one who made that assumption (and it was in fact, code reviewed by a whole team that has used kOps for years), and I, myself, literally said there are better ways of implementing their code. But I also would not have been able to definitively tell them not to rely on that. That is, similarly, undocumented behavior.
It is also a CA, they are not really meant to change frequently either. Whether a CA should be considered an "internal" is debatable.
One person making an incorrect assumption about internals
the CN of a root CA is not something that is supposed to matter to anything.
One could say that this too, is an assumption. And there is an existing counter-example that shows it did, in fact, matter to something... So one could further say that that is "an incorrect assumption" 🤷
I'm not here to play word games and state opinions, I am stating facts about events that literally already happened. I am here to warn other users about that, because if we did not catch that in QA, that would have resulted in a major issue to a large enterprise company. I would not wish production outages upon anybody, nor hours of debugging and root cause analysis (which required reading through kOps source code and individual commit diffs), and the absolute least I can do is add a tiny warning to the release notes.
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: hakman
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [hakman]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages.
The list of commits with invalid commit messages:
- 3138f04 Apply suggestion from @hakman
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.