aws-samples / amazon-cloudwatch-container-insights

CloudWatch Agent Dockerfile and K8s YAML templates for CloudWatch Container Insights.
MIT No Attribution
162 stars 106 forks source link

IMDSv2 should be default and version should be configurable #131

Open schollii opened 1 year ago

schollii commented 1 year ago

Following the docs on AWS, I end up with a fluent-bit that uses IMDSv1. Whereas the recommendation by AWS is to only use IMDSv2.

So the configmap fluent-bit-cluster-info should have an extra field to choose v1 vs v2. Also the default should be v2, not v1. However it doesn't seem like a default is possible, the user would have to choose one or the other. A helm chart would be so much easier. Any chance for that?

BTW https://github.com/aws/aws-for-fluent-bit/issues/177 discusses the issue I am having. In case others hit this issue, if you shell into one of the fluent-bit containers and run /fluent-bit/bin/fluent-bit --version, you will I get fluent-bit version, but this is not the same as the docker image version, which you can find in /AWS_FOR_FLUENT_BIT_VERSION file in the docker image:

bash-4.2# ls /
AWS_FOR_FLUENT_BIT_VERSION  boot  ecs        etc         home  lib64  media  opt   root  sbin  sys  usr
bin             dev   entrypoint.sh  fluent-bit  lib   local  mnt    proc  run   srv   tmp  var
bash-4.2# more AWS_FOR_FLUENT_BIT_VERSION
2.28.5
bash-4.2# 
SaxyPandaBear commented 1 year ago

I don't have a lot of context on the aws-for-fluent-bit container. Does that have support for a feature toggle to use IMDSv1 vs v2? This repo doesn't really influence that decision making.

schollii commented 1 year ago

In K8s-deployment-manifest-templates/deployment-mode/daemonset/container-insights-monitoring/fluent-bit/fluent-bit-compatible.yaml line 224 and k8s-deployment-manifest-templates/deployment-mode/daemonset/container-insights-monitoring/fluent-bit/fluent-bit-compatible.yaml line 176

the value "v1" is hardcoded. The default should be "v2" and parameterized so one can change it to "v1" if really necessary.

I can give that a shot. Will you consider a PR?