Skip to content

Commit 12d967f

Browse files
steve-thousandsteve-thousand
committed
progress on improving account_member
1 parent 9d1f3ba commit 12d967f

File tree

6 files changed

+264
-98
lines changed

6 files changed

+264
-98
lines changed

‎internal/services/account_member/custom.go‎

Lines changed: 103 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,80 @@ package account_member
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67

78
"github.com/cloudflare/terraform-provider-cloudflare/internal/apijson"
89
"github.com/cloudflare/terraform-provider-cloudflare/internal/customfield"
910
"github.com/hashicorp/terraform-plugin-framework/types"
1011
)
1112

13+
type ConfiguredPermissionType int
14+
15+
const (
16+
Unknown ConfiguredPermissionType = iota
17+
Policies
18+
Roles
19+
)
20+
21+
// Given a config value, determine which permission type is configured. This
22+
// should only be called with the config value, never the plan or state value.
23+
func checkConfiguredPermissionType(configuredModel *AccountMemberModel) ConfiguredPermissionType {
24+
permissionType := Unknown
25+
26+
if !configuredModel.Roles.IsNull() && len(configuredModel.Roles.Elements()) > 0 {
27+
permissionType = Roles
28+
}
29+
if !configuredModel.Policies.IsNull() && len(configuredModel.Policies.Elements()) > 0 {
30+
if permissionType == Roles {
31+
// if both are found, return Unknown
32+
return Unknown
33+
}
34+
return Policies
35+
}
36+
return permissionType
37+
}
38+
1239
func (m AccountMemberModel) marshalCustom() (data []byte, err error) {
1340
return apijson.MarshalRoot(m)
1441
}
1542

