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

Feature/aws profile auth #139

Merged
merged 4 commits into from
Aug 23, 2020
Merged

Feature/aws profile auth #139

merged 4 commits into from
Aug 23, 2020

Conversation

kaplanelad
Copy link
Contributor

What type of PR is this?
/Feature

What this PR does / why we need it:
Allow authenticate to aws with multiple profiles

Which issue(s) this PR fixes (if exists):

Fixes #80

Special notes for your reviewer:
I have reordered the AWS auth logic

@kaplanelad kaplanelad added kind/new-feature New feature area/collector Improvements or additions to collector labels Aug 19, 2020
@kaplanelad kaplanelad requested a review from cregev August 19, 2020 18:40
@kaplanelad kaplanelad self-assigned this Aug 19, 2020
@kaplanelad kaplanelad added this to In progress in v0.4.0 via automation Aug 19, 2020
@kaplanelad kaplanelad removed this from In progress in v0.4.0 Aug 19, 2020
collector/aws/auth.go Show resolved Hide resolved
collector/aws/auth.go Show resolved Hide resolved
log.WithField("region", region).Info("auth: using aws profile")
config := &awsClient.Config{
Region: &region,
Credentials: credentials.NewSharedCredentials("", profile),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you give the option to pass custom Profile file ?

@@ -12,6 +12,7 @@ providers:
# access_key: <access_key>
# secret_key: <secret_key>
# role:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we want to order the types of login like the code expects them ( hierarchy)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

@@ -32,6 +32,7 @@ const (

// DetectorManager describe tje detector manager
type DetectorManager struct {
awsAuth AuthDescriptor
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

cloudWatchCLient := cloudwatch.NewCloudWatchManager(awsCloudwatch.New(regionSession, regionConfig))

callerIdentityOutput, _ := stsManager.client.GetCallerIdentity(&sts.GetCallerIdentityInput{})
return &DetectorManager{
awsAuth: awsAuth,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need it ? I don't see any reference for using this type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@kaplanelad kaplanelad requested a review from cregev August 23, 2020 09:47
@cregev cregev added the lgtm label Aug 23, 2020
@kaplanelad kaplanelad merged commit 4c8c7dd into master Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/collector Improvements or additions to collector kind/new-feature New feature lgtm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Organization mode" to scan all AWS accounts in organization
2 participants