Skip to content

Commit b4e6e91

Browse files
fix: prevent account perpetual diffs and wrong-user selection
Follow-up from the review loop on the account resource: - Mark account and domain_id as Computed (in addition to Optional). Create defaults the account name to the username and the API resolves an omitted domain to the caller's domain; Read writes those server-side values back, which produced a perpetual diff whenever the fields were omitted. - Only send domainid to createAccount when one is configured, instead of passing an empty value. - Gate account_type on explicit presence in config via GetRawConfig so an explicit 0 (a valid account type) is honored while an omitted value stays role-derived; GetOk treats 0 as unset. - Drop the Read fallback to the first user; selecting an arbitrary user could bind state to the wrong user on multi-user accounts. - Guard the account rename so an empty new name is never sent.
1 parent 220cf1b commit b4e6e91

1 file changed

Lines changed: 17 additions & 13 deletions

File tree

cloudstack/resource_cloudstack_account.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,12 @@ func resourceCloudStackAccount() *schema.Resource {
6767
"account": {
6868
Type: schema.TypeString,
6969
Optional: true,
70+
Computed: true,
7071
},
7172
"domain_id": {
7273
Type: schema.TypeString,
7374
Optional: true,
75+
Computed: true,
7476
},
7577
},
7678
}
@@ -89,18 +91,21 @@ func resourceCloudStackAccountCreate(d *schema.ResourceData, meta interface{}) e
8991

9092
// Create a new parameter struct
9193
p := cs.Account.NewCreateAccountParams(email, first_name, last_name, password, username)
92-
// account_type is derived from the role when omitted; only set it when
93-
// explicitly configured.
94-
if v, ok := d.GetOk("account_type"); ok {
95-
p.SetAccounttype(v.(int))
94+
// account_type is derived from the role when omitted; only send it when the
95+
// user explicitly set it in config. GetRawConfig distinguishes an explicit
96+
// 0 (a valid account type) from an omitted value, which GetOk cannot.
97+
if cfg := d.GetRawConfig(); !cfg.IsNull() && !cfg.GetAttr("account_type").IsNull() {
98+
p.SetAccounttype(d.Get("account_type").(int))
9699
}
97100
p.SetRoleid(role_id)
98101
if account != "" {
99102
p.SetAccount(account)
100103
} else {
101104
p.SetAccount(username)
102105
}
103-
p.SetDomainid(domain_id)
106+
if domain_id != "" {
107+
p.SetDomainid(domain_id)
108+
}
104109

105110
log.Printf("[DEBUG] Creating Account %s", account)
106111
a, err := cs.Account.CreateAccount(p)
@@ -141,13 +146,10 @@ func resourceCloudStackAccountRead(d *schema.ResourceData, meta interface{}) err
141146
d.Set("account", account.Name)
142147
d.Set("domain_id", account.Domainid)
143148

144-
user := accountUser(account, d.Get("username").(string))
145-
if user == nil && len(account.User) > 0 {
146-
// Managed username not found (e.g. renamed out of band); fall back to the
147-
// first user so state still reflects a real user of the account.
148-
user = &account.User[0]
149-
}
150-
if user != nil {
149+
// Match the user this resource manages by username. CloudStack accounts can
150+
// hold multiple users, so falling back to an arbitrary index could bind state
151+
// to the wrong user; leave the user fields untouched when no match is found.
152+
if user := accountUser(account, d.Get("username").(string)); user != nil {
151153
d.Set("email", user.Email)
152154
d.Set("first_name", user.Firstname)
153155
d.Set("last_name", user.Lastname)
@@ -172,7 +174,9 @@ func resourceCloudStackAccountUpdate(d *schema.ResourceData, meta interface{}) e
172174
p.SetRoleid(d.Get("role_id").(string))
173175
}
174176
if d.HasChange("account") {
175-
p.SetNewname(d.Get("account").(string))
177+
if newName := d.Get("account").(string); newName != "" {
178+
p.SetNewname(newName)
179+
}
176180
}
177181
if d.HasChange("domain_id") {
178182
p.SetDomainid(d.Get("domain_id").(string))

0 commit comments

Comments
 (0)