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

[aoc-collector non-root user]: Update vended config #2298

Closed
wants to merge 1 commit into from

Conversation

PaurushGarg
Copy link
Member

Description:

Update vended config for awscontainerinsightreceiver to run Collector with privilidged root-access
Dependent on PR: #2297

Link to tracking Issue:
#2222

Testing:

Documentation:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@PaurushGarg PaurushGarg requested a review from a team as a code owner August 25, 2023 16:34
Copy link
Contributor

@bryan-aguilar bryan-aguilar left a comment

Choose a reason for hiding this comment

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

This PR contains changes other than the vended config. It should be updated to only reflect changes to the vended config

@@ -60,6 +60,7 @@ Resources:
ContainerDefinitions:
- Name: aws-collector
Image: 'public.ecr.aws/aws-observability/aws-otel-collector:latest'
Privileged: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Use user field instead of priveleged to set to the root user.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have reverted the changes made to this deployment template as it didn't require root user access. This template is only for instance metrics and not for container/task level metrics and hence don't require root access.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this template used? Does the config that is passed as a parameter always contain container insights receiver?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have reverted the changes made to this deployment template as it didn't require root user access. This template is only for instance metrics and not for container/task level metrics and hence don't require root access.

I think this is a an example template referenced on the ADOT website here. The config of awscontainerinsightreceiver is passed as default but it can be changed through parameters.

@PaurushGarg
Copy link
Member Author

This PR contains changes other than the vended config. It should be updated to only reflect changes to the vended config

I have removed the changes made to ECS deployment template.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

This PR is stale because it has been open 30 days with no activity.

@PaurushGarg PaurushGarg force-pushed the non-root-user-update-vended-config branch from a03d253 to dea7575 Compare October 20, 2023 23:07
Removing changes to ECS deployment template
@PaurushGarg PaurushGarg force-pushed the non-root-user-update-vended-config branch from dea7575 to 2555c64 Compare October 20, 2023 23:45
@PaurushGarg
Copy link
Member Author

Closing this PR as this change has been added to this PR: #2301

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.

2 participants