-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add role attributes to launchdarkly_team_member
#289
feat: add role attributes to launchdarkly_team_member
#289
Conversation
…-5765/add-role-attributes-to-custom-roles
…nto imiller/REL-5765/add-role-attributes-to-member
new := roleAttributesFromResourceData(d.Get(ROLE_ATTRIBUTES).(*schema.Set).List()) | ||
if new != nil { | ||
if old == nil { | ||
patch = append(patch, patchAdd("/roleAttributes", new)) | ||
} else { | ||
for k, v := range *new { | ||
if _, ok := (*old)[k]; !ok { | ||
patch = append(patch, patchAdd(fmt.Sprintf("/roleAttributes/%s", k), &v)) | ||
} else { | ||
if oldV := (*old)[k]; slices.Compare(oldV, v) == 0 { | ||
continue | ||
} else { | ||
patch = append(patch, patchReplace(fmt.Sprintf("/roleAttributes/%s", k), &v)) | ||
} | ||
} | ||
} | ||
for k := range *old { | ||
if _, ok := (*new)[k]; !ok { | ||
patch = append(patch, patchRemove(fmt.Sprintf("/roleAttributes/%s", k))) | ||
} | ||
} | ||
} | ||
} else { | ||
if old != nil { | ||
for k := range *old { | ||
patch = append(patch, patchRemove(fmt.Sprintf("/roleAttributes/%s", k))) | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a little more complicated than it needs to be. Did you try just sending something like:
{
"action": "replace",
"path": "/roleAttributes",
"value": <The entire new role attributes map>
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unfortunately - it hits an 'unsupported json-patch operation' on some operations, although I can't remember which exactly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work for me. Can you give it a try:
func getRoleAttributePatches(d *schema.ResourceData) []ldapi.PatchOperation {
var patch []ldapi.PatchOperation
if o, n := d.GetChange(ROLE_ATTRIBUTES); o != n {
new := roleAttributesFromResourceData(d.Get(ROLE_ATTRIBUTES).(*schema.Set).List())
if new != nil {
patch = append(patch, patchReplace("/roleAttributes", new))
} else {
patch = append(patch, patchReplace("/roleAttributes", make(map[string][]string)))
}
}
return patch
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the tests are passing here. Feel free to merge it into this PR if it helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, done
new := roleAttributesFromResourceData(d.Get(ROLE_ATTRIBUTES).(*schema.Set).List()) | ||
if new != nil { | ||
if old == nil { | ||
patch = append(patch, patchAdd("/roleAttributes", new)) | ||
} else { | ||
for k, v := range *new { | ||
if _, ok := (*old)[k]; !ok { | ||
patch = append(patch, patchAdd(fmt.Sprintf("/roleAttributes/%s", k), &v)) | ||
} else { | ||
if oldV := (*old)[k]; slices.Compare(oldV, v) == 0 { | ||
continue | ||
} else { | ||
patch = append(patch, patchReplace(fmt.Sprintf("/roleAttributes/%s", k), &v)) | ||
} | ||
} | ||
} | ||
for k := range *old { | ||
if _, ok := (*new)[k]; !ok { | ||
patch = append(patch, patchRemove(fmt.Sprintf("/roleAttributes/%s", k))) | ||
} | ||
} | ||
} | ||
} else { | ||
if old != nil { | ||
for k := range *old { | ||
patch = append(patch, patchRemove(fmt.Sprintf("/roleAttributes/%s", k))) | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the tests are passing here. Feel free to merge it into this PR if it helps.
Simplify roleAttributes logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
This PR defines the
role_attributes
schema block and adds a newrole_attributes
field to thelaunchdarkly_team_member
resource and data source. It also adds tests and updates the docs accordingly.