cockroach-operator: operator incorrectly uses `--logtostderr` + can't use new --log flag with Operator

Okay, so I took some time to explore the solution space here and I have a few updates.

For version 21.1 and beyond, we can capture the appropriate log messages using the following log config on startup:

cockroach start --log='sinks: {stderr: {channels: OPS, redact: true}}' <other flags>

This will only pass redacted messages from the OPS channel to the stderr sink as Raphael pointed out above.

For versions 20.1 and 20.2 that we are supporting on the Operator the story is a bit more complicated. I spoke with Raphael, Josh on our SRE team for Cloud, and Daniel from the SE team about expectations for K8s logs. On CockroachCloud, we actually do not log anything out to stderr (outside of the initial logs emitted on node startup) in K8s and instead the SREs exec into pods to check logs. Josh noted that this ran against his expectations initially but this has become standard practice and the team is now used to doing this. Of course, this still runs the risk that if the node is crashing, you won’t be able to access logs that are persisted. Josh noted that having the full unredacted logs available somewhere is important as an operator though.

Daniel from our field team had a similar concern. When he has stood up K8s instances for customers, he usually sets up a sidecar pod to separately persist logs or streams them out to a log collection service. This lines up with our long term vision, but the complexity of setting up a sidecar for logging is too much for the GA timeline.

Raphael suggested that we may be able to backport some log redaction functionality into the 20.x releases. @knz would you please share how much of an effort you would expect that to be? I’m very hesitant to go with this route for the GA release for the operator in ~2 weeks.

In sum, we have 3 options here:

1. Keep the 20.x releases as they are with all logs going to stderr. There is a security risk inherent in doing this as those logs will contain sensitive information like PKs from the database (and much more).
2. Try to modify stderr logging behavior for 20.x release via backport.
3. Remove stderr logging, keeping only what is printed out on node startup. End users will need to exec into pods or use the HTTP logging endpoints to access logs.

Given our time constraints and that the SRE team has been using option 3 in prod in CockroachCloud, I would lean towards option 3. Versions 20.x will only be supported for another year, and depending on our TDB support policy for the operator, we may even sunset those versions earlier. After that, all versions will be able to support the above setting available from v21.1 and beyond.

ok so this is still a “bug” issue, not “enhancement”.

I think we’ve been talking past each other and somehow I have not been successful at conveying how serious the situation is.

I’m going to rephrase this more clearly.

As of today, the current operator code wrt logging has been misdesigned. What is being done was incorrect from the start, and the fact nobody noticed does not detract from the fact it’s a problem and needs to be corrected. (See this definition)

What is wrong exactly?

1. the flag --logtostderr exposes sensitive information to the process’ stderr in a way that does not provide redactability. Consequently, plugging that output to the kubectl log command can defeat IT policies about confidentiality.

   This is a serious issue that multiple customers have complained about in the 19.x cycle and we solved in 20.1/20.2 by creating product features and generally documenting that stderr logging should not be used, but somehow this operator project did not get the memo.

2. the flag --logtostderr creates output that cannot be reliably parsed. Presumably, customers who integrate with orchestration wish the ability to process logging data automatically. The output produced by crdb on its stderr stream fails at this. Even though it looks like something parseable on the surface, it is a documented fact that under stress and in problem situations, the log entries become malformed. This is not fixable via stderr logging, but is fixable via file or network logging.

   Right now the default config used by the k8s operator code is mistakenly giving users the impression they can rely on the logging output by kubectl log and automate log monitoring etc based on that data. This is a lie to our users/customers.

3. you and others who are working on the operator project are working under the mistaken impression that “logging” is somehow distinct from “database state”. I see this for example in your suggestion that “if we store log files we’d need ephemeral storage” or the question “how would the operator look at the logs without execing into the node”. That is something we need to work on culturally.

   In a nutshell, crdb logging data has the same quality as general stored SQL data.

   We don’t think about “how would the operator access the SQL data without execing into the node”, right? To access SQL data, we open a SQL client.

   The same applies to crdb logs. To access the log data, we use the log retrieval API (over HTTP).

   Generally, the log files should be treated with the same level of care for persistence and confidentiality as the data files: they are absolutely crucial to preserve durably, just like the crdb store data directories, and that’s specificially the reason why crdb choses a logs sub-directory of the store directory by default. If the customer cares about at-rest confidentiality, the log files should be encrypted together with the data files.

   Right now, we are lucky that the flag --logtostderr merely copies log data to stderr but still preserves crdb’s default behavior, which is to also create discrete files in the logs directory. The statefulset code, either intendedly or not, was not implemented to explicitly disable writing logging data to files in the store directory, and that’s a good thing!

4. starting in v21.2, the --logtostderr flag will be removed. So the current startup code will break.

5. the best practice with distributed systems with crdb is to aggregate the logging output from all the nodes into a network logging collector. Arguably, it is the k8s operator’s role to spawn a logging collector, such as Fluentd, alongside the crdb nodes inside the k8s cluster and “plug” the services together by making the logging config inside crdb point to the logging collector. This way the crdb nodes would send their data reliably, in a parseable format, to Fluentd, where it can be stored reliably and confidentially.

   This organization for log collection is what both our on-prem customers and our CC teams have requested and we’ve built in 21.1. It is legitimately the best practice for SREs and our k8s operator should definitely exemplify this best practice.

   I am surprised this hasn’t been discussed yet.

So these are the problems and now let’s review the suitable actions:

Let me know if you have further questions.

@piyush-singh can you please document business requirements. And keep in mind we always need logging to console.