APM: remove output attribute

[pebrc]

The output attribute in the APM spec duplicates the output element from APM server config and is confusing users as the effective configuration will be the merged result of both output elements. Also users could be under the impression that they can add any valid output attribute into the top level output element and in the absence of preserveUnknownFields:false the API server will actually accept those additional attributes which will have no effect and lead to more confusion.

We should retire the output attribute at the top level of the CRD in favour of a simple ElasticsearchRef .

This will make it clearer to users what the difference between the two things is.

[barkbay]

While I'm working on #1240 here are some thoughts about the spec of the APM server:

Current APM server spec

Here is the current spec for the APM server (the only purpose is to show all the available fields in spec.output):

apiVersion: apm.k8s.elastic.co/v1alpha1
kind: ApmServer
metadata:
  name: apm-server-sample
spec:
  version: "7.1.0"
  nodeCount: 1
  config:
    output:
      elasticsearch:
        headers:
          X-My-Header: Just an example of a custom settings
  output:
    elasticsearch:
      ref:
        name: elasticsearch-sample
        namespace: default
      hosts: ["es.example.com"] # Does not make sense to keep it, can be set in spec.config
      auth:
        inline: # Does not make sense to keep it:
          username: foo # can be set in spec.config
          password: bar # can also be set in spec.config with help of the keystore
        secret: # key is the username, Data[key] is the password, any reason to keep it ?
          name: my-secret
          key: key-in-secret
      ssl:
        certificateAuthorities:
          secretName: my-secret

Proposal for a new specification

Elasticsearch is managed by the operator

If the user has deployed Elasticsearch with the operator, only the elasticsearch.ref is needed:

apiVersion: apm.k8s.elastic.co/v1alpha1
kind: ApmServer
metadata:
  name: apm-server-sample
spec:
  version: "7.1.0"
  nodeCount: 1
  config:
    output:
      elasticsearch:
        headers:
          X-My-Header: Just an example of a custom settings
  elasticsearch: # does it still look like a duplicate ? Could be elasticsearchRef to be consistent with Kibana.
    ref:
      name: elasticsearch-sample
      namespace: default

External / Unmanaged Elasticsearch instance

With a custom Elasticsearch cluster user can set the password with the help of the keystore.
We only keep certificateAuthorities to help the user to import an existing certificate authority:

apiVersion: apm.k8s.elastic.co/v1alpha1
kind: ApmServer
metadata:
  name: apm-server-sample
spec:
  version: "7.1.0"
  nodeCount: 1
  secureSettings:
    secretName: apm-secret-settings # secret that contains ES_PASSWORD
  config:
    output.elasticsearch:
      hosts: ["my-apm-server:9200"]
      protocol: "https"
      username: elastic
      password: "${ES_PASSWORD}"
      headers:
        X-My-Header: Just an example of a custom settings
  elasticsearch:
    certificateAuthorities:
      name: additional-ca # Secret in the same namespace that holds additionnal CAs

[pebrc]

Elasticsearch is managed by the operator

👍 that is what I had in mind. I am not sure if elasticsearch.ref is better or elasticsearchRef maybe we should check what we have done in other places

External / Unmanaged Elasticsearch instance

elasticsearch:
   certificateAuthorities:
      name: additional-ca

I am not sure about that one. Should we not do something similar to #1133 and just allow people to use the podtemplate spec to mount the secret and use append semantics to add user provided CAs to the ECK provided ones if any.

[barkbay]

elasticsearchRef is used in the Kibana spec:

apiVersion: kibana.k8s.elastic.co/v1alpha1
kind: Kibana
metadata:
  name: kibana-sample
spec:
  version: "7.1.0"
  nodeCount: 1
  elasticsearchRef:
    name: "elasticsearch-sample"

I would use the same for the APM server.

I am not sure about that one. Should we not do something similar to #1133 and just allow people to use the podtemplate spec to mount the secret and use append semantics to add user provided CAs to the ECK provided ones if any.

Yes, I agree, just wanted to offer an easy way for the user to set it's own CA. But you are right, if it can be done with what we already offer we should keep the spec simple.

[pebrc]

I think this was done in #1345