ec2_info breaks on empty endpoint

[choopooly]

By default events/maintenance/ is empty which makes _get_ec2_hostinfo to fail with the following error:

IndexError: string index out of range

[pdecat]

Noticed the same here with the recent 2018-08-17 version of EC2 metadata that introduces events:

admin@ip-10-205-0-21:~$ diff <(curl -s http://169.254.169.254/2018-03-28/meta-data/) <(curl -s http://169.254.169.254/2018-08-17/meta-data/)
4a5
> events/
admin@ip-10-205-0-21:~$ curl -s http://169.254.169.254/latest/meta-data/events/
maintenance/
admin@ip-10-205-0-21:~$ curl http://169.254.169.254/latest/meta-data/events/maintenance/
admin@ip-10-205-0-21:~$

I came up with quite about the same resolution:

# diff /srv/salt/_grains/ec2_info.py /var/cache/salt/minion/extmods/grains/ec2_info.py
30a31,32
>         if not line:
>           continue

[kdecoteauCS]

I was in the process of adding this as a PR:
if path == "events/maintenance/": continue
We have an open ticket with AWS to determine whether the change in metadata is related only to maintenance or everything. I like the "not line" approach, but I'd like to get the official word about whether it's JUST maintenance which is doing it.

It looks like they're not done with these changes though. Maintenance events aren't actually getting into the metadata service yet.

[kdecoteauCS]

I like. len(line) ==0 is the best approach. We can always test for a variable being missing. This covers the current problem well and potentially avoids other issues with metadata drift.

[blueyed]

Please consider merging this already as-is (without waiting for feedback from AWS etc), given the impact of this.

[beiriannydd]

There is no line. It isn’t that there is a blank line which I think would break the protocol since each line means something. When you split nothing you get an array with a single empty string. My solution was to wrap everything so that an empty response returns {}

[blueyed]

Yeah, changing for line in resp_data.split("\n"): into the following might be better:

lines = resp_data.split("\n")
if lines:
    for line in lines:

[mchugh19]

Just noticed this and submitted a PR before seeing this one. I think the fix in #201 is a little bit more direct and clean. It also matches the now upstream behavior from https://github.com/saltstack/salt/blob/develop/salt/grains/metadata.py#L61

[blueyed]

@mchugh19
I agree, using endswith looks to me also.

[beiriannydd]

It seems that if implemented that way it would try to get a value for a non existent key. Surely the right way is to return an empty set if the result is empty?

[pnickerson-cashstar]

Here is AWS's response on the ticket that @kdecoteauCS mentioned:

On September 4th EC2 started rolling out a new instance metadata namespace. On September 6th EC2 was made aware that the new namespace was causing impact to some customers that were using a third-party Saltstack script. Customers who relied on the third-party script would have experienced an error affecting their workflows when the script attempted to retrieve information from the instance metadata. The regions that received the new namespace were US-EAST-1, US-WEST-2, EU-WEST-1 and AP-SOUTHEAST-2. As mitigation EC2 has stopped rolling out the new namespace into any other regions. The root cause of the issue is that the third-party script fails to handle an empty namespace entry in the instance metadata. The issue can be fixed by updating the script to correctly handle an empty namespace. In addition to that EC2 has started rolling out a change that will accommodate the usage of the existing third-party script without breaking it.

[blueyed]

Thanks.

TLDR:

In addition to that EC2 has started rolling out a change that will accommodate the usage of the existing third-party script without breaking it.