Skip to content
This repository has been archived by the owner on Nov 8, 2021. It is now read-only.

Add support for EC2 Tags #74

Merged
merged 3 commits into from
Jul 28, 2017
Merged

Conversation

sistemi-etime
Copy link
Contributor

These changes add support for EC2 Tags.
The authorised groups are retrieved by a request to a specified key, saved on IAM_AUTHORIZED_GROUPS_TAG.
In this way, the befitting groups can be set from EC2 console/API, without edit the configuration in local instance.

Copy link
Contributor

@michaelwittig michaelwittig left a comment

Choose a reason for hiding this comment

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

Hi @sistemi-etime
I like the idea to fetch the configuration from the instance tags!
I added a few comments regarding documentation.

README.md Outdated
@@ -37,21 +37,6 @@ A picture is worth a thousand words:

## How to integrate this system into your environment

### Install via RPM
Copy link
Contributor

Choose a reason for hiding this comment

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

any reasons why you removed the rpm block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Sorry for mistake.

Copy link
Contributor

Choose a reason for hiding this comment

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

no worries. can you re add the text block in a new commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Done.

README.md Outdated
@@ -94,6 +80,8 @@ one or more of the following lines:
ASSUMEROLE="IAM-role-arn" # IAM Role ARN for multi account. See below for more info
IAM_AUTHORIZED_GROUPS="GROUPNAMES" # Comma seperated list of IAM groups to import
SUDOERS_GROUPS="GROUPNAMES" # Comma seperated list of IAM groups that should have sudo access
IAM_AUTHORIZED_GROUPS_TAG="KeyTag" # Key Tag of EC2 that contains a Comma seperated list of IAM groups to import
Copy link
Contributor

Choose a reason for hiding this comment

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

if I understand this correctly, that IAM_AUTHORIZED_GROUPS_TAG will override IAM_AUTHORIZED_GROUPS, so I can only use one of the config values. right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. Could you add this to the documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Done.

README.md Outdated
@@ -94,6 +80,8 @@ one or more of the following lines:
ASSUMEROLE="IAM-role-arn" # IAM Role ARN for multi account. See below for more info
IAM_AUTHORIZED_GROUPS="GROUPNAMES" # Comma seperated list of IAM groups to import
SUDOERS_GROUPS="GROUPNAMES" # Comma seperated list of IAM groups that should have sudo access
IAM_AUTHORIZED_GROUPS_TAG="KeyTag" # Key Tag of EC2 that contains a Comma seperated list of IAM groups to import
SUDOERS_GROUPS_TAG="KeyTag" # Key Tag of EC2 that contains a Comma seperated list of IAM groups that should have sudo access
Copy link
Contributor

Choose a reason for hiding this comment

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

if I understand this correctly, that SUDOERS_GROUPS_TAG will override SUDOERS_GROUPS, so I can only use one of the config values. right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. Could you add this to the documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Done.

@michaelwittig michaelwittig merged commit 97bae15 into widdix:master Jul 28, 2017
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