Skip to content

Conversation

mirelap-amazon
Copy link
Contributor

@mirelap-amazon mirelap-amazon commented Mar 24, 2021

Issue #, if available:

Description of changes:

Testing:
I tested one unit test on a specific host and here are the logs:

pytest -vv -o log_cli=true test/unit/agent_metadata/test_aws_ec2_instance.py

DEBUG    codeguru_profiler_agent.agent_metadata.fleet_info:fleet_info.py:20 Making a request to http://169.254.169.254/latest/api/token with headers set for these keys: dict_keys(['X-aws-ec2-metadata-token-ttl-seconds'])
DEBUG    codeguru_profiler_agent.agent_metadata.fleet_info:fleet_info.py:20 Making a request to http://169.254.169.254/latest/meta-data/local-hostname with headers set for these keys: dict_keys(['X-aws-ec2-metadata-token'])
DEBUG    codeguru_profiler_agent.agent_metadata.fleet_info:fleet_info.py:20 Making a request to http://169.254.169.254/latest/meta-data/instance-type with headers set for these keys: dict_keys(['X-aws-ec2-metadata-token'])
PASSED                                                                                                             [ 33%]
test/unit/agent_metadata/test_aws_ec2_instance.py::TestAWSEC2Instance::TestLookUpMetadata::TestWhenHostNameCannotBeDetermined::test_it_returns_none 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mirelap-amazon mirelap-amazon requested review from gimki and a team March 24, 2021 16:29
@mirelap-amazon mirelap-amazon self-assigned this Mar 24, 2021
httpretty.register_uri(
httpretty.GET,
EC2_API_TOKEN_URI,
body="PARIOq_FXbIyL0maE9RcmrsyWtylvFh1ZDt0NrRUyNxeV1-DlpFpA==")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure you need to provide a real token here, it may trigger some security scripts that search the code for those. Could you just put a fake string here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not real, it's based on a real one, but then I updates random characters from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I agree it still can trigger some security scripts, I will update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updated it, thanks.

Comment on lines 54 to 56
def __look_up_with_IMDSv2(cls, url):
return http_get(url=url,
headers={EC2_METADATA_TOKEN_HEADER_KEY: cls.__look_up_ec2_api_token()}) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we call both __look_up_instance_type and __look_up_host_name successively, we are going to get 2 different tokens, can you maybe update the look_up_metadata function to get a token once and reuse it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, updated; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants