elastic / cloud-on-k8s

Elastic Cloud on Kubernetes
Other
53 stars 707 forks source link

Somehow reduce repetition in ES node config. #1757

Open jordansissel opened 5 years ago

jordansissel commented 5 years ago

Today we had Kibana giving 500 error and the following error from Elasticsearch in Kibana's logs:

[security_exception] Cannot find any matching realm for [SamlPrepareAuthenticationRequest{realmName=cloud-saml, assertionConsumerServiceURL=null}]

Ultimately, this occurred because we have 3 different groups of nodes:

apiVersion: elasticsearch.k8s.elastic.co/v1alpha1
kind: Elasticsearch
metadata:
  name: redacted
spec:
  version: 7.3.1
  nodes:
  - nodeCount: 3
    config:
      node.master: true
      node.data: false
      node.ingest: false
      node.ml: false
      xpack.security.authc.realms: &xpack-realms
  - nodeCount: 20
    config:
      node.master: false
      node.data: true
      node.ingest: true
      node.ml: false
      xpack.security.authc.realms: *xpack-realms
  - nodeCount: 1
    config:
      node.master: false
      node.data: false
      node.ingest: false
      node.ml: true

Spot the error? The solo ML node was missing xpack.security.authc.realms

Now, I'm already trying to reduce copy/paste by using YAML anchors, but that doesn't save me from forgetting.

Any thoughts on having a broad shared/default config for the whole Elasticsearch resource, with config: overrides possible per nodeSet?

jordansissel commented 5 years ago

Oops, premature submission. I fixed the description to be more complete.

jordansissel commented 5 years ago

I'm also thinking about maybe using OPA to reject configurations where we might make small mistakes like this.

anyasabo commented 5 years ago

I wasn't familiar with OPA but looks like it is essentially a framework for admission webhooks? We do some validating already but so far have punted on trying to validate ES configs (I believe because of the high chance of not doing it well, but someone may correct me here).

I'm definitely open to this, but I'm wary of having multiple levels of settings, defaults, and overrides. It gets complicated fast. But as you mentioned there can be a lot of repetition and the security configs themselves are pretty nested, so it can be a pain to troubleshoot.

jordansissel commented 5 years ago

+1 to it being complicated, and it may not even be possible, since ECK may not be familiar with any business-specific ideas that need validating, such as in my case where I want all nodes to have SAML setup -- I can imagine for some companies, "Only coordinating nodes have may saml" or something might be the correct config. I dunno.

nkvoll commented 5 years ago

I've hinted a few times at having some kind of base nodeset spec (ignore the specific key name for the purposes of the idea):

apiVersion: elasticsearch.k8s.elastic.co/v1alpha1
kind: Elasticsearch
metadata:
  name: redacted
spec:
  version: 7.3.1
  nodeSetTemplate:
    # same format as elements under "nodes", will be used as a base template where the node-specific
    # elements may override.
    config:
      xpack.security.authc.realms: &xpack-realms
    podTemplate:
      annotations:
        foo: bar
    ...
  nodes:
  - nodeCount: 3
    config:
      node.data: false
      node.ingest: false
      node.ml: false
  - nodeCount: 20
    config:
      node.master: false
      node.ml: false
  - nodeCount: 1
    config:
      node.master: false
      node.data: false
      node.ingest: false

This would provide a base way of providing configuration or settings / pod settings that would be common between all the different nodesets in the resource. WDYT?

sebgl commented 5 years ago

One technical detail that is quite hard is merging podTemplates together. So far, we inherit a nodeset podTemplate and add our own values into it. Which is not really "merging", but rather "patching a few specific fields we know how to patch".

Here we would have to first merge the base nodeSetTemplate with the specific nodeSetTemplate. I think technically it means we have to go through every single field of the specific nodeSetTemplate to check if maybe there's a difference with the base one. Some of those being deeply nested pointers, lists, maps, etc. Which is a mess (not saying it's not feasible, though) that needs to be maintained over time with pod schema evolution; may not be worth it.

Merging the ES config bit is easier.

Overall I'm not sure we should do this vs. the user just fiddling around with their own yaml tooling if they want to avoid repetition.

pebrc commented 4 years ago

I wonder if we could introduce the configRef we have in Beats and Enterprise Search to share Elasticsearch configuration between nodeSets and even Elasticsearch clusters if desired.

We would need to merge the referenced secret with the inline config section but that should be manageable (as we are already doing that for internal Elasticsearch configuration ECK creates). This would allow users to share common segments like LDAP configuration at so forth.

thbkrkr commented 4 years ago

Relates to #3401.