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.

Sign up for GitHub

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

Jump to bottom

adding needed flags to impersonate #49

Merged
merged 2 commits into from
Oct 5, 2022
Merged

adding needed flags to impersonate #49

merged 2 commits into from
Oct 5, 2022

Conversation

45cali
Copy link
Contributor

@45cali 45cali commented Oct 4, 2022

this pull request provides the needed flags to impersonate when using the get, upsert and remove commands

  • --as
  • --as-user
 go run main.go upsert -h
upsert updates or inserts a user or role to the aws-auth configmap

Usage:
  aws-auth upsert [flags]

Flags:
      --append                    append to a existing group list
      --as string                 Username to impersonate for the operation
      --as-group strings          Group to impersonate for the operation, this flag can be repeated to specify multiple groups
      --groups strings            Groups to upsert
  -h, --help                      help for upsert
      --kubeconfig string         Path to kubeconfig
      --maproles                  Upsert a role
      --mapusers                  Upsert a user
      --retry                     Retry on failure with exponential backoff
      --retry-max-count int       Maximum number of retries before giving up (default 12)
      --retry-max-time duration   Maximum wait interval (default 30s)
      --retry-min-time duration   Minimum wait interval (default 200ms)
      --rolearn string            Role ARN to upsert
      --update-username           set to false to not overwite username (default true)
      --userarn string            User ARN to upsert
      --username string           Username to upsert

@45cali 45cali requested a review from a team as a code owner October 4, 2022 23:58
@codecov
Copy link

codecov bot commented Oct 5, 2022 •
edited
Loading

Codecov Report

Merging #49 (8f1b0ef) into master (4c9d160) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #49   +/-   ##
=======================================
  Coverage   86.00%   86.00%           
=======================================
  Files           5        5           
  Lines         343      343           
=======================================
  Hits          295      295           
  Misses         25       25           
  Partials       23       23
Impacted Files Coverage Δ
pkg/mapper/types.go 79.48% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

eytan-avisror
eytan-avisror reviewed Oct 5, 2022
View reviewed changes
cmd/cli/get.go
@@ -80,4 +85,6 @@ func init() {
rootCmd.AddCommand(getCmd)
getCmd.Flags().StringVar(&getArgs.KubeconfigPath, "kubeconfig", "", "Path to kubeconfig")
getCmd.Flags().StringVar(&getArgs.Format, "format", "table", "The format in which to display results (currently only 'table' supported)")
getCmd.Flags().StringVar(&upsertArgs.AsUser, "as", "", "Username to impersonate for the operation")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering if these flags can be rootCmd flags and carry over to all commands as a result

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had them initially as root but then they show up in version -h if your fine with that more then happy to change it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point - thats fine, lets leave as is then

eytan-avisror
eytan-avisror reviewed Oct 5, 2022
View reviewed changes
cmd/cli/get.go
@@ -80,4 +85,6 @@ func init() {
rootCmd.AddCommand(getCmd)
getCmd.Flags().StringVar(&getArgs.KubeconfigPath, "kubeconfig", "", "Path to kubeconfig")
getCmd.Flags().StringVar(&getArgs.Format, "format", "table", "The format in which to display results (currently only 'table' supported)")
getCmd.Flags().StringVar(&upsertArgs.AsUser, "as", "", "Username to impersonate for the operation")
getCmd.Flags().StringSliceVar(&upsertArgs.AsGroups, "as-group", []string{}, "Group to impersonate for the operation, this flag can be repeated to specify multiple groups")
Copy link
Collaborator

@eytan-avisror eytan-avisror Oct 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Will a comma separated list work in this case as well? if thats the case description can just say "List of groups to impersonate..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i believe so ill write a test to validate. I pulled the wording directly from kubectl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eytan-avisror actually not sure off hand how to write a test for this, there isnt anything in place in the tests where the cli is being called. this works though

 go run main.go upsert --as me --as-group "system:masters" --as-group "sudoers"  --maproles --rolearn arn:aws:iam::xxxxxxxxxxxxxxxx:role/test_team   --append --groups testing --username foo --update-username=false
role arn:aws:iam::xxxxxxxxxxxxxxxx:role/test_team has been updated

eytan-avisror
eytan-avisror reviewed Oct 5, 2022
View reviewed changes
cmd/cli/root.go
@@ -75,6 +80,9 @@ func getKubernetesClient(kubePath string) (kubernetes.Interface, error) {
}
}

config.Impersonate.UserName = options.AsUser
config.Impersonate.Groups = options.AsGroups

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you elaborate more on the use-case for being able to impersonate in the case of modifying aws-auth?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this allows the ability to not have to hardcode --as and --as-group values in the kube config.

@eytan-avisror
Copy link
Collaborator

Thanks @45cali 👍
Added some comments

@eytan-avisror eytan-avisror self-requested a review October 5, 2022 17:05
eytan-avisror
eytan-avisror approved these changes Oct 5, 2022
View reviewed changes
Copy link
Collaborator

@eytan-avisror eytan-avisror left a comment

Choose a reason for hiding this comment

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

LGTM, you need to sign the commits to address the DCO

45cali added 2 commits October 5, 2022 10:12
@45cali
adding needed flags to impersonate
d3cbd89 
Signed-off-by: Nicholas Colbert <[email protected]>
@45cali
updated readme
8f1b0ef 
Signed-off-by: Nicholas Colbert <[email protected]>
@45cali 45cali force-pushed the feature/impersonate branch from 5bcfb25 to 8f1b0ef Compare October 5, 2022 17:15
@eytan-avisror eytan-avisror merged commit 232324b into keikoproj:master Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eytan-avisror eytan-avisror eytan-avisror approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@45cali @eytan-avisror