[ohai][ec2][looks_like_ec2] Do not depend on metadata endpoint

[gbatye]

For environments where full tunnel VPN is used, requiring a connection to the local metadata endpoint causes node['cloud'] = nil. At a minimum node['cloud']['provided'] attribute should be populated even if metadata can not be fetched.

Signed-off-by: Greg Batye gbatye@gmail.com

Related Issue

#1767

Types of changes

• [ X] Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

• I have read the CONTRIBUTING document.
• I have run the pre-merge tests locally and they pass.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commits have been signed-off for the Developer Certificate of Origin.

[gbatye]

before

C:\Windows> ohai cloud
C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/lib/ohai/system.rb:185:in `attributes_print': I cannot find an attribute named cloud! (ArgumentError)
        from C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/lib/ohai/application.rb:151:in `block in run_application'
        from C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/lib/ohai/application.rb:150:in `each'
        from C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/lib/ohai/application.rb:150:in `run_application'
        from C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/lib/ohai/application.rb:92:in `block in run'
        from C:/opscode/chef/embedded/lib/ruby/3.0.0/benchmark.rb:308:in `realtime'
        from C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/lib/ohai/application.rb:90:in `run'
        from C:/opscode/chef/embedded/lib/ruby/gems/3.0.0/gems/ohai-17.9.0/bin/ohai:25:in `<top (required)>'
        from C:/opscode/chef/bin/ohai:173:in `load'
        from C:/opscode/chef/bin/ohai:173:in `<main>'

after

C:\Windows> ohai cloud
{
 "provider": "ec2"
}

[nishchintraina]

This looks dangerous, EC2 metadata service is pretty standard service and should always be available to any process running on the local network to pull important metadata from.

[jaymzh]

After discussion in the PR review meeting, we believe this is a Good Change(TM). Here's the rationale:

1. All the other tests are REALLY GOOD indicators that we're on EC2, checking if the MD endpoint is not necessary
2. What if I configure my, say, IP space based on is-EC2, then that metadata endpoint goes down and I reconfigure all my VMs into the wrong IP space?
3. There are options in EC2 to not allow your VMs to talk to to the MD endpoint because it can be sensitive

So the change seems good, but tests will fail now, so please update the tests accordingly.

[gbatye]

@jaymzh I believe the current test cover this code change: https://github.com/chef/ohai/blob/main/spec/unit/plugins/ec2_spec.rb#L316

All unit test pass locally

Finished in 14.45 seconds (files took 1.27 seconds to load)
2155 examples, 0 failures, 1 pending

Running RuboCop...
Inspecting 323 files
...................................................................................................................................................................................................................................................................................................................................

323 files inspected, no offenses detected

[marcparadise]

Thanks @gbatye , this looks good. If you can update it correct the lint failure, we'll get it merged.

[jaymzh]

Lint updates requested above.

[kcbraunschweig]

For the record it looks like the socket test was added in:
60dd6c8

Always try the quick to timeout socket "can_connect_metadata?" method if we think we're on EC2. We're often wrong

Not a lot of detail as to why the other tests were often wrong and this is a while ago so kinda hard to prove conclusively if this is more reliable now without the socket test.

[kcbraunschweig]

when you remove can_socket_connect? I think you can remove lines 31 and 34 referring to http_helper

[jaymzh]

@kcbraunschweig 's request to remove the unneeded require's makes sense, can we do that before we merge?

Also, we're missing DCO on some of your committs.

[gbatye]

Signed-off-by: Greg Batye gbatye@gmail.com

[gbatye]

@jaymzh , any pointers on how-to update DCO for all the commits?

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
20.9% Duplication