16-
func (m AccountMemberModel) marshalCustomForUpdate(state AccountMemberModel) (data []byte, err error) {
43+
func (m AccountMemberModel) marshalCustomForUpdate(state AccountMemberModel, configuredPermissionType ConfiguredPermissionType) (data []byte, err error) {
1744
data, err = apijson.MarshalForUpdate(m, state)
1845
if err != nil {
1946
return
2047
}
2148

22-
if m.Roles == nil || len(*m.Roles) == 0 {
23-
return
24-
}
25-
2649
var payload map[string]interface{}
2750
if err = json.Unmarshal(data, &payload); err != nil {
2851
return
2952
}
3053

31-
// Transform roles: ["role_id"] -> [{"id": "role_id"}]
32-
roleObjects := make([]map[string]interface{}, 0, len(*m.Roles))
33-
for _, role := range *m.Roles {
34-
if !role.IsNull() && !role.IsUnknown() {
35-
roleObjects = append(roleObjects, map[string]interface{}{
36-
"id": role.ValueString(),
37-
})
54+
if configuredPermissionType == Roles {
55+
delete(payload, "policies")
56+
57+
roles := m.Roles.Elements()
58+
59+
// Transform roles: ["role_id"] -> [{"id": "role_id"}]
60+
roleObjects := make([]map[string]interface{}, 0, len(roles))
61+
for _, role := range roles {
62+
if !role.IsNull() && !role.IsUnknown() {
63+
roleString, ok := role.(types.String)
64+
if !ok {
65+
return nil, fmt.Errorf("unexpected role type: %T", role)
66+
}
67+
roleObjects = append(roleObjects, map[string]interface{}{
68+
"id": roleString.ValueString(),
69+
})
70+
}
71+
}
72+
73+
if len(roleObjects) > 0 {
74+
payload["roles"] = roleObjects
3875
}
3976
}
4077

41-
if len(roleObjects) > 0 {
42-
payload["roles"] = roleObjects
78+
if configuredPermissionType == Policies {
79+
delete(payload, "roles")
4380
}
4481

4582
// Ensure account_id is included for the API even though it's a path param
@@ -51,75 +88,76 @@ func (m AccountMemberModel) marshalCustomForUpdate(state AccountMemberModel) (da
5188
}
5289

5390
func unmarshalCustom(data []byte, configuredModel *AccountMemberModel) (*AccountMemberModel, error) {
91+
ctx := context.Background()
5492
var env AccountMemberResultEnvelope
55-
if err := apijson.UnmarshalComputed(data, &env); err != nil {
93+
if err := apijson.Unmarshal(data, &env); err != nil {
5694
return nil, err
5795
}
96+
result := &env.Result
97+
98+
result.AccountID = configuredModel.AccountID
99+
100+
return parsePoliciesAndRoles(ctx, data, result)
101+
}
58102

103+
func unmarshalComputedCustom(data []byte, configuredModel *AccountMemberModel) (*AccountMemberModel, error) {
104+
ctx := context.Background()
105+
var env AccountMemberResultEnvelope
106+
if err := apijson.UnmarshalComputed(data, &env); err != nil {
107+
return nil, err
108+
}
59109
result := &env.Result
60110

61-
// Preserve required fields from configured model to avoid replacement
62111
result.AccountID = configuredModel.AccountID
63-
result.Email = configuredModel.Email
112+
if result.Email.IsNull() && !configuredModel.Email.IsNull() {
113+
// Preserve required fields from configured model to avoid replacement
114+
result.Email = configuredModel.Email
115+
}
64116

65117
// Only preserve status if it was explicitly configured
66118
if !configuredModel.Status.IsNull() && !configuredModel.Status.IsUnknown() {
67119
result.Status = configuredModel.Status
68120
}
69121

70-
// Determine comparison strategy based on what's configured in Terraform
71-
// as user can use roles or policies to configure the account member
72-
configUsesRoles := configuredModel.Roles != nil && len(*configuredModel.Roles) > 0
73-
configUsesPolicies := !configuredModel.Policies.IsNull() && !configuredModel.Policies.IsUnknown()
122+
return parsePoliciesAndRoles(ctx, data, result)
123+
}
74124

75-
if configUsesRoles && !configUsesPolicies {
76-
// User configured roles - preserve computed policies from API to prevent diffs
77-
// Don't modify result.Policies - let it keep the computed values from API response
125+
func parsePoliciesAndRoles(ctx context.Context, data []byte, result *AccountMemberModel) (*AccountMemberModel, error) {
126+
// Extract role IDs from GET response (roles come as objects with id field)
127+
var fullResponse struct {
128+
Result struct {
129+
Policies []AccountMemberPoliciesModel `json:"policies"`
130+
Roles []struct {
131+
ID types.String `json:"id"`
132+
} `json:"roles"`
133+
} `json:"result"`
134+
}
78135

79-
// Extract role IDs from GET response (roles come as objects with id field)
80-
var fullResponse struct {
81-
Result struct {
82-
Roles []struct {
83-
ID string `json:"id"`
84-
} `json:"roles"`
85-
} `json:"result"`
86-
}
136+
err := apijson.Unmarshal(data, &fullResponse)
137+
if err != nil {
138+
return nil, err
139+
}
87140

88-
if err := json.Unmarshal(data, &fullResponse); err == nil && len(fullResponse.Result.Roles) > 0 {
89-
roleIDs := make([]types.String, 0, len(fullResponse.Result.Roles))
90-
for _, role := range fullResponse.Result.Roles {
91-
roleIDs = append(roleIDs, types.StringValue(role.ID))
92-
}
93-
result.Roles = &roleIDs
94-
} else {
95-
result.Roles = configuredModel.Roles
96-
}
97-
} else if configUsesPolicies && !configUsesRoles {
98-
// User configured policies - ignore roles, set roles to nil to prevent diffs
99-
result.Roles = nil
100-
101-
// Extract policies from the API response and ensure they match the configured structure
102-
var fullResponse struct {
103-
Result struct {
104-
Policies []AccountMemberPoliciesModel `json:"policies"`
105-
} `json:"result"`
106-
}
141+
roleIDs := make([]types.String, 0, len(fullResponse.Result.Roles))
142+
for _, role := range fullResponse.Result.Roles {
143+
roleIDs = append(roleIDs, role.ID)
144+
}
107145

108-
if err := json.Unmarshal(data, &fullResponse); err == nil && len(fullResponse.Result.Policies) > 0 {
109-
policies := append([]AccountMemberPoliciesModel(nil), fullResponse.Result.Policies...)
110-
policiesList, diags := customfield.NewObjectSet(context.Background(), policies)
111-
if !diags.HasError() {
112-
result.Policies = policiesList
113-
} else {
114-
result.Policies = configuredModel.Policies
115-
}
116-
} else {
117-
result.Policies = configuredModel.Policies
118-
}
146+
roleSet, diags := types.SetValueFrom(ctx, types.StringType, roleIDs)
147+
if !diags.HasError() {
148+
result.Roles = roleSet
149+
} else {
150+
// TODO what to do here?
151+
return result, fmt.Errorf("failed to parse roles")
152+
}
153+
154+
policies := append([]AccountMemberPoliciesModel(nil), fullResponse.Result.Policies...)
155+
policiesSet, diags := customfield.NewObjectSet(ctx, policies)
156+
if !diags.HasError() {
157+
result.Policies = policiesSet
119158
} else {
120-
// Fallback: preserve configured values
121-
result.Roles = configuredModel.Roles
122-
result.Policies = configuredModel.Policies
159+
// TODO what to do here?
160+
return result, fmt.Errorf("failed to parse policies")
123161
}
124162

125163
return result, nil

‎internal/services/account_member/custom_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package account_member
2+
3+
import "testing"
4+
5+
func Test_unmarshalCustom(t *testing.T) {
6+
json := `{"result":{"id":"5d5430af856e91ae03cac034b10d7dea","email":"[email protected]","user":{"id":null,"first_name":null,"last_name":null,"email":"[email protected]","two_factor_authentication_enabled":false},"status":"pending","api_access_enabled":null,"policies":[{"id":"073126da887b4291a32b71602542cc9e","access":"allow","permission_groups":[{"id":"ce2c69b09baf4ca38223910a8b7e07a9","name":"Administrator","meta":{"category":"general","description":"Can access the full account and edit subscriptions. Cannot manage memberships nor billing profile.","editable":"false","label":"admin","scopes":"com.cloudflare.api.account"}}],"resource_groups":[{"id":"402006e2284a45fe98db34cf0a4688fc","name":"com.cloudflare.api.account.a67e14daa5f8dceeb91fe5449ba496eb","meta":{"editable":"false"},"scope":{"key":"com.cloudflare.api.account.a67e14daa5f8dceeb91fe5449ba496eb","objects":[{"key":"*"}]}}]}],"roles":[{"id":"6251014a0ecd81ba24f4779a5434767c","name":"Administrator","description":"Can access the full account and edit subscriptions. Cannot manage memberships nor billing profile.","permissions":{"access":{"edit":true,"read":true},"analytics":{"edit":false,"read":true},"api_gateway":{"edit":true,"read":true},"app":{"edit":true,"read":false},"auditlogs":{"edit":false,"read":true},"billing":{"edit":false,"read":true},"blocks":{"edit":true,"read":true},"cache_purge":{"edit":true,"read":false},"casb":{"edit":true,"read":true},"cds":{"edit":true,"read":true},"cds_compute_account":{"edit":true,"read":true},"ces_analytics":{"edit":false,"read":true},"ces_integration":{"edit":true,"read":true},"ces_phishguard":{"edit":false,"read":true},"ces_policies":{"edit":true,"read":true},"ces_pra_report":{"edit":true,"read":true},"ces_search":{"action":true,"edit":false,"preview":true,"raw":true,"read":true,"trace":true},"ces_settings":{"edit":true,"read":true},"ces_submissions":{"edit":false,"read":true},"cf1_integration":{"casb":true,"ces":true,"edit":true,"read":true},"d1":{"edit":true,"read":false},"dex":{"edit":true,"read":true},"dns_records":{"edit":true,"read":true},"domain":{"edit":false,"read":true},"fbm":{"edit":true,"read":true},"fbm_acc":{"edit":true,"read":false},"healthchecks":{"edit":true,"read":true},"http_applications":{"edit":true,"read":true},"image":{"edit":true,"read":true},"integration":{"edit":true,"install":true,"read":true},"lb":{"edit":true,"read":true},"legal":{"edit":false,"read":true},"logs":{"edit":true,"read":true},"magic":{"edit":true,"read":true},"member":{"edit":false,"read":true},"organization":{"edit":true,"read":true},"page_shield":{"edit":true,"read":true},"query_cache":{"edit":true,"read":true},"r2_bucket":{"edit":true,"read":true},"r2_bucket_item":{"edit":true,"read":true},"r2_bucket_warehouse":{"edit":true,"read":true},"r2_bucket_warehouse_sql":{"edit":false,"read":true},"resilience":{"edit":false,"read":true},"ssl":{"edit":true,"read":true},"stream":{"edit":true,"read":true},"subscription":{"edit":true,"read":true},"teams":{"edit":true,"read":true,"report":true},"teams_device":{"edit":false,"read":true},"vectorize":{"edit":true,"read":true},"waf":{"edit":true,"read":true},"waitingroom":{"edit":true,"read":true},"web3":{"edit":true,"read":true},"worker":{"edit":true,"read":true},"zaraz":{"edit":true,"publish":true,"read":true},"zone":{"edit":true,"read":true},"zone_settings":{"edit":true,"read":true},"zone_versioning":{"edit":true,"read":true}}}]},"success":true,"errors":[],"messages":[]}`
7+
bytes := []byte(json)
8+
data := &AccountMemberModel{}
9+
unmarshaled, _ := unmarshalCustom(bytes, data)
10+
bytes, err := unmarshaled.marshalCustomForUpdate(*unmarshaled, Roles)
11+
if err != nil {
12+
t.Fatal(err)
13+
}
14+
policies := unmarshaled.Policies.Elements()
15+
for _, policy := range policies {
16+
_ = policy
17+
}
18+
}

‎internal/services/account_member/model.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type AccountMemberModel struct {
1717
AccountID types.String `tfsdk:"account_id" path:"account_id,required"`
1818
Email types.String `tfsdk:"email" json:"email,required"`
1919
Status types.String `tfsdk:"status" json:"status,computed_optional"`
20-
Roles *[]types.String `tfsdk:"roles" json:"roles,optional,no_refresh"`
20+
Roles types.Set `tfsdk:"roles" json:"roles,computed_optional,no_refresh"`
2121
Policies customfield.NestedObjectSet[AccountMemberPoliciesModel] `tfsdk:"policies" json:"policies,computed_optional"`
2222
User customfield.NestedObject[AccountMemberUserModel] `tfsdk:"user" json:"user,computed"`
2323
}

0 commit comments

Comments
 (0)