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

Introduce add family of commands for company IAM #140

Merged
merged 9 commits into from
Jan 4, 2024

Conversation

JGiola
Copy link
Member

@JGiola JGiola commented Jan 3, 2024

What this PR is for?

This PR will introduce the new add family of command for company iam.

Those command are for:

  • add a new user in a company with a given role
  • create a new group in a company with a given role
  • create a service account in a company with a given role
  • add one or more user to an existing group in a company

@JGiola JGiola requested a review from a team as a code owner January 3, 2024 09:28
@JGiola JGiola requested a review from davidebianchi January 3, 2024 09:28
@@ -210,15 +223,17 @@ Available flags for the command:
- `--insecure-skip-tls-verify`, to disallow the check the validity of the certificate of the remote endpoint
- `--context`, to specify a different context from the currently selected one
- `--company-id`, to set the ID of the desired Company
- `--output`, optional flag to save the service account json description in a file at the provided path
Copy link
Member

Choose a reason for hiding this comment

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

Description? Are saved the credentials?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've changed the docs to better reflect what we are saving

}

options.CreateNewGroupFlags(cmd.Flags())
err := cmd.RegisterFlagCompletionFunc("role", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
Copy link
Member

Choose a reason for hiding this comment

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

Those roles should be dynamic? There is a role api, which could be used to dynamically configure this completion. What do you think?
In the future, probably the roles will change

Copy link
Member Author

Choose a reason for hiding this comment

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

I will postpone the feature until a proper caching mechanism is in place for the various calls, because making an http call for every autocomplete try is not the best experience...

}

func createNewGroup(ctx context.Context, client *client.APIClient, companyID, groupName string, role resources.ServiceAccountRole) error {
if !resources.IsValidServiceAccountRole(role) {
Copy link
Member

Choose a reason for hiding this comment

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

Also this check is static

}

options.AddNewUserFlags(cmd.Flags())
err := cmd.RegisterFlagCompletionFunc("role", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
Copy link
Member

Choose a reason for hiding this comment

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

Also here, static completion

Copy link
Member

@davidebianchi davidebianchi left a comment

Choose a reason for hiding this comment

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

LGTM

@davidebianchi davidebianchi merged commit a7dc817 into main Jan 4, 2024
9 checks passed
@davidebianchi davidebianchi deleted the feat/add-iam-commands branch January 4, 2024 14:32
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