Some template alias settings are not persisted on PUT template

[davidkyle]

Templates that use aliases do not persist all of the alias options. The fix is to copy over those settings to the alias metadata

The bug is easy to recreate

PUT _template/with_alias
{
  "index_patterns": [
    "my-prefix-*"
  ],
  "mappings": {
    "properties": {
      "timestamp": {
        "type": "date"
      }
    }
  },
  "aliases": {
    "my-read": {
      "is_hidden" : true
    }
  }
}

Then when the template is read back the is_hidden field has mysteriously disappeared.

GET _template/with_alias



{
  "with_alias" : {
    "order" : 0,
    "index_patterns" : [
      "my-prefix-*"
    ],
    "settings" : { },
    "mappings" : {
      "properties" : {
        "timestamp" : {
          "type" : "date"
        }
      }
    },
    "aliases" : {
      "my-read" : { }.  <-- somethings changed here
    }
  }
}

[elasticmachine]

Pinging @elastic/es-core-features (:Core/Features/Features)

[davidkyle]

run elasticsearch-ci/packaging-sample-windows

[davidkyle]

pinging @elastic/es-core-features I've got a small change here for review

[dakrone]

Thanks for opening this @davidkyle, I left a comment about a change I think we should do in this PR, but otherwise this looks good.

[dakrone]

I actually don't think we should persist this one. Putting a template in place with is_write_index: true will be confusing because only a single index can be marked as the write index. I think we should instead throw an exception if alias.writeIndex() is set to true here (fail early rather than fail late)

[davidkyle]

Makes perfect sense. I added a validation against the alias having is_write_index: true and updated the tests

[davidkyle]

The build failures are all a result of the new validation that the template alias cannot be a write alias.

is_write_index: true is used in the tests but those tests cannot have been testing the behaviour of that setting because, as we now know, the field was never persisted.

@dakrone I will defer to you on how to proceed with this change. Not persisting the is_hidden setting is the bug I was having trouble with.

[2020-10-08T16:10:20,774][INFO ][o.e.t.r.ClientYamlTestSuiteIT] [test] [p0=indices.put_index_template/15_composition/Component and index template composition] before test	
[2020-10-08T16:10:20,806][INFO ][o.e.t.r.ClientYamlTestSuiteIT] [test] Stash dump on test failure [{	
  "stash" : {	
    "body" : {	
      "error" : {	
        "root_cause" : [	
          {	
            "type" : "illegal_argument_exception",	
            "reason" : "alias [aliasname] cannot be a write alias",

https://gradle-enterprise.elastic.co/s/kwfgkm34j7avy/tests/failed

[dakrone]

Okay, I looked into what it would take to fix the tests, I think that gets a little bit more out of scope for this PR.

I think it's reasonable to persist all the settings for this PR, and then I can open a follow-up issue where we disallow the write index alias setting (fixing the tests in a separate PR).

Do you want to revert/undo the change that adds the exception throwing and we can merge this? (I'll open up the follow-up issue and make sure our team discusses the repercussions)

[davidkyle]

Thanks @dakrone I reverted the validate write alias change.

[dakrone]

LGTM, thanks for iterating on this!