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

New flags --disable-user-data & --disable-sensitive-metadata #137

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mabartosz
Copy link

userdata and some metadata paths return more than a pod needs and contents can be safely blanked.

This patch under public domain or BSD license as preferred.

@coveralls
Copy link

coveralls commented May 24, 2018

Coverage Status

Coverage remained the same at 19.786% when pulling cd189d0 on mabartosz:security into 80efbb1 on jtblin:master.

README.md Outdated
@@ -515,6 +515,8 @@ Usage of ./build/bin/darwin/kube2iam:
--base-role-arn string Base role ARN
--debug Enable debug features
--default-role string Fallback role to use when annotation is not set
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a 👍 on the PR in general but I am wondering if there are more informative names we could use here to outline their meaning.

Not sure if it's "better" but the following seem a bit more clear about their purpose to me. What do you think?

  • --disable-sensitive-metadata
  • --disable-iam-userdata

Copy link
Collaborator

Choose a reason for hiding this comment

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

These names seem a lot more clear to me at first glance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mabartosz Any thoughts here?

Copy link
Author

Choose a reason for hiding this comment

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

People are alive! Thanks for the suggestion. I will update the PR soon.

@akumria
Copy link

akumria commented Dec 10, 2018

It looks like this just needs to be re-based and have the option updated.

@mabartosz do you have bandwidth for this at the moment ?

@mabartosz mabartosz force-pushed the security branch 2 times, most recently from b497b7d to b5d4d02 Compare March 15, 2019 09:18
@mabartosz mabartosz changed the title New flags --hide-user-data & --extra-security New flags --disable-user-data & --disable-sensitive-metadata Mar 17, 2019
@mabartosz
Copy link
Author

sorry for the delay.

the last version of patch was used in my prod cluster for several months.

@akumria
Copy link

akumria commented Mar 25, 2019

Thanks, we'll test this out and report back in a few days.

@akumria
Copy link

akumria commented May 15, 2019

Just an update, we've now been using this in production without any problems.

It'd be great if this was merged into a released version of kube2iam.

@akumria
Copy link

akumria commented May 16, 2019

@jtblin , @jrnt30 , @mwhittington21 : Anything further you need to get this over the line and merged in?

@jonmoter
Copy link

Howdy! Any word on this getting merged and released??

@fradee
Copy link

fradee commented Apr 21, 2021

@jtblin @mwhittington21 @jrnt30 Please add this feature. I tested on many clusters with custom-built images - and it is working perfectly.

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.

7 participants