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

feat(iam-role-for-service-accounts-eks): Introduce adot collector #397

Closed
wants to merge 3 commits into from
Closed

feat(iam-role-for-service-accounts-eks): Introduce adot collector #397

wants to merge 3 commits into from

Conversation

levivannoort
Copy link

@levivannoort levivannoort commented Jun 27, 2023

Description

This pull request introduces the option to create an IRSA for the ADOT collector. ADOT - AWS Distro for Open Telemetry: "with AWS Distro for OpenTelemetry, you can instrument your applications just once to send correlated metrics and traces to multiple monitoring solutions."

Motivation and Context

With the introduction of the of AWS Distro for Open Telemetry as an installable EKS add-on, it would be nice to be able to create an IRSA, which could for example be used instead of it being inherited from the node.

These three policies are the policies needed for each service we want to export to. If you only plan on using 1 or 2 of the services, you only need to attach the policies for that service:

arn:aws:iam::aws:policy/AmazonPrometheusRemoteWriteAccess grants write access to the Prometheus service.
arn:aws:iam::aws:policy/AWSXrayWriteOnlyAccess grants write access to the AWS X-Ray service.
arn:aws:iam::aws:policy/CloudWatchAgentServerPolicy grants access to write the CloudWatch service.

As described in the documentation it does support various use-cases and the option to only attach specific policies has been introduced to the created IRSA module.

Note
The policy attachment is based on the amazon managed policies, as these are recommended to be used within the ADOT documentation, I didn't see another IRSA use the managed policies so please let me know if this is on purpose and why.

Breaking Changes

No breaking changes, since this is an add-on to the current submodule.

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
module.adot_collector_irsa_role.aws_iam_role.this[0]: Creating...
module.adot_collector_irsa_role.aws_iam_role.this[0]: Creation complete after 1s [id=adot-collector-v2]
module.adot_collector_irsa_role.aws_iam_role_policy_attachment.adot_collector_cloudwatch_agent[0]: Creating...
module.adot_collector_irsa_role.aws_iam_role_policy_attachment.adot_collector_prometheus[0]: Creating...
module.adot_collector_irsa_role.aws_iam_role_policy_attachment.adot_collector_xray[0]: Creating...
module.adot_collector_irsa_role.aws_iam_role_policy_attachment.adot_collector_xray[0]: Creation complete after 1s [id=adot-collector-v2-20230627154023422600000001]
module.adot_collector_irsa_role.aws_iam_role_policy_attachment.adot_collector_cloudwatch_agent[0]: Creation complete after 1s [id=adot-collector-v2-20230627154023569800000002]
module.adot_collector_irsa_role.aws_iam_role_policy_attachment.adot_collector_prometheus[0]: Creation complete after 1s [id=adot-collector-v2-20230627154023707200000003]

Apply complete! Resources: 4 added, 0 changed, 0 destroyed.
  • I have executed pre-commit run -a on my pull request

@levivannoort levivannoort changed the title feat(iam-role-for-service-accounts-eks): introduce adot collector feat(iam-role-for-service-accounts-eks): Introduce adot collector Jun 27, 2023
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

I don't see the difference between this and simply passing the policies like this

role_policy_arns = {
AmazonEKS_CNI_Policy = "arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy"
additional = aws_iam_policy.additional.arn
}

What is the benefit of this approach?

@levivannoort
Copy link
Author

I don't see the difference between this and simply passing the policies like this

role_policy_arns = {
AmazonEKS_CNI_Policy = "arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy"
additional = aws_iam_policy.additional.arn
}

What is the benefit of this approach?

Thanks for responding, there are no benefits. I completely missed the option to be able to pass the policy_arns in that way. I'll close the PR.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants