Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct ecs environment variable name #26

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

windmgc
Copy link
Collaborator

@windmgc windmgc commented Sep 22, 2022

Summary

I accidentally caught this by testing the lib in the ECS environment, logs show that

2022/09/22 08:19:18 [debug] 1#0: [lua] utils.lua:180: detect_region(): no AWS_REGION env variable
2022/09/22 08:19:18 [debug] 1#0: [lua] utils.lua:188: detect_region(): no AWS_DEFAULT_REGION env variable
2022/09/22 08:19:18 [debug] 1#0: [lua] utils.lua:201: detect_region(): no ECS_CONTAINERMETADATA_URI_V4 env variable
2022/09/22 08:19:18 [debug] 1#0: [lua] utils.lua:214: detect_region(): no ECS_CONTAINERMETADATA_URI env variable

The environment variable name for the ECS metadata URI seems to be wrong.

It should be ECS_CONTAINER_METADATA_URI and ECS_CONTAINER_METADATA_URI_V4, according to https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-metadata-endpoint-v4.html#task-metadata-endpoint-v4-paths and https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-metadata-endpoint-v3.html#task-metadata-endpoint-v3-paths

To keep all the references in the same format I also changed these variable names.

Changelog

  • Correct ECS metadata URI environment variable name
  • Update doc

@CLAassistant
Copy link

CLAassistant commented Sep 22, 2022

CLA assistant check
All committers have signed the CLA.

@Tieske
Copy link
Member

Tieske commented Sep 22, 2022

can you please remove the documentation updates? the docs shouldn't be rendered on PR's because they go live immeditely after merging and then the docs are newer than the latest release. So only on a release render the docs. See instructions here: https://github.com/Kong/lua-resty-aws#history

@windmgc
Copy link
Collaborator Author

windmgc commented Sep 23, 2022

@Tieske got it, I've dropped the docs changes, please take a look again, thanks!

Copy link
Member

@Tieske Tieske left a comment

Choose a reason for hiding this comment

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

Thx! good catch.

@Tieske Tieske merged commit 5a3b7f9 into Kong:main Sep 26, 2022
@Tieske
Copy link
Member

Tieske commented Sep 26, 2022

version 0.5.5 is now up, including this fix.

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.

None yet

3 participants