Preserve order of project members even if API re-orders them (#484)

* Preserve order of project members even if API re-orders them

* Adjust role field description

* Fix backwards compatibility of deprecated owner_email field

* Fix typo
This commit is contained in:
João Palet 2024-07-30 13:32:49 +01:00 committed by GitHub
parent 63b07c4422
commit 8dc894cacc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 174 additions and 38 deletions

View file

@ -43,5 +43,5 @@ data "stackit_resourcemanager_project" "example" {
Read-Only:
- `role` (String) The role of the member in the project. At least one user must have the `owner` role. Legacy roles (`project.admin`, `project.auditor`, `project.member`, `project.owner`) are not supported.
- `role` (String) The role of the member in the project. Legacy roles (`project.admin`, `project.auditor`, `project.member`, `project.owner`) are not supported.
- `subject` (String) Unique identifier of the user, service account or client. This is usually the email address for users or service accounts, and the name in case of clients.

View file

@ -50,5 +50,5 @@ resource "stackit_resourcemanager_project" "example" {
Required:
- `role` (String) The role of the member in the project. Possible values include, but are not limited to: `owner`, `editor`, `reader`. At least one user must have the `owner` role. Legacy roles (`project.admin`, `project.auditor`, `project.member`, `project.owner`) are not supported.
- `role` (String) The role of the member in the project. Possible values include, but are not limited to: `owner`, `editor`, `reader`. Legacy roles (`project.admin`, `project.auditor`, `project.member`, `project.owner`) are not supported.
- `subject` (String) Unique identifier of the user, service account or client. This is usually the email address for users or service accounts, and the name in case of clients.

View file

@ -112,7 +112,7 @@ func (d *projectDataSource) Schema(_ context.Context, _ datasource.SchemaRequest
"owner_email": "Email address of the owner of the project. This value is only considered during creation. Changing it afterwards will have no effect.",
"owner_email_deprecation_message": "The \"owner_email\" field has been deprecated in favor of the \"members\" field. Please use the \"members\" field to assign the owner role to a user, by setting the \"role\" field to `owner`.",
"members": "The members assigned to the project. At least one subject needs to be a user, and not a client or service account.",
"members.role": fmt.Sprintf("The role of the member in the project. At least one user must have the `owner` role. Legacy roles (%s) are not supported.", strings.Join(utils.QuoteValues(utils.LegacyProjectRoles), ", ")),
"members.role": fmt.Sprintf("The role of the member in the project. Legacy roles (%s) are not supported.", strings.Join(utils.QuoteValues(utils.LegacyProjectRoles), ", ")),
"members.subject": "Unique identifier of the user, service account or client. This is usually the email address for users or service accounts, and the name in case of clients.",
}
@ -246,7 +246,7 @@ func (d *projectDataSource) Read(ctx context.Context, req datasource.ReadRequest
return
}
err = mapMembersFields(membersResp.Members, &model)
err = mapMembersFields(ctx, membersResp.Members, &model)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading project", fmt.Sprintf("Processing API response: %v", err))
return

View file

@ -157,7 +157,7 @@ func (r *projectResource) Schema(_ context.Context, _ resource.SchemaRequest, re
"owner_email": "Email address of the owner of the project. This value is only considered during creation. Changing it afterwards will have no effect.",
"owner_email_deprecation_message": "The \"owner_email\" field has been deprecated in favor of the \"members\" field. Please use the \"members\" field to assign the owner role to a user, by setting the \"role\" field to `owner`.",
"members": "The members assigned to the project. At least one subject needs to be a user, and not a client or service account.",
"members.role": fmt.Sprintf("The role of the member in the project. Possible values include, but are not limited to: `owner`, `editor`, `reader`. At least one user must have the `owner` role. Legacy roles (%s) are not supported.", strings.Join(utils.QuoteValues(utils.LegacyProjectRoles), ", ")),
"members.role": fmt.Sprintf("The role of the member in the project. Possible values include, but are not limited to: `owner`, `editor`, `reader`. Legacy roles (%s) are not supported.", strings.Join(utils.QuoteValues(utils.LegacyProjectRoles), ", ")),
"members.subject": "Unique identifier of the user, service account or client. This is usually the email address for users or service accounts, and the name in case of clients.",
}
@ -316,7 +316,7 @@ func (r *projectResource) Create(ctx context.Context, req resource.CreateRequest
return
}
err = mapMembersFields(membersResp.Members, &model)
err = mapMembersFields(ctx, membersResp.Members, &model)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating project", fmt.Sprintf("Processing API payload: %v", err))
return
@ -364,7 +364,7 @@ func (r *projectResource) Read(ctx context.Context, req resource.ReadRequest, re
return
}
err = mapMembersFields(membersResp.Members, &model)
err = mapMembersFields(ctx, membersResp.Members, &model)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading project", fmt.Sprintf("Processing API response: %v", err))
return
@ -428,7 +428,7 @@ func (r *projectResource) Update(ctx context.Context, req resource.UpdateRequest
return
}
err = mapMembersFields(members, &model)
err = mapMembersFields(ctx, members, &model)
if err != nil {
core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating project", fmt.Sprintf("Processing API response: %v", err))
return
@ -561,7 +561,7 @@ func mapProjectFields(ctx context.Context, projectResp *resourcemanager.GetProje
return nil
}
func mapMembersFields(members *[]authorization.Member, model *Model) error {
func mapMembersFields(ctx context.Context, members *[]authorization.Member, model *Model) error {
if members == nil {
model.Members = types.ListNull(types.ObjectType{AttrTypes: memberTypes})
return nil
@ -574,14 +574,42 @@ func mapMembersFields(members *[]authorization.Member, model *Model) error {
return nil
}
membersList := []attr.Value{}
for i, m := range *members {
modelMembers := []member{}
if !(model.Members.IsNull() || model.Members.IsUnknown()) {
diags := model.Members.ElementsAs(ctx, &modelMembers, false)
if diags.HasError() {
return fmt.Errorf("processing members: %w", core.DiagsToError(diags))
}
}
modelMemberIds := make([]string, len(modelMembers))
for i, m := range modelMembers {
modelMemberIds[i] = memberId(authorization.Member{
Role: m.Role.ValueStringPointer(),
Subject: m.Subject.ValueStringPointer(),
})
}
apiMemberIds := []string{}
for _, m := range *members {
if utils.IsLegacyProjectRole(*m.Role) {
continue
}
apiMemberIds = append(apiMemberIds, memberId(m))
}
reconciledMembersIds := utils.ReconcileStringSlices(modelMemberIds, apiMemberIds)
membersList := []attr.Value{}
for i, m := range reconciledMembersIds {
role := roleFromId(m)
subject := subjectFromId(m)
if role == "" || subject == "" {
return fmt.Errorf("reconcile list of members")
}
membersMap := map[string]attr.Value{
"subject": types.StringPointerValue(m.Subject),
"role": types.StringPointerValue(m.Role),
"subject": types.StringValue(subject),
"role": types.StringValue(role),
}
memberTF, diags := types.ObjectValue(memberTypes, membersMap)
@ -609,7 +637,16 @@ func toMembersPayload(ctx context.Context, model *Model) (*[]authorization.Membe
return nil, fmt.Errorf("nil model")
}
if model.Members.IsNull() || model.Members.IsUnknown() {
return &[]authorization.Member{}, nil
if model.OwnerEmail.IsNull() {
return nil, fmt.Errorf("members and owner_email are both null or unknown")
}
return &[]authorization.Member{
{
Subject: model.OwnerEmail.ValueStringPointer(),
Role: sdkUtils.Ptr(projectOwnerRole),
},
}, nil
}
membersModel := []member{}
@ -618,19 +655,12 @@ func toMembersPayload(ctx context.Context, model *Model) (*[]authorization.Membe
return nil, core.DiagsToError(diags)
}
members := []authorization.Member{}
// If the new "members" fields is set, it has precedence over the deprecated "owner_email" field
if !model.Members.IsNull() && !model.Members.IsUnknown() {
for _, m := range membersModel {
members = append(members, authorization.Member{
Role: m.Role.ValueStringPointer(),
Subject: m.Subject.ValueStringPointer(),
})
}
} else {
members := []authorization.Member{}
for _, m := range membersModel {
members = append(members, authorization.Member{
Subject: model.OwnerEmail.ValueStringPointer(),
Role: sdkUtils.Ptr(projectOwnerRole),
Role: m.Role.ValueStringPointer(),
Subject: m.Subject.ValueStringPointer(),
})
}
@ -791,3 +821,21 @@ func updateMembers(ctx context.Context, projectId string, modelMembers *[]author
func memberId(member authorization.Member) string {
return fmt.Sprintf("%s,%s", *member.Subject, *member.Role)
}
// Extract the role from the member ID representation
func roleFromId(id string) string {
parts := strings.Split(id, ",")
if len(parts) != 2 {
return ""
}
return parts[1]
}
// Extract the subject from the member ID representation
func subjectFromId(id string) string {
parts := strings.Split(id, ",")
if len(parts) != 2 {
return ""
}
return parts[0]
}

View file

@ -12,6 +12,7 @@ import (
"github.com/gorilla/mux"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/stackitcloud/stackit-sdk-go/core/config"
"github.com/stackitcloud/stackit-sdk-go/core/utils"
"github.com/stackitcloud/stackit-sdk-go/services/authorization"
@ -165,6 +166,7 @@ func TestMapProjectFields(t *testing.T) {
func TestMapMembersFields(t *testing.T) {
tests := []struct {
description string
configMembers basetypes.ListValue
membersResp *[]authorization.Member
expected Model
expectedLabels *map[string]string
@ -172,6 +174,7 @@ func TestMapMembersFields(t *testing.T) {
}{
{
"default_ok",
types.ListNull(types.ObjectType{AttrTypes: memberTypes}),
&[]authorization.Member{
{
Subject: utils.Ptr("owner_email"),
@ -209,8 +212,64 @@ func TestMapMembersFields(t *testing.T) {
nil,
true,
},
{
"default_ok (preserve model order)",
types.ListValueMust(types.ObjectType{AttrTypes: memberTypes}, []attr.Value{
types.ObjectValueMust(
memberTypes,
map[string]attr.Value{
"subject": types.StringValue("reader_email"),
"role": types.StringValue("reader"),
},
),
types.ObjectValueMust(
memberTypes,
map[string]attr.Value{
"subject": types.StringValue("owner_email"),
"role": types.StringValue("owner"),
},
),
}),
&[]authorization.Member{
{
Subject: utils.Ptr("owner_email"),
Role: utils.Ptr("owner"),
},
{
Subject: utils.Ptr("reader_email"),
Role: utils.Ptr("reader"),
},
},
Model{
Id: types.StringNull(),
ProjectId: types.StringNull(),
ContainerId: types.StringNull(),
ContainerParentId: types.StringNull(),
Name: types.StringNull(),
Labels: types.MapNull(types.StringType),
Members: types.ListValueMust(types.ObjectType{AttrTypes: memberTypes}, []attr.Value{
types.ObjectValueMust(
memberTypes,
map[string]attr.Value{
"subject": types.StringValue("reader_email"),
"role": types.StringValue("reader"),
},
),
types.ObjectValueMust(
memberTypes,
map[string]attr.Value{
"subject": types.StringValue("owner_email"),
"role": types.StringValue("owner"),
},
),
}),
},
nil,
true,
},
{
"empty members",
types.ListNull(types.ObjectType{AttrTypes: memberTypes}),
&[]authorization.Member{},
Model{
Id: types.StringNull(),
@ -226,6 +285,7 @@ func TestMapMembersFields(t *testing.T) {
},
{
"nil members",
types.ListNull(types.ObjectType{AttrTypes: memberTypes}),
nil,
Model{
Id: types.StringNull(),
@ -250,7 +310,10 @@ func TestMapMembersFields(t *testing.T) {
Name: types.StringNull(),
Labels: types.MapNull(types.StringType),
}
err := mapMembersFields(tt.membersResp, state)
if !tt.configMembers.IsNull() {
state.Members = tt.configMembers
}
err := mapMembersFields(context.Background(), tt.membersResp, state)
if !tt.isValid && err == nil {
t.Fatalf("Should have failed")
}
@ -275,18 +338,6 @@ func TestToCreatePayload(t *testing.T) {
expected *resourcemanager.CreateProjectPayload
isValid bool
}{
{
"default_ok",
&Model{},
nil,
&resourcemanager.CreateProjectPayload{
ContainerParentId: nil,
Labels: nil,
Members: nil,
Name: nil,
},
true,
},
{
"mapping_with_conversions_single_member",
&Model{
@ -404,6 +455,43 @@ func TestToCreatePayload(t *testing.T) {
},
true,
},
{
"deprecated owner_email field still works",
&Model{
ContainerParentId: types.StringValue("pid"),
Name: types.StringValue("name"),
OwnerEmail: types.StringValue("some_email_deprecated"),
},
&map[string]string{
"label1": "1",
"label2": "2",
},
&resourcemanager.CreateProjectPayload{
ContainerParentId: utils.Ptr("pid"),
Labels: &map[string]string{
"label1": "1",
"label2": "2",
},
Members: &[]resourcemanager.Member{
{
Subject: utils.Ptr("some_email_deprecated"),
Role: utils.Ptr("owner"),
},
},
Name: utils.Ptr("name"),
},
true,
},
{
"no members or owner_email fails",
&Model{
ContainerParentId: types.StringValue("pid"),
Name: types.StringValue("name"),
},
&map[string]string{},
nil,
false,
},
{
"nil_model",
nil,