Skip to content

Commit

Permalink
feat: allow Service Account updates to Account Role (#82)
Browse files Browse the repository at this point in the history
* feat: allow Service Account updates to Account Role

* remove computed, bc it can change

* fix
  • Loading branch information
parkedwards authored Oct 30, 2023
1 parent 5199e17 commit 96a9869
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 20 deletions.
3 changes: 2 additions & 1 deletion internal/api/service_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ type ServiceAccountCreateRequest struct {
}

type ServiceAccountUpdateRequest struct {
Name string `json:"name"`
Name string `json:"name"`
AccountRoleID *uuid.UUID `json:"account_role_id,omitempty"`
}

type ServiceAccountRotateKeyRequest struct {
Expand Down
63 changes: 51 additions & 12 deletions internal/provider/resources/service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
Expand Down Expand Up @@ -115,6 +116,7 @@ func (r *ServiceAccountResource) Schema(_ context.Context, _ resource.SchemaRequ
Optional: true,
Computed: true,
Description: "Account Role name of the service account",
Default: stringdefault.StaticString("Member"),
Validators: []validator.String{
stringvalidator.OneOf("Admin", "Member"),
},
Expand Down Expand Up @@ -284,8 +286,10 @@ func (r *ServiceAccountResource) Read(ctx context.Context, req resource.ReadRequ
// Update updates the resource and sets the updated Terraform state on success.
func (r *ServiceAccountResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
var plan ServiceAccountResourceModel

resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)

var state ServiceAccountResourceModel
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
if resp.Diagnostics.HasError() {
return
}
Expand All @@ -297,10 +301,56 @@ func (r *ServiceAccountResource) Update(ctx context.Context, req resource.Update
return
}

// Here, we'll retrieve the API Key from the previous State, as it's
// not included in the Terraform Plan / user configuration, nor is it
// returned on any API response other than Create and RotateKey.
// This means that we'll want to grab + persist the value in State
// if no rotation takes place. If a rotation does take place, we'll
// update this variable with the new API Key value returned from that call,
// and persist that into State.
// Note that using the stringplanmodifier.UseStateForUnknown() helper for
// this attribute will not work in this case, as the Provider will throw an
// error upon key rotation, as the value coming back will be different with
// the previous value held in State.
apiKey := state.APIKey.ValueString()

updateReq := api.ServiceAccountUpdateRequest{
Name: plan.Name.ValueString(),
}

// Check if the Account Role Name was specifically changed in the plan,
// so we can only perform the Account Role lookup if we need to.
accountRoleName := state.AccountRoleName.ValueString()
if accountRoleName != plan.AccountRoleName.ValueString() {
accountRoleClient, err := r.client.AccountRoles(plan.AccountID.ValueUUID())
if err != nil {
resp.Diagnostics.Append(helpers.CreateClientErrorDiagnostic("Account Role", err))

return
}

accountRoles, err := accountRoleClient.List(ctx, []string{plan.AccountRoleName.ValueString()})
if err != nil {
resp.Diagnostics.AddError(
"Error fetching Account Role",
fmt.Sprintf("Could not fetch Account Role, unexpected error: %s", err),
)

return
}

if len(accountRoles) != 1 {
resp.Diagnostics.AddError(
"Could not find Account Role",
fmt.Sprintf("Could not find Account Role with name %s", plan.AccountRoleName.String()),
)

return
}

updateReq.AccountRoleID = &accountRoles[0].ID
}

// Update client method requires context, botID, request args
err = client.Update(ctx, plan.ID.ValueString(), updateReq)
if err != nil {
Expand All @@ -322,17 +372,6 @@ func (r *ServiceAccountResource) Update(ctx context.Context, req resource.Update
return
}

// Here, we'll retrieve the API Key from the previous State, as it's
// not included in the Terraform Plan / user configuration, nor is it
// returned on any API response other than Create and RotateKey.
// Additionally, the Provider framework will throw an exception if we
// set the `api_key` property to use stringmodifier.UseStateForUnknown()
// during legitimate cases where the API Key State value will be updated
// during key rotation.
var state ServiceAccountResourceModel
req.State.Get(ctx, &state)
apiKey := state.APIKey.ValueString()

// Practitioners can rotate their Service Account API Key my modifying the
// `api_key_expiration` attribute. If the provided value is different than the current
// value, we'll call the RotateKey method on the client, which returns the
Expand Down
30 changes: 23 additions & 7 deletions internal/provider/resources/service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ resource "prefect_service_account" "bot" {
}`, name, expiration.Format(time.RFC3339))
}

func fixtureAccServiceAccountResourceAccountRole(name string) string {
func fixtureAccServiceAccountResourceAccountRole(name string, roleName string) string {
return fmt.Sprintf(`
data "prefect_account_role" "admin" {
name = "Admin"
data "prefect_account_role" "role" {
name = "%s"
}
resource "prefect_service_account" "bot2" {
name = "%s"
account_role_name = "Admin"
}`, name)
account_role_name = "%s"
}`, roleName, name, roleName)
}

//nolint:paralleltest // we use the resource.ParallelTest helper instead
Expand Down Expand Up @@ -102,11 +102,27 @@ func TestAccResource_service_account(t *testing.T) {
),
},
{
Config: fixtureAccServiceAccountResourceAccountRole(randomName2),
Config: fixtureAccServiceAccountResourceAccountRole(randomName2, "Admin"),
Check: resource.ComposeTestCheckFunc(
// @TODO: This is a superficial test, until we can pull in the provider client
// and actually test the API call to Prefect Cloud
resource.TestCheckResourceAttrPair(resourceName2, "account_role_name", "data.prefect_account_role.admin", "name"),
resource.TestCheckResourceAttrPair(resourceName2, "account_role_name", "data.prefect_account_role.role", "name"),
),
},
// @TODO: These two tests are superficial, until we can pull in the provider client
// and actually test the API call to Prefect Cloud
{
Config: fixtureAccServiceAccountResourceAccountRole(randomName2, "Admin"),
// Ensure that setting the account role name successfully applies
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName2, "account_role_name", "data.prefect_account_role.role", "name"),
),
},
{
// Ensure that updating the account role name successfully applies
Config: fixtureAccServiceAccountResourceAccountRole(randomName2, "Member"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName2, "account_role_name", "data.prefect_account_role.role", "name"),
),
},
},
Expand Down

0 comments on commit 96a9869

Please sign in to comment.