Fix custom index name in setup

[cwurm]

beatname setup -E setup.dashboards.index="myname-*" is supposed to create an index pattern in Kibana with a custom index name.

This currently doesn't work (discuss thread) because of an impossible type assertion in the code (x.([]interface{}) will never work if x is of type []common.MapStr which it is).

This fixes the type assertions. I've tested that setup can create custom index patterns.

[andrewkroh]

It looks like this ReplaceIndexInIndexPattern function is used by a few code paths. There is one case where it is used with the internally generated index pattern (this probably contains []common.MapStr) and another case where it operates on data produced directly from a json.Unmarshal (this would have []interface).

So perhaps this needs to handle both cases? Take a look at the callers ReplaceIndexInIndexPattern.

[cwurm]

Ah, you're right. I don't think we usually use the direct file import functionality since #10478, but it's still there. I pushed an update to make both cases work.

[cwurm]

jenkins, test this

[cwurm]

This slipped through the cracks for a while. Rebased and retested, CI is green. @andrewkroh let me know if this looks good now.

[andrewkroh]

You might as well constrain the scope of objectMaps to the if block by declaring as part of the if statement just like it's done in the if else.

[urso]

When dealing with external input and some implicitely assumed types, then we should validate and log if something doesn't match our expectations.

Maybe use this pattern:

  switch objects := content["objects"].(type) {
    case []interface{}:
        ...
    case []common.MapStr;
        ...
    default:
        // log error with type + contents e.g.
        logp.Errf("Unexpected object type %T, expected list of objects. Got: %#v", content["objects"], content["objects"])
  }

This if-else construct seems to normalize the object contents for further processing. Maybe we should error if we can't do so, but the field is present? How about moving the normalization into a separate function?

[cwurm]

Thanks, I've changed to a switch.

I initially tried separating the normalization from the rest of the logic. Unfortunately, it's not just a single type cast. The whole structure is nested, with each nested object being either []interface{} or []common.MapStr. So we'd need to recursively cast everything from one to the other, and maybe that's out of scope for this PR (which is essentially just fixing a regression bug)?

[kvch]

I prefer you rewrite the if-else part of the code to use type switches as @urso mentioned in his comment. It's more idiomatic when multiple types are valid/can be handled.

[cwurm]

@urso @kvch I changed to a switch as suggested and rebased on master.

[cbricedavis]

I've run into this issue today and it doesn't look like there have been any updates since July - is there a way to implement this fix?

[botelastic]

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[cbricedavis]

Still relevant 👍

[kvch]

The issue was fixed by #17749