-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Added AWS X-Ray Support #805
Conversation
This change adds an `--xray-access` CLI flag, that when specified will add an IAM policy to the node group IAM role. The IAM policy allows outbound write access to AWS X-Ray. The specific policy allows access to: ``` xray:PutTraceSegments xray:PutTelemetryRecords xray:GetSamplingRules xray:GetSamplingTargets xray:GetSamplingStatisticSummaries ``` This is useful, as AWS App Mesh (supported with `--appmesh-access` flag) can optionally push metrics into AWS X-Ray. For an example of this see the screenshots [here](https://github.com/PaulMaddox/aws-appmesh-helm). Also see PaulMaddox/aws-appmesh-helm#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PaulMaddox Thanks a lot for your contribution!
Code LGTM. But I think eksctl is generally moving forward with declarative configuration. That said, it will be even nicer if the new flag is removed for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall, but let's avoid adding a new flag unless there is a very strong reason to have it?
No strong reason - was just copying the previous PR for adding App Mesh, and all of the other IAM options had CLI flags for them. I'll remove the CLI flag. |
Was trying to do a build of the project to test out a currently open PR, and didn't have a preexisting Go setup on my machine. Looks like you need dep installed to do the `make install-build-deps` step? Adding a bit to the documentation to save someone else time in the future
Expands the help text for `eksctl completion bash --help` to include the workaround for bash3 which is commonly found on macOSes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This change adds an
--xray-access
CLI flag, that when specified willadd an IAM policy to the node group IAM role. The IAM policy allows
outbound write access to AWS X-Ray.
The specific policy generated allows access to:
This is useful, as AWS App Mesh (supported with
--appmesh-access
flag)can optionally push metrics into AWS X-Ray. For an example of this see
the screenshots here.
I have not updated the README, as it doesn't seem to mention every
--*-access
type flag today (for example the appmesh flag isn't mentioned).Also see PaulMaddox/aws-appmesh-helm#1
Description
Checklist
make build
)make test
)make integration-test
)README.md
, andexamples
directory)humans.txt
file