Add metadata field to ES role model #7927
Conversation
| Cluster []string `json:"cluster,omitempty"` | ||
| Indices []IndexRole `json:"indices,omitempty"` | ||
| Applications []ApplicationRole `json:"applications,omitempty"` | ||
| Metadata map[string]interface{} `json:"metadata,omitempty"` |
There was a problem hiding this comment.
This seems reasonable. The original PR that implemented this:
had Metadata as a slightly different field. What was the intent of the previous design @thbkrkr (since you made the change).
Looking back over v6, v7, and v8 of Elasticsearch, this metadata field has existed in the same form object, so I see no backwards compatibility issues with this change.
There was a problem hiding this comment.
AFAIR, there was no real intention, it was unnecessary premature anticipation. I had left in comments, because not used, different fields seen in an ES response that we could potentially have used one day. We removed them in #2777.
I think the small change of this PR is ok. Nit: we could use the any alias which is more pleasant to read:
Metadata map[string]any `json:"metadata,omitempty"`There was a problem hiding this comment.
Thanks both. I pushed the readability improvement
|
buildkite test this -f p=gke,E2E_TAGS=es -m s=7.17.8,s=8.14.1,s=8.15.0-SNAPSHOT |
|
buildkite test this -f p=gke,E2E_TAGS=es -m s=7.17.8,s=8.14.1,s=8.15.0-SNAPSHOT |
|
@thbkrkr thanks for the review! Could you merge this once CI is green? Turns out I don't have permissions to merge. |
barkbay
added
the
exclude-from-release-notes
Elasticsearch roles also include a
metadatafield (docs).This PR updates the
Rolemodel accordingly.Relates: ES-8909