Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nil pointer dereference in Agent config transpiler #23734

Closed
pebrc opened this issue Jan 28, 2021 · 10 comments · Fixed by #24760
Closed

Nil pointer dereference in Agent config transpiler #23734

pebrc opened this issue Jan 28, 2021 · 10 comments · Fixed by #24760
Assignees
Labels
bug Team:Elastic-Agent Label for the Agent team v7.13.0

Comments

@pebrc
Copy link

pebrc commented Jan 28, 2021

While exploring the standalone Elastic Agent support through the ECK operator I ran into this sigsegv:

2021-01-28T13:09:23.591Z    DEBUG    application/periodic.go:82    Updated 1 files: /etc/agent.yml                                                                                                                                                                                                                   │
│ panic: runtime error: invalid memory address or nil pointer dereference                                                                                                                                                                                                                                              │
│ [signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0xb6b8f3]                                                                                                                                                                                                                                              │
│ goroutine 131 [running]:                                                                                                                                                                                                                                                                                             │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*List).Clone(0xc000339a40, 0xc000627620, 0xc000537650)                                                                                                                                                                                        │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:353 +0xc3                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*Key).Clone(0xc000626b00, 0x1c34c00, 0xc000627620)                                                                                                                                                                                            │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:237 +0x41                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*Dict).Clone(0xc0003395c0, 0x0, 0x1)                                                                                                                                                                                                          │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:129 +0xcd                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*List).Clone(0xc000339590, 0xc000627560, 0xc000627540)                                                                                                                                                                                        │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:353 +0xcd                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*Key).Clone(0xc000626c20, 0x1c34c00, 0xc000627560)                                                                                                                                                                                            │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:237 +0x41                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*Dict).Clone(0xc000339320, 0x0, 0x1)                                                                                                                                                                                                          │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:129 +0xcd                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*List).Clone(0xc0003392f0, 0xc000627400, 0xc000537230)                                                                                                                                                                                        │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:353 +0xcd                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*Key).Clone(0xc000626cc0, 0x1c34c00, 0xc000627400)                                                                                                                                                                                            │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:237 +0x41                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*Dict).Clone(0xc0003391a0, 0xc000a79a40, 0x44e57c)                                                                                                                                                                                            │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:129 +0xcd                                                                                                                                                                                                                      │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler.(*AST).Clone(...)                                                                                                                                                                                                                              │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/transpiler/ast.go:774                                                                                                                                                                                                                            │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.(*emitterController).update(0xc0000b9400, 0x0, 0x0)                                                                                                                                                                                           │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/emitter.go:107 +0xfe                                                                                                                                                                                                                 │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.(*emitterController).Update(0xc0000b9400, 0xc0009f0810, 0x0, 0x0)                                                                                                                                                                             │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/emitter.go:79 +0x35b                                                                                                                                                                                                                 │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.emitter.func2(0xc0009f0810, 0x1, 0x1)                                                                                                                                                                                                         │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/emitter.go:165 +0x34                                                                                                                                                                                                                 │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.readfiles(0xc000a202f0, 0x1, 0x1, 0xc0009a6100, 0xc000a79e08, 0x2)                                                                                                                                                                            │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/emitter.go:175 +0x111                                                                                                                                                                                                                │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.(*periodic).work(0xc0009a8120, 0x0, 0x0)                                                                                                                                                                                                      │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/periodic.go:89 +0x445                                                                                                                                                                                                                │
│ github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.(*periodic).Start.func1(0xc0009a8120)                                                                                                                                                                                                         │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/periodic.go:27 +0x40                                                                                                                                                                                                                 │
│ created by github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application.(*periodic).Start                                                                                                                                                                                                                  │
│     /go/src/github.com/elastic/beats/x-pack/elastic-agent/pkg/agent/application/periodic.go:26 +0x3f     

