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

adding needed flags to impersonate #49

Merged
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

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

@@ -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

@@ -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

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

@@ -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
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

Signed-off-by: Nicholas Colbert <ncolbert@goodrx.com>
Signed-off-by: Nicholas Colbert <ncolbert@goodrx.com>
@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants