terraform-provider-rancher2 icon indicating copy to clipboard operation
terraform-provider-rancher2 copied to clipboard

Add rancher2_service_account_token resource

Open sboschman opened this issue 3 years ago • 4 comments
trafficstars

See #315 for different use-cases.

The original proposed solution by rawmind0 was to modify the rancher2_user resource to have it optionally return a token. A preview of that solution is available in #926.

As there is also the request for multiple tokens for a user, and a bunch of user accounts to manage from tf this alternative solution is up for discussion. Reasons are juggling multiple rancher2_user resources linked to user specific rancher2 provider instances (with alias) to generate additional tokens is complicated, and the current tf sdk does not support nested blocks with logical keys, making it hard/impossible to expose multiple tokens from the rancher2_user resource.

Hence a new resource, which takes a username and password to perform a login, to generate a token for an arbitrary user. The existing rancher2_token resource only generates tokens for the provider user.

This solution duplicates the need to provide the username/password from the rancher2_user to the new rancher2_service_account_token resource and in doing so this new resource uses it's own credentials to communicate with the Rancher api, instead of the provider credentials. Even with these drawbacks I think this is the better of the two approaches (#926 vs #932). Less complicated code and more flexibility to use tf constructs to generate many user / service acoounts, each with multiple tokens if desired.

fixes #315 related to #926

sboschman avatar May 19 '22 11:05 sboschman

Thank you for doing this @sboschman let me know if you need anything as I have previous experience on working with development of providers

Shocktrooper avatar May 24 '22 15:05 Shocktrooper

@annablender Can we get a review for this so we can potentially add it to the v2.6.6 milestone?

Shocktrooper avatar Jul 01 '22 01:07 Shocktrooper

Bump, this feature would be very useful!

JWackerbauer avatar Aug 22 '22 14:08 JWackerbauer

Is there anything I can do to get a review going @annablender or @HarrisonWAffel ?

If there are different ideas on how to solve the issues described in #315 with Rancher + Terraform than please let us know.

sboschman avatar Oct 19 '22 05:10 sboschman

In general, I agree with the approach and good job on the implementation and error handling. There may be security concerns around being able to create a new user with a custom username/password to access the Rancher API for a specific cluster id via terraform so I'd discuss this with my team before moving forward. @HarrisonWAffel if you have any other ideas for the approach here let me know

In general, if this is approved we will also need

  • Test instructions (make testacc? How did you run the tests and do they work?)
  • Test provisioning an RKE/K3s cluster on the latest Rancher v2.7-head with a custom non-default user and verify that it works for the requested RFE https://github.com/rancher/terraform-provider-rancher2/issues/315

a-blender avatar Dec 12 '22 22:12 a-blender

There may be security concerns around being able to create a new user with a custom username/password to access the Rancher API for a specific cluster id via terraform so I'd discuss this with my team before moving forward.

My 2c, the provider itself requires an admin token, so a new user with token created with terraform has at most the same admin permissions as the provider. Which does not grant you any more permissions than you already have. In practise we want to be able to create users+tokens with less permissions.

sboschman avatar Dec 13 '22 09:12 sboschman

@sboschman Yep ok, I discussed with my team as well and have adjusted/responded to my comments. I saw there's been a new force push, let me know when it is ready for re-review. As a note: please add commits for any additional changes. It is easier to see what has changed for reviewers. Don't worry about additional commits. Those can be squashed into one when we merge into the provider

a-blender avatar Dec 13 '22 17:12 a-blender

outside of the other changes requested by Anna this lgtm, i can reapprove once the other changes have been made

HarrisonWAffel avatar Dec 13 '22 18:12 HarrisonWAffel

Changes made in response to the review:

  1. rebased on current master

  2. Add [ERROR] to all fmt.Errorf object strings returned from a function

    rancher2/resource_rancher2_service_account_token.go: line 120

    rancher2/resource_rancher2_service_account_token_test.go: line 113, line 117, line 121 and line 140

  3. Add Description: to password, username, temp token, and temp token id fields

    rancher2/schema_service_account_token.go: username desc, password desc, temp_token and temp_token_id

sboschman avatar Dec 13 '22 19:12 sboschman