when using the following input:

    inputs:
      - id: d8b4d520-615b-11eb-847e-6bd07fa09e69
        name: log-1
        revision: 1
        type: logfile
        use_output: default
        meta:
          package:
            name: log
            version: 0.4.6
        data_stream:
          namespace: default
        streams:
          - id: logfile-log.log
            data_stream:
              dataset: generic
            type: container
            paths:
               - /var/log/containers/*.log
            symlinks: true
            processors:
               - add_kubernetes_metadata:

It is the null valued -add_kubernetes_metadata: that leads to the panic.

Affected version: 7.10.2

@pebrc pebrc added bug Team:Elastic-Agent Label for the Agent team labels Jan 28, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@ph
Copy link
Contributor

ph commented Jan 28, 2021

I though we had unit test for this kind of scenario in the AST, I believe using add_kubernetes_metadata: ~ is a valid work around, but I haven't tested it.

@michalpristas michalpristas self-assigned this Jan 29, 2021
@ph ph assigned faec and unassigned michalpristas Feb 24, 2021
@michalpristas
Copy link
Contributor

i'm having issues reproducing this both using tests and using 7.10.2 release, can you attach whole config as you have it?

@pebrc
Copy link
Author

pebrc commented Mar 23, 2021

OK so this took me a while to reproduce. This is on ECK so Agent is running in standalone mode. The trick is that you first create the agent without the offending -add_kubernetes_metadata: attribute and then update the configuration to include it.

So to begin we have an Elasticsearch cluster (and a Kibana which I omitted here):

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: elasticsearch
spec:
  version: 7.10.2
  nodeSets:
  - name: default
    count: 3
    config:
      node.store.allow_mmap: false

Then I create the Elastic Agent:

---
apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
metadata:
  name: elastic-agent
spec:
  version: 7.10.2
  elasticsearchRefs:
  - name: elasticsearch
  daemonSet:
    podTemplate:
      spec:
        serviceAccountName: agent
        automountServiceAccountToken: true
        terminationGracePeriodSeconds: 30
        dnsPolicy: ClusterFirstWithHostNet
        hostNetwork: true # Allows to provide richer host metadata
        containers:
        - name: agent
          securityContext:
            runAsUser: 0
            # If using Red Hat OpenShift uncomment this:
            #privileged: true
          volumeMounts:
          - name: varlogcontainers
            mountPath: /var/log/containers
          - name: varlogpods
            mountPath: /var/log/pods
          - name: varlibdockercontainers
            mountPath: /var/lib/docker/containers
          env:
            - name: NODE_NAME
              valueFrom:
                fieldRef:
                  fieldPath: spec.nodeName
        volumes:
        - name: varlogcontainers
          hostPath:
            path: /var/log/containers
        - name: varlogpods
          hostPath:
            path: /var/log/pods
        - name: varlibdockercontainers
          hostPath:
            path: /var/lib/docker/containers
  config:
    id: 2d70a6f0-33a5-11eb-bb2f-418d0388a8cf
    revision: 2
    agent:
      monitoring:
        enabled: true
        use_output: default
        logs: true
        metrics: true
    inputs:
      - id: d8b4d520-615b-11eb-847e-6bd07fa09e69
        name: log-1
        revision: 1
        type: logfile
        use_output: default
        meta:
          package:
            name: log
            version: 0.4.6
        data_stream:
          namespace: default
        streams:
          - id: logfile-log.log
            data_stream:
              dataset: generic
            type: container
            paths:
               - /var/log/containers/*.log
            symlinks: true
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: agent
rules:
- apiGroups: [""] # "" indicates the core API group
  resources:
  - namespaces
  - pods
  verbs:
  - get
  - watch
  - list
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: agent
  namespace: default
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: agent
subjects:
- kind: ServiceAccount
  name: agent
  namespace: default
roleRef:
  kind: ClusterRole
  name: agent
  apiGroup: rbac.authorization.k8s.io

Then I add the -add_kubernetes_metadata: attribute:

apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
metadata:
  name: elastic-agent
spec:
  version: 7.10.2
  elasticsearchRefs:
  - name: elasticsearch
  daemonSet:
    podTemplate:
      spec:
        serviceAccountName: agent
        automountServiceAccountToken: true
        terminationGracePeriodSeconds: 30
        dnsPolicy: ClusterFirstWithHostNet
        hostNetwork: true # Allows to provide richer host metadata
        containers:
        - name: agent
          securityContext:
            runAsUser: 0
            # If using Red Hat OpenShift uncomment this:
            #privileged: true
          volumeMounts:
          - name: varlogcontainers
            mountPath: /var/log/containers
          - name: varlogpods
            mountPath: /var/log/pods
          - name: varlibdockercontainers
            mountPath: /var/lib/docker/containers
          env:
            - name: NODE_NAME
              valueFrom:
                fieldRef:
                  fieldPath: spec.nodeName
        volumes:
        - name: varlogcontainers
          hostPath:
            path: /var/log/containers
        - name: varlogpods
          hostPath:
            path: /var/log/pods
        - name: varlibdockercontainers
          hostPath:
            path: /var/lib/docker/containers
  config:
    id: 2d70a6f0-33a5-11eb-bb2f-418d0388a8cf
    revision: 2
    agent:
      monitoring:
        enabled: true
        use_output: default
        logs: true
        metrics: true
    inputs:
      - id: d8b4d520-615b-11eb-847e-6bd07fa09e69
        name: log-1
        revision: 1
        type: logfile
        use_output: default
        meta:
          package:
            name: log
            version: 0.4.6
        data_stream:
          namespace: default
        streams:
          - id: logfile-log.log
            data_stream:
              dataset: generic
            type: container
            paths:
               - /var/log/containers/*.log
            symlinks: true
            processors:
               - add_kubernetes_metadata:

and it reliably panics.

@michalpristas
Copy link
Contributor

thanks this is helpful

@michalpristas
Copy link
Contributor

running this on master and 7.10.2
starting standalone clean and slowly adding stuff.
add_kuberenetes_metadata for metrics then new input per your config without metadata and then i add metadata
everything is fine.

the only difference is that you're using k8s and i'm not.
i will try to install and reproduce but this seems fishy.
it's not visible in the cnofig but do you have output defined because from what i see this config should not work at all in the first place

@pebrc
Copy link
Author

pebrc commented Mar 24, 2021

ECK generates the output configuration. If it helps I can later extract the effective configuration from k8s.

@michalpristas
Copy link
Contributor

it would be helpful as well, thank you

@pebrc
Copy link
Author

pebrc commented Mar 24, 2021

Actually I think this might be partially caused on the ECK end. We use https://github.com/elastic/go-ucfg to render the configuration. And it turns this processor statement into a null value:

agent:
  monitoring:
    enabled: true
    logs: true
    metrics: true
    use_output: default
id: 2d70a6f0-33a5-11eb-bb2f-418d0388a8cf
inputs:
- data_stream:
    namespace: default
  id: d8b4d520-615b-11eb-847e-6bd07fa09e69
  meta:
    package:
      name: log
      version: 0.4.6
  name: log-1
  revision: 1
  streams:
  - data_stream:
      dataset: generic
    id: logfile-log.log
    paths:
    - /var/log/containers/*.log
    processors:
    - null
    symlinks: true
    type: container
  type: logfile
  use_output: default
outputs:
  default:
    hosts:
    - https://elasticsearch-es-http.default.svc:9200
    password: <redacted>
    ssl:
      certificate_authorities:
      - /mnt/elastic-internal/elasticsearch-association/default/elasticsearch/certs/ca.crt
    type: elasticsearch
    username: default-elastic-agent-default-elasticsearch-agent-user
revision: 2

Apologies I should have checked the rendered configuration right away 😞

We are documenting this behaviour for Beats and probably should do the same for Agent.
https://www.elastic.co/guide/en/cloud-on-k8s/master/k8s-beat-troubleshooting.html#k8s-beat-configuration-containing-key-null-is-malformed

However I think it is probably still a reasonable bugfix on the Agent side that even a malformed configuration like this should not let the process crash.

@michalpristas
Copy link
Contributor

this seems like something we can handle definitely, will raise a PR soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Team:Elastic-Agent Label for the Agent team v7.13.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants