Skip to content
This repository has been archived by the owner on Jun 17, 2024. It is now read-only.

Unneccesary lookup of BusinessGroupId via Identity API #103

Closed
dmorr12 opened this issue Nov 30, 2020 · 10 comments
Closed

Unneccesary lookup of BusinessGroupId via Identity API #103

dmorr12 opened this issue Nov 30, 2020 · 10 comments

Comments

@dmorr12
Copy link

dmorr12 commented Nov 30, 2020

terraform-provider-vra7 plugin version 3.0.0

The latest release now sets the "businessgroup_name" as party of resourceVrs7DeploymentRead - line 448 of resource_vra7_deployment.go

On line 637 of resource_vra7_deployment.go, a check is made to see if the name is provided and then the BusinessGroupId is looked up from the name. This involves a call to the identity API. Unfortunately for us, we do not have permission to access this particular api.

Given that the BusinessGroupId has already been provided and set, this is unnecessary. It would make sense to only lookup the BusinessGroupId if its not already set.

@LeoK80
Copy link

LeoK80 commented Jan 8, 2021

We're encountering the same issue. However groups for the requesting user (consumer) can be looked up from /identity/api/tenants/{tenant}/subtenants/membership

Would it be possible to add a routine for this lookup when lookup on path /identity/api/tenants/{tenant}/subtenants ends up returning no Content?

@LeoK80
Copy link

LeoK80 commented Jan 8, 2021

Something along these lines inserted after line 147 should provide a fallback check within the memberships without breaking existing functionality.

  • check for original call to have retrieved and empty BusinessGroups array, if so ...
  • when array empty try and update variable reference &businessGroups to response of original path appended with /membership
  • rely on original code to pick out and return the appropriate businessGroupID from matching businessGroup found by name match.
if len(businessGroups.Content) == 0 {
    path = path + "/membership"

    membershipURL := c.BuildEncodedURL(path, nil)

    resp, respErr := c.Get(membershipURL, nil)
    if respErr != nil {
        return "", respErr
    }
    unmarshallErr := utils.UnmarshalJSON(resp.Body, &businessGroups)
    if unmarshallErr != nil {
        return "", unmarshallErr
    }
}

@Prativa20
Copy link
Contributor

Looking into it.

-Thanks,
Prativa

@hobovirtual
Copy link

hi @Prativa20

having the same issue with provider 3.0

since the /identity/api/tenants/{tenant}/subtenants uri requires tenantadmin role within vra, this is causing issue for all user that wish to do any day2 operations including delete.

/identity/api/tenants/{tenant}/subtenants/membership is a viable solution as mentioned by @LeoK80

the businessgroupid field is also in the tfstate since it is gathered by the requestemplate during creation

please let me know if you need anything to fix this!
thanks!

@Prativa20
Copy link
Contributor

Created a PR for this #105

@hobovirtual
Copy link

Hi @Prativa20

i see the fix has been merged, are we expecting a version 3.0.1 soon ?

Thanks!

@Prativa20
Copy link
Contributor

@hobovirtual released version 3.0.1

Thanks,
Prativa

@mmn01-sky
Copy link

@Prativa20 When can we expect version 3.0.1 to be available in the terraform registry?

@hobovirtual
Copy link

fyi
new provider is available and working as expected!!!

@Prativa20
Copy link
Contributor

@hobovirtual thanks for verifying!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants