[AUTHN-66] iam provider and pool commands by kevin-wu24 · Pull Request #1303 · confluentinc/cli

Kevin Wu (kevin-wu24) · 2022-06-07T18:22:21Z

Checklist

[CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?
- no

What

Implemented the CRUD sub commands for iam identity providers and iam identity pools, both of which are placed behind the cli.identity-provider feature flag

References

Test & Review

I wrote an integration test for each sub command in the iam_tests.go file which can be run with: make int-test INT_TEST_ARGS="-run TestCLI/TestIAM" and unit tests were not needed because the code is just hitting endpoints.

…eature flag

Brian Strauch (brianstrauch)

Looks great! Super clean! 🎉

internal/cmd/iam/command.go

internal/cmd/iam/command_provider.go

internal/cmd/iam/command_provider_create.go

internal/pkg/cmd/prerunner.go

internal/pkg/ccloudv2/identity-provider.go

internal/pkg/errors/error_message.go

test/test-server/ccloud_handlers.go

Brian Strauch (brianstrauch)

Looking good 🚀

internal/cmd/iam/command.go

internal/cmd/iam/command_pool.go

internal/cmd/iam/command_pool_create.go

internal/pkg/ccloudv2/identity-provider.go

internal/pkg/errors/strings.go

internal/pkg/resource/resource.go

test/test-server/iam_handlers.go

internal/cmd/iam/command_pool_create.go

internal/cmd/iam/command_pool_describe.go

internal/cmd/iam/command_pool_update.go

internal/pkg/errors/error_message.go

internal/pkg/resource/resource.go

Brian Strauch (brianstrauch) · 2022-07-06T21:10:18Z

internal/cmd/iam/command.go

 package iam

 import (
+	"github.com/confluentinc/cli/internal/pkg/featureflags"


Sort your imports!

internal/cmd/iam/command_pool.go

internal/cmd/iam/command_pool_create.go

internal/cmd/iam/command_pool_delete.go

internal/cmd/iam/command_pool_list.go

internal/cmd/iam/command_pool_update.go

internal/cmd/iam/command_provider_create.go

Brian Strauch (brianstrauch) · 2022-07-06T21:20:31Z

internal/cmd/iam/command_provider_delete.go

+		RunE:              c.delete,
+		Example: examples.BuildExampleString(
+			examples.Example{
+				Text: `Delete identity provider op-12345.`,


Suggested change

Text: `Delete identity provider op-12345.`,

Text: `Delete identity provider "op-12345".`,

Same comment

internal/pkg/errors/catcher.go

go.mod

Arvind Thirunarayanan (arvindth)

Couple of observations:

When I run this command:

> XX_LAUNCH_DARKLY_TEST_ENV=true dist/confluent_darwin_amd64/confluent iam pool get pool-q0yR --provider op-Jn
Error: unknown flag: --provider
Usage: ...

It gives me an unknown flag --provider error. I would have expected unknown command get instead.

When you receive a 400 bad request, it would be helpful to also print out the error message to help diagnose the issue. eg: Deleting a provider that still has some pools gives a response of {"error_code":400,"message":"OpenID provider op-Pj at Org 5f93392c-f9bd-4bef-8523-2da64591e058 has 2 active identity pools associated."} but this message is not printed out by the cli.

internal/cmd/iam/command_pool_create.go

internal/cmd/iam/command_provider_update.go

Arvind Thirunarayanan (arvindth) · 2022-07-14T22:32:31Z

internal/cmd/iam/command_pool_create.go

+		IdentityClaim: *resp.SubjectClaim,
+		Filter:        *resp.Policy,


We'll be updating the api, and then the sdk soon to return identityClaim and filter. We'll keep the cli team posted when that happens, and you can update this to match at that time.

internal/cmd/iam/command_pool_update.go

test/iam_test.go

internal/cmd/iam/command_pool_create.go

test/fixtures/output/iam/identity-provider/delete-dne.golden

Arvind Thirunarayanan (arvindth) · 2022-07-23T00:15:15Z

Kevin Wu (@kevin-wu24) looks like the iam rbac role-binding list --role commands don't work when the principal is a pool id. eg:

XX_LAUNCH_DARKLY_TEST_ENV=true dist/confluent_darwin_amd64/confluent iam rbac role-binding list --principal User:pool-AB4O
    Principal    | Email |       Role        | Environment | Cloud Cluster | Cluster Type | Logical Cluster | Resource Type | Name | Pattern Type
-----------------+-------+-------------------+-------------+---------------+--------------+-----------------+---------------+------+---------------
  User:pool-AB4O |       | OrganizationAdmin |             |               |              |                 |               |      |

but

XX_LAUNCH_DARKLY_TEST_ENV=true dist/confluent_darwin_amd64/confluent iam rbac role-binding list --role OrganizationAdmin
    Principal    |                   Email                    |  Service Name
-----------------+--------------------------------------------+------------------
  User:sa-3wknoo |                                            | OAuthTestSvcAct
  User:sa-w5vny9 |                                            | testprivesc1
  User:u-lgzo0m  | athirunar+rbac+devel+orgadmin@confluent.io |

I see a call to /users being made when I run with -vvv. I think this call may be being made to get additional details about the user such as email/name. Since the pool id isn't part of the /users response, I think it's getting dropped.

Arvind Thirunarayanan (arvindth)

Hey Kevin Wu (@kevin-wu24) had a couple of follow comments. Otherwise, functionality-wise it lgtm.

internal/cmd/iam/command_pool_update.go

internal/cmd/iam/command_provider_update.go

go.sum

internal/cmd/iam/command_pool_create.go

Brian Strauch (brianstrauch) · 2022-07-27T23:56:50Z

internal/cmd/iam/command_pool_delete.go

+		RunE:              c.delete,
+		Example: examples.BuildExampleString(
+			examples.Example{
+				Text: `Delete identity pool pool-12345.`,


I don't see this change. Did you forget to commit?

internal/cmd/iam/command_pool_delete.go

internal/pkg/errors/error_message.go

internal/pkg/resource/resource.go

test/test-server/ccloudv2_router.go

test/test-server/iam_handlers.go

internal/cmd/iam/command_rbac_role_binding_test.go

test/fixtures/output/iam/rbac/role-binding/list-user-orgadmin-cloud.golden

implemented the 5 indentity provider commands and put them behind a f…

af8e181

…eature flag

Kevin Wu (kevin-wu24) requested a review from a team as a code owner June 7, 2022 18:22

Kevin Wu (kevin-wu24) added 3 commits June 7, 2022 11:27

fixed lint error

ebd97ec

removed commented code from commit

4fab6f5

fixed bug in command.go

de90a82

Brian Strauch (brianstrauch) reviewed Jun 7, 2022

View reviewed changes

Kevin Wu (kevin-wu24) added 5 commits June 7, 2022 14:15

code review fixes

847beb2

code review fixes pt2

dc29c7d

fixed some bugs caused by changes in error_message.go

c8b873f

implemented and tested identity pool commands

24eed22

fixed typo in internal/pkg/ccloudv2/client.go

9f605c7

Brian Strauch (brianstrauch) reviewed Jun 8, 2022

View reviewed changes

Kevin Wu (kevin-wu24) added 3 commits June 8, 2022 15:18

code review fixes

992d2ec

refactoring provider update and adding policy for identity pool commands

877a42f

Merge branch 'main' of github.com:confluentinc/cli into AUTHN-66

158f4b0

Brian Strauch (brianstrauch) reviewed Jun 9, 2022

View reviewed changes

code review fixes

2604197

Kevin Wu (kevin-wu24) changed the title ~~iam provider commands~~ [AUTHN-66] iam provider and pool commands Jun 13, 2022

Kevin Wu (kevin-wu24) added 2 commits July 6, 2022 13:30

updating pool create doc

4d766fc

merged with main to grab Muwei's PR

64c91f6

Kevin Wu (kevin-wu24) requested a review from Brian Strauch (brianstrauch) July 6, 2022 20:57

Brian Strauch (brianstrauch) reviewed Jul 6, 2022

View reviewed changes

Kevin Wu (kevin-wu24) added 3 commits July 6, 2022 15:18

code review fixes

6adba2b

sorting imports

fda4d54

adding forgotten code review changes

aed3f67

Kevin Wu (kevin-wu24) requested a review from Brian Strauch (brianstrauch) July 7, 2022 21:01

removing some redundant code

f2c7c7d

Brian Strauch (brianstrauch) reviewed Jul 11, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

Arvind Thirunarayanan (arvindth) reviewed Jul 14, 2022

View reviewed changes

code review fixes from oauth team

cf81ebf

Arvind Thirunarayanan (arvindth) reviewed Jul 14, 2022

View reviewed changes

internal/cmd/iam/command_pool_update.go Outdated Show resolved Hide resolved

Arvind Thirunarayanan (arvindth) reviewed Jul 14, 2022

View reviewed changes

test/iam_test.go Show resolved Hide resolved

Kevin Wu (kevin-wu24) added 2 commits July 14, 2022 15:49

adding errors for update no-ops

761405e

refactored provider command CLI output to match UI in stag

258b84f

Brian Strauch (brianstrauch) reviewed Jul 18, 2022

View reviewed changes

internal/cmd/iam/command_pool_create.go Outdated Show resolved Hide resolved

internal/cmd/iam/command_pool_create.go Outdated Show resolved Hide resolved

test/fixtures/output/iam/identity-provider/delete-dne.golden Outdated Show resolved Hide resolved

Kevin Wu (kevin-wu24) added 3 commits July 18, 2022 13:05

code review fixes

05c49bb

Merge branch 'main' into AUTHN-66

6dbf52c

provider and user delete int-tests updated

d3553cc

Brian Strauch (brianstrauch) mentioned this pull request Jul 20, 2022

Add Stream Sharing commands for provider shares list, describe and delete #1367

Merged

Kevin Wu (kevin-wu24) added 3 commits July 26, 2022 21:30

renaming pool fields after SDK update

ec4f6dd

fixed rbac list bug not showing identity pools

f391a2b

adding identity pool support for ACL commands

841f5a9

Arvind Thirunarayanan (arvindth) reviewed Jul 27, 2022

View reviewed changes

internal/cmd/iam/command_pool_update.go Outdated Show resolved Hide resolved

internal/cmd/iam/command_provider_update.go Outdated Show resolved Hide resolved

Brian Strauch (brianstrauch) reviewed Jul 28, 2022

View reviewed changes

Kevin Wu (kevin-wu24) added 3 commits July 28, 2022 12:06

code review changes, prepping for release

ff08dd8

Merge branch 'main' into AUTHN-66

173d7e6

merged with main and moving to public sdk

b12f65c

Brian Strauch (brianstrauch) reviewed Jul 28, 2022

View reviewed changes

internal/cmd/iam/command_rbac_role_binding_test.go Outdated Show resolved Hide resolved

internal/cmd/iam/command_rbac_role_binding_test.go Outdated Show resolved Hide resolved

test/fixtures/output/iam/rbac/role-binding/list-user-orgadmin-cloud.golden Outdated Show resolved Hide resolved

code review fixes

5109a8b

Brian Strauch (brianstrauch) approved these changes Jul 29, 2022

View reviewed changes

Brian Strauch (brianstrauch) merged commit eeee83e into main Jul 29, 2022

Brian Strauch (brianstrauch) deleted the AUTHN-66 branch July 29, 2022 00:39

	Text: `Delete identity provider op-12345.`,
	Text: `Delete identity provider "op-12345".`,

Conversation

Kevin Wu (kevin-wu24) commented Jun 7, 2022 • edited by arvindth Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

What

References

Test & Review

Uh oh!

Brian Strauch (brianstrauch) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Brian Strauch (brianstrauch) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Brian Strauch (brianstrauch) Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Brian Strauch (brianstrauch) Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

Brian Strauch (brianstrauch) Jul 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Arvind Thirunarayanan (arvindth) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Arvind Thirunarayanan (arvindth) Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Arvind Thirunarayanan (arvindth) commented Jul 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Arvind Thirunarayanan (arvindth) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Kevin Wu (kevin-wu24) commented Jun 7, 2022 •

edited by arvindth

Loading

Arvind Thirunarayanan (arvindth) commented Jul 23, 2022 •

edited

Loading