ADR about security-responder #9006
Conversation
|
This looks good to me 👍 |
manuelbuil
force-pushed
the
adr-metrics-responder
branch
from
3ec4545 to
e0892fd
Compare
|
Ok for me I would add that there should be a possibility in Rancher / Rancher Prime deployment context to setup for the data to be collected by Rancher (with Rancher acting as "proxy" to needed data) |
|
@pgonin We'll have EIO stand up another RMS/Prime gathering similar data about systems under management is a very different discussion, let's have that elsewhere please. |
|
Agreed, it should be a second step. To be discussed together with Rancher actually |
docs/adrs/010-security-responder.md
Outdated
| - nodeCount | ||
| - serverNodeCount | ||
| - agentNodeCount |
There was a problem hiding this comment.
nit: nodeCount = serverNodeCount + agentNodeCount. A separate field seems unnecessary?
There was a problem hiding this comment.
Agreed.
I think the intent is to differentiate between pure control plane nodes and those that run workloads. For example, SUSE's pricing model excludes pure control plane nodes.
There was a problem hiding this comment.
How is that determined? There is no such thing as a "pure" control-plane node as far as Kubernetes or RKE2 are concerned. Are you saying that customers don't have to pay for server nodes (cp/etcd/cp+etcd) if they put some taints on them to prevent workload scheduling? What taints, exactly?
brandond
left a comment
brandond
left a comment
There was a problem hiding this comment.
a few nits, and a comment on deployment opt-out and configuration
manuelbuil
force-pushed
the
adr-metrics-responder
branch
from
e0892fd to
df0de20
Compare
docs/adrs/010-security-responder.md
Outdated
| } | ||
| ``` | ||
|
|
||
| The `clusteruuid` is needed to differentiate between different deployments (the UUID of `kube-system`). It is completely random and does not expose privacy considerations. |
There was a problem hiding this comment.
Probably not worth addressing here, but I have seen some users that build images with RKE2 already installed and started. This is a bad idea for several reasons, but it happens. When this is done, the clusters will have the same root CAs - and the same kube-system namespace UID. This will cause them to look like the same cluster to anything using that UID as a cluster UID. I don't know that there's a good way to work around this, but it is something to be aware of.
manuelbuil
force-pushed
the
adr-metrics-responder
branch
from
df0de20 to
0c35abc
Compare
docs/adrs/010-security-responder.md
Outdated
|
|
||
| ```yaml | ||
| # /etc/rancher/rke2/config.yaml | ||
| security-responder-enabled: true # default |
There was a problem hiding this comment.
| security-responder-enabled: true # default | |
| security-responder: true # default |
Having "enabled" in the name is redundant for a bool flag. We don't follow that style or any of our other bool flags like embedded-registry or supervisor-metrics.
There was a problem hiding this comment.
yeah I already requested replacement of this with a normal --disable flag, since it'll be packaged as a helm chart. no special flag necessary. That bit of review seems to have been dropped though.
There was a problem hiding this comment.
Sorry about that! 🤦
docs/adrs/010-security-responder.md
Outdated
| } | ||
| ``` | ||
|
|
||
| The `clusteruuid` is needed to differentiate between different deployments (the UUID of `kube-system`). It is completely random and does not expose privacy considerations. |
There was a problem hiding this comment.
We might consider hashing the clusteruuid to ensure another level of obfuscation for collected metrics.
There was a problem hiding this comment.
Let me add it as an option. I think we can discuss about this in the real PR
There was a problem hiding this comment.
Yeah, I'm kinda meh on that. I don't see what hashing an already random number would add, other than the appearance of obfuscation.
docs/adrs/010-security-responder.md
Outdated
|
|
||
| ```yaml | ||
| # /etc/rancher/rke2/config.yaml | ||
| security-responder-enabled: true # default |
There was a problem hiding this comment.
| security-responder-enabled: true # default | |
| disable: | |
| - rke2-security-responder |
Signed-off-by: manuelbuil <[email protected]>
Signed-off-by: manuelbuil <[email protected]>
manuelbuil
force-pushed
the
adr-metrics-responder
branch
from
0c35abc to
bcc461d
Compare
Could you check again Brad? Thanks |
5 participants
Proposed Changes
Types of Changes
Verification
Testing
Linked Issues
User-Facing Change
Further Comments