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

ecs: add iptables rules for ECS introspection server #2267

Conversation

mchaker
Copy link
Contributor

@mchaker mchaker commented Jul 5, 2022

Issue number:

Closes #2195

Description of changes:

Adds iptables rules that forward traffic destined for the ECS Introspection port on the Docker Bridge interface address to the ECS Introspection port on the localhost interface (where the ECS Introspection Server is listening).

Testing done:

  • tested on Bottlerocket aws-ecs-1 variant in an ECS cluster, which consisted of:
    1. Run an ECS Task on a cluster consisting of Bottlerocket nodes (task was: a fedora image that sat and waited)
    2. Attempt to curl the API endpoint (172.17.0.1:51678) over HTTP from within that fedora container (ECS Task)

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@mchaker mchaker changed the title Add iptables rules for ecs introspection server and some comments Add iptables rules for ECS introspection server Jul 5, 2022
@mchaker mchaker marked this pull request as ready for review July 6, 2022 00:17
Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

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

Can you elaborate on the testing you've done to specifically test these changes? I'm not too familiar with the ECS agent introspection API. Looking at the issue, can you try verifying you can access the introspection API endpoint in a running ECS task?

ExecStartPre=/sbin/iptables -t filter -I INPUT --dst 127.0.0.0/8 ! \
--src 127.0.0.0/8 -m conntrack ! --ctstate RELATED,ESTABLISHED,DNAT -j DROP
ExecStart=/usr/bin/amazon-ecs-agent
# Grant access to metadata server
Copy link
Contributor

Choose a reason for hiding this comment

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

These ExecStopPost directives are deleting the iptable rules right? Would this be more accurate?:

Suggested change
# Grant access to metadata server
# Remove access to IMDS from ECS tasks

Same for the new comments below

@@ -13,17 +13,29 @@ RestartSec=1s
EnvironmentFile=-/etc/ecs/ecs.config
EnvironmentFile=/etc/network/proxy.env
Environment=ECS_CHECKPOINT=true
# Grant access to metadata server
Copy link
Contributor

Choose a reason for hiding this comment

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

"metadata server" is somewhat ambiguous. Also might be good specify what we're granting access to.
Would this be accurate?

Suggested change
# Grant access to metadata server
# Grant ECS tasks access to IMDS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed with @arnaldo2792 and I agree - clarifying the "metadata server" is a good idea. :)

@mchaker
Copy link
Contributor Author

mchaker commented Jul 6, 2022

Can you elaborate on the testing you've done to specifically test these changes? I'm not too familiar with the ECS agent introspection API. Looking at the issue, can you try verifying you can access the introspection API endpoint in a running ECS task?

@etungsten Sure, I'll add further details in the PR description, but essentially:

  1. Ran ECS task which was just a fedora container
  2. Attempted to curl the API endpoint (172.17.0.1:51678) over HTTP

@mchaker mchaker added the area/ecs ECS label Jul 6, 2022
@mchaker mchaker force-pushed the mchaker-2022-07-ecs-introspection-access branch from b06ebf6 to 45f8494 Compare July 6, 2022 20:49
@mchaker
Copy link
Contributor Author

mchaker commented Jul 6, 2022

Latest force push: respond to PR suggestions (fix comment clarity)

@mchaker mchaker force-pushed the mchaker-2022-07-ecs-introspection-access branch from 45f8494 to 697d227 Compare July 6, 2022 20:59
@mchaker
Copy link
Contributor Author

mchaker commented Jul 6, 2022

Latest force push: replace IMDS with proper name

@mchaker mchaker requested a review from bcressey July 6, 2022 23:33
@mchaker mchaker changed the title Add iptables rules for ECS introspection server ECS agent: add iptables rules for ECS introspection server Jul 7, 2022
@mchaker mchaker force-pushed the mchaker-2022-07-ecs-introspection-access branch from 697d227 to cf61982 Compare July 7, 2022 17:56
@mchaker
Copy link
Contributor Author

mchaker commented Jul 7, 2022

Latest force push: fix commit message

@bcressey
Copy link
Contributor

bcressey commented Jul 8, 2022

The code change looks fine to me as long as the Datadog agent is working on the ECS variant now.

Can you fix the commit subject to align to conventions?

ecs: add iptables rules for introspection server

@mchaker
Copy link
Contributor Author

mchaker commented Jul 8, 2022

The code change looks fine to me as long as the Datadog agent is working on the ECS variant now.

Can you fix the commit subject to align to conventions?

ecs: add iptables rules for introspection server

@bcressey: @arnaldo2792 and I just finished testing the Datadog agent. We confirmed that docker labels get translated to Datadog tags using the following environment variable in the Datadog agent Task configuration:

{
      "name": "DD_DOCKER_LABELS_AS_TAGS",
      "value": "'{\"com.amazonaws.ecs.task-definition-family\": \"task_name_test\", \"com.amazonaws.ecs.task-definition-version\": \"task_version_test\"}'"
}

The tags appeared for a Task scoped to a Bottlerocket host containing my changes, and did not appear for a Task scoped to a Bottlerocket host without my changes.

@mchaker mchaker changed the title ECS agent: add iptables rules for ECS introspection server ecs: add iptables rules for ECS introspection server Jul 8, 2022
Enables ECS Tasks to query the ECS Introspection Server.
@mchaker mchaker force-pushed the mchaker-2022-07-ecs-introspection-access branch from cf61982 to 0075507 Compare July 8, 2022 01:43
@mchaker
Copy link
Contributor Author

mchaker commented Jul 8, 2022

Latest force push: fix commit message to align to convention

@mchaker mchaker merged commit 84d074d into bottlerocket-os:develop Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ECS tasks don't have access to introspection server
4 participants