Skip to content

Commit

Permalink
Bugfix and enhancement from customers. (#127)
Browse files Browse the repository at this point in the history
Solutions:
1. Update Read action to make cluster-side updates invisible to
terraform for User, User Groups, Groupnet and Access Zone resources.
2. Remove hard codes of auth providers for Access Zone resource.
3. Add validation for different quota type for Quota resource.
4. Change owner and groups fields to Optional for File System resource.
  • Loading branch information
taohe1012 authored Mar 7, 2024
1 parent ed516c0 commit 7b77b5c
Show file tree
Hide file tree
Showing 24 changed files with 632 additions and 136 deletions.
16 changes: 12 additions & 4 deletions docs/resources/accesszone.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ limitations under the License.
resource "powerscale_accesszone" "zone" {
# Required name of the new access zone
name = "testAccessZone3"
name = "testGroupnetResourceSample"
# Required Groupnet identifier to be assoicated with this access zone
# Note can not be changed after the access zone is created
Expand All @@ -63,8 +63,16 @@ resource "powerscale_accesszone" "zone" {
# Required Specifies the access zone base directory path
path = "/ifs"
# Optional pecifies the list of authentication providers available on this access zon
custom_auth_providers = ["System"]
# Optional pecifies the list of authentication providers available on this access zone
# A provider name should be of the form '[provider-type:]provider-name', the provider-type defaults to 'lsa-local-provider'.
custom_auth_providers = [
"localProviderName",
"lsa-local-provider:testGroupnetResourceSample",
"lsa-local-provider:localProviderName",
"lsa-file-provider:fileProviderName",
"lsa-activedirectory-provider:adsProviderName",
"lsa-ldap-provider:testProvider",
]
}
# After the execution of above resource block, accesszone would have been created on the PowerScale array. For more information, Please check the terraform state file.
Expand All @@ -81,7 +89,7 @@ resource "powerscale_accesszone" "zone" {

### Optional

- `custom_auth_providers` (List of String) An optional parameter which adds new auth_providers to the access zone. (Update Supported)
- `custom_auth_providers` (List of String) An optional parameter which adds new auth_providers to the access zone. A provider name should be of the form '[provider-type:]provider-name', the provider-type defaults to 'lsa-local-provider'. (Update Supported)

### Read-Only

Expand Down
4 changes: 2 additions & 2 deletions docs/resources/filesystem.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ resource "powerscale_filesystem" "file_system_test" {
<a id="nestedatt--group"></a>
### Nested Schema for `group`

Required:
Optional:

- `id` (String) group identifier
- `name` (String) group name
Expand All @@ -130,7 +130,7 @@ Required:
<a id="nestedatt--owner"></a>
### Nested Schema for `owner`

Required:
Optional:

- `id` (String) Owner identifier
- `name` (String) Owner name
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/quota.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ resource "powerscale_quota" "quota_test" {
- `force` (Boolean) Force creation of quotas on the root of /ifs or percent based quotas.
- `ignore_limit_checks` (Boolean) If true, skip child quota's threshold comparison with parent quota path.
- `linked` (Boolean) For user, group and directory quotas, true if the quota is linked and controlled by a parent default-* quota. Linked quotas cannot be modified until they are unlinked. Computed by PowerScale, do not set Linked while creating.
- `persona` (Attributes) Specifies the persona of the file group. (see [below for nested schema](#nestedatt--persona))
- `persona` (Attributes) Specifies the persona of the file group. persona is required for user and group type. (see [below for nested schema](#nestedatt--persona))
- `thresholds` (Attributes) The thresholds of quota (see [below for nested schema](#nestedatt--thresholds))
- `thresholds_on` (String) Thresholds apply on quota accounting metric.
- `zone` (String) Optional named zone to use for user and group resolution.
Expand Down
14 changes: 11 additions & 3 deletions examples/resources/powerscale_accesszone/resource.tf
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ limitations under the License.
resource "powerscale_accesszone" "zone" {

# Required name of the new access zone
name = "testAccessZone3"
name = "testGroupnetResourceSample"

# Required Groupnet identifier to be assoicated with this access zone
# Note can not be changed after the access zone is created
Expand All @@ -31,8 +31,16 @@ resource "powerscale_accesszone" "zone" {
# Required Specifies the access zone base directory path
path = "/ifs"

# Optional pecifies the list of authentication providers available on this access zon
custom_auth_providers = ["System"]
# Optional pecifies the list of authentication providers available on this access zone
# A provider name should be of the form '[provider-type:]provider-name', the provider-type defaults to 'lsa-local-provider'.
custom_auth_providers = [
"localProviderName",
"lsa-local-provider:testGroupnetResourceSample",
"lsa-local-provider:localProviderName",
"lsa-file-provider:fileProviderName",
"lsa-activedirectory-provider:adsProviderName",
"lsa-ldap-provider:testProvider",
]
}

# After the execution of above resource block, accesszone would have been created on the PowerScale array. For more information, Please check the terraform state file.
37 changes: 28 additions & 9 deletions powerscale/helper/access_zone_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,36 @@ func GetSpecificZone(ctx context.Context, matchZone string, zoneList []powerscal
}

// ExtractCustomAuthForInput extracts the custom auth provider from actual auth provider for input.
func ExtractCustomAuthForInput(ctx context.Context, authProv basetypes.ListValue, mainAuth string) (basetypes.ListValue, diag.Diagnostics) {
var filteredAuths []attr.Value
for _, v := range authProv.Elements() {
name := strings.Split(v.String(), ":")[1]
name = strings.Split(name, "\"")[0]
if name != mainAuth {
stringVal := types.StringValue(name)
filteredAuths = append(filteredAuths, stringVal)
func ExtractCustomAuthForInput(ctx context.Context, stateResponse, plan models.AccessZoneResourceModel) basetypes.ListValue {

automaticProviderAdded := false
automaticProviderName := "lsa-local-provider:" + plan.Name.ValueString()
var customAuthProviders []attr.Value
for _, v := range plan.CustomAuthProviders.Elements() {
name := strings.Trim(v.String(), "\"")
if !strings.Contains(name, ":") {
name = "lsa-local-provider:" + name
}
if name == automaticProviderName {
automaticProviderAdded = true
}
customAuthProviders = append(customAuthProviders, types.StringValue(name))
}
if !automaticProviderAdded {
customAuthProviders = append(customAuthProviders, types.StringValue(automaticProviderName))
}

authProviders := stateResponse.AuthProviders.Elements()
if len(customAuthProviders) != len(authProviders) {
return stateResponse.AuthProviders
}
return types.ListValue(types.StringType, filteredAuths)
for i, v := range authProviders {
if !customAuthProviders[i].Equal(v) {
return stateResponse.AuthProviders
}
}

return plan.CustomAuthProviders
}

// QueryZoneNameByID returns a specific zone id by name.
Expand Down
3 changes: 0 additions & 3 deletions powerscale/helper/file_pool_policy_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,6 @@ func parseActionParams(ctx context.Context, actionsResponse []powerscale.V1Filep
default:
return actions, fmt.Errorf("unexpected action type: %s", action.ActionType)
}
if err != nil {
return actions, fmt.Errorf("failed to parse action param: %v for this action type: %s, Error: %s", action.ActionParam, action.ActionType, err.Error())
}
if !correctType {

return actions, fmt.Errorf("unexpected action param: %v for this action type: %s", action.ActionParam, action.ActionType)
Expand Down
70 changes: 53 additions & 17 deletions powerscale/helper/file_system_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,14 +293,26 @@ func UpdateFileSystemResourceState(ctx context.Context, plan *models.FileSystemR
}

if owner, ok := acl.GetOwnerOk(); ok {
state.Owner.ID = types.StringValue(*owner.Id)
state.Owner.Name = types.StringValue(*owner.Name)
state.Owner.Type = types.StringValue(*owner.Type)
if ownerID, ok := owner.GetIdOk(); ok {
state.Owner.ID = types.StringValue(*ownerID)
}
if ownerName, ok := owner.GetNameOk(); ok {
state.Owner.Name = types.StringValue(*ownerName)
}
if ownerType, ok := owner.GetTypeOk(); ok {
state.Owner.Type = types.StringValue(*ownerType)
}
}
if group, ok := acl.GetGroupOk(); ok {
state.Group.ID = types.StringValue(*group.Id)
state.Group.Name = types.StringValue(*group.Name)
state.Group.Type = types.StringValue(*group.Type)
if groupID, ok := group.GetIdOk(); ok {
state.Group.ID = types.StringValue(*groupID)
}
if groupName, ok := group.GetNameOk(); ok {
state.Group.Name = types.StringValue(*groupName)
}
if groupType, ok := group.GetTypeOk(); ok {
state.Group.Type = types.StringValue(*groupType)
}
}
if authoritative, ok := acl.GetAuthoritativeOk(); ok {
state.Authoritative = types.StringValue(*authoritative)
Expand Down Expand Up @@ -352,15 +364,27 @@ func UpdateFileSystem(ctx context.Context, client *client.Client, dirPath string
namespaceUpdateUser.SetAuthoritative(mode)

owner := *powerscale.NewMemberObject()
owner.Id = plan.Owner.ID.ValueStringPointer()
owner.Name = plan.Owner.Name.ValueStringPointer()
owner.Type = plan.Owner.Type.ValueStringPointer()
if !plan.Owner.ID.IsNull() && !plan.Owner.ID.IsUnknown() {
owner.Id = plan.Owner.ID.ValueStringPointer()
}
if !plan.Owner.Name.IsNull() && !plan.Owner.Name.IsUnknown() {
owner.Name = plan.Owner.Name.ValueStringPointer()
}
if !plan.Owner.Type.IsNull() && !plan.Owner.Type.IsUnknown() {
owner.Type = plan.Owner.Type.ValueStringPointer()
}
namespaceUpdateUser.SetOwner(owner)

group := *powerscale.NewMemberObject()
group.Id = plan.Group.ID.ValueStringPointer()
group.Name = plan.Group.Name.ValueStringPointer()
group.Type = plan.Group.Type.ValueStringPointer()
if !plan.Group.ID.IsNull() && !plan.Group.ID.IsUnknown() {
group.Id = plan.Group.ID.ValueStringPointer()
}
if !plan.Group.Name.IsNull() && !plan.Group.Name.IsUnknown() {
group.Name = plan.Group.Name.ValueStringPointer()
}
if !plan.Group.Type.IsNull() && !plan.Group.Type.IsUnknown() {
group.Type = plan.Group.Type.ValueStringPointer()
}
namespaceUpdateUser.SetGroup(group)

setACLUpdReq = setACLUpdReq.NamespaceAcl(namespaceUpdateUser)
Expand Down Expand Up @@ -417,8 +441,20 @@ func getNewAccessControlParams(accessControl string) (string, string) {

// ValidateUserAndGroup check if owner/group information is correct.
func ValidateUserAndGroup(ctx context.Context, client *client.Client, owner models.MemberObject, group models.MemberObject, accessZone string) error {
var ownerAuthID, groupAuthID string
if !owner.Name.IsNull() && !owner.Name.IsUnknown() {
ownerAuthID = owner.Name.ValueString()
} else if !owner.ID.IsNull() && !owner.ID.IsUnknown() {
ownerAuthID = owner.ID.ValueString()
}
if !group.Name.IsNull() && !group.Name.IsUnknown() {
groupAuthID = group.Name.ValueString()
} else if !group.ID.IsNull() && !group.ID.IsUnknown() {
groupAuthID = group.ID.ValueString()
}

// Validate owner information
userReq := client.PscaleOpenAPIClient.AuthApi.GetAuthv1AuthUser(ctx, owner.Name.ValueString())
userReq := client.PscaleOpenAPIClient.AuthApi.GetAuthv1AuthUser(ctx, ownerAuthID)

// If zone filter is set use it otherwise leave blank and it will use the default zone
if accessZone != "" {
Expand All @@ -434,15 +470,15 @@ func ValidateUserAndGroup(ctx context.Context, client *client.Client, owner mode
user, ok := users.GetUsersOk()
if ok && len(user) > 0 {
userEntity := user[0].OnDiskUserIdentity
if *userEntity.Id != *owner.ID.ValueStringPointer() || *userEntity.Name != *owner.Name.ValueStringPointer() || *userEntity.Type != *owner.Type.ValueStringPointer() {
return fmt.Errorf("Incorrect owner information. Please make sure owner id, name, and type are valid")
if !owner.ID.IsUnknown() && *userEntity.Id != owner.ID.ValueString() || !owner.Name.IsUnknown() && *userEntity.Name != owner.Name.ValueString() || !owner.Type.IsUnknown() && *userEntity.Type != owner.Type.ValueString() {
return fmt.Errorf("incorrect owner information. Please make sure owner id, name, and type are valid")
}
} else {
return fmt.Errorf("unable to retrieve user information")
}

// Validate group information
groupReq := client.PscaleOpenAPIClient.AuthApi.GetAuthv1AuthGroup(ctx, group.Name.ValueString())
groupReq := client.PscaleOpenAPIClient.AuthApi.GetAuthv1AuthGroup(ctx, groupAuthID)

// If zone filter is set use it otherwise leave blank and it will use the default zone
if accessZone != "" {
Expand All @@ -458,7 +494,7 @@ func ValidateUserAndGroup(ctx context.Context, client *client.Client, owner mode
grp, okGroup := groups.GetGroupsOk()
if okGroup && len(grp) > 0 {
grpEntity := grp[0].Gid
if *grpEntity.Id != *group.ID.ValueStringPointer() || *grpEntity.Name != *group.Name.ValueStringPointer() || *grpEntity.Type != *group.Type.ValueStringPointer() {
if !group.ID.IsUnknown() && *grpEntity.Id != group.ID.ValueString() || !group.Name.IsUnknown() && *grpEntity.Name != group.Name.ValueString() || !group.Type.IsUnknown() && *grpEntity.Type != group.Type.ValueString() {
return fmt.Errorf("incorrect group information. Please make sure group id, name, and type are valid")
}
} else {
Expand Down
33 changes: 21 additions & 12 deletions powerscale/helper/groupnet_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import (
"context"
powerscale "dell/powerscale-go-client"
"fmt"
"strings"
"terraform-provider-powerscale/client"
"terraform-provider-powerscale/powerscale/constants"
"terraform-provider-powerscale/powerscale/models"

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
)
Expand Down Expand Up @@ -56,21 +56,30 @@ func UpdateGroupnetDataSourceState(ctx context.Context, groupnetState *models.Gr

// UpdateGroupnetResourceState updates resource state.
func UpdateGroupnetResourceState(ctx context.Context, groupnetModel *models.GroupnetModel, groupnetResponse *powerscale.V10NetworkGroupnetExtended) (err error) {
originModel := *groupnetModel

groupnetModel.ID = types.StringValue(*groupnetResponse.Id)
groupnetModel.Name = types.StringValue(*groupnetResponse.Name)
groupnetModel.AllowWildcardSubdomains = types.BoolValue(*groupnetResponse.AllowWildcardSubdomains)
groupnetModel.DNSCacheEnabled = types.BoolValue(*groupnetResponse.DnsCacheEnabled)
groupnetModel.ServerSideDNSSearch = types.BoolValue(*groupnetResponse.ServerSideDnsSearch)
if err = CopyFields(ctx, groupnetResponse, groupnetModel); err != nil {
return
}

if groupnetResponse.HasSubnets() {
var subnetAttrs []attr.Value
for _, subnet := range groupnetResponse.Subnets {
subnetAttrs = append(subnetAttrs, types.StringValue(subnet))
if strings.Trim(*groupnetResponse.Description, " ") == strings.Trim(originModel.Description.ValueString(), " ") {
groupnetModel.Description = originModel.Description
}
if IsListValueEquals(originModel.DNSSearch, groupnetModel.DNSSearch) {
groupnetModel.DNSSearch = originModel.DNSSearch
}
if IsListValueEquals(originModel.DNSServers, groupnetModel.DNSServers) {
groupnetModel.DNSServers = originModel.DNSServers
}
groupnetModel.DNSResolverRotate = types.BoolValue(false)
if groupnetResponse.HasDnsOptions() {
for _, option := range groupnetResponse.DnsOptions {
if option == "rotate" {
groupnetModel.DNSResolverRotate = types.BoolValue(true)
break
}
}
groupnetModel.Subnets, _ = types.ListValue(types.StringType, subnetAttrs)
}

return
}

Expand Down
31 changes: 26 additions & 5 deletions powerscale/helper/quota_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,20 +112,41 @@ func ListQuotas(ctx context.Context, client *client.Client, quotaFilter *models.

// ValidateQuotaUpdate validates if update params contain params only for creating.
func ValidateQuotaUpdate(plan models.QuotaResource, state models.QuotaResource) error {
if (plan.Zone.IsNull() && !state.Zone.IsNull()) || !plan.Zone.Equal(state.Zone) {
if !plan.Zone.IsNull() && !plan.Zone.Equal(state.Zone) && (plan.Zone.ValueString() != "System" || !state.Zone.IsNull()) {
return fmt.Errorf("do not update field Zone")
}
if (plan.Path.IsNull() && !state.Path.IsNull()) || !plan.Path.Equal(state.Path) {
if !plan.Path.IsNull() && !plan.Path.Equal(state.Path) {
return fmt.Errorf("do not update field Path")
}
if (plan.Type.IsNull() && !state.Type.IsNull()) || !plan.Type.Equal(state.Type) {
if !plan.Type.IsNull() && !plan.Type.Equal(state.Type) {
return fmt.Errorf("do not update field Type")
}
if (plan.IncludeSnapshots.IsNull() && !state.IncludeSnapshots.IsNull()) || !plan.IncludeSnapshots.Equal(state.IncludeSnapshots) {
if !plan.IncludeSnapshots.IsNull() && !plan.IncludeSnapshots.Equal(state.IncludeSnapshots) {
return fmt.Errorf("do not update field IncludeSnapshots")
}
if (plan.Persona.IsNull() && !state.Persona.IsNull()) || !plan.Persona.Equal(state.Persona) {
if !plan.Persona.IsNull() && !plan.Persona.Equal(state.Persona) {
return fmt.Errorf("do not update field Persona.ID")
}
return nil
}

// IsQuotaParamInvalid Verify if persona and zone params are valid for different quota type.
func IsQuotaParamInvalid(plan models.QuotaResource) error {
quotaType := plan.Type.ValueString()
switch quotaType {
case "user", "group":
if plan.Persona.IsNull() || plan.Persona.IsUnknown() {
return fmt.Errorf("\"persona\" is required for %s type", quotaType)
}
case "directory":
if !plan.Persona.IsNull() {
return fmt.Errorf("\"persona\" is not needed for %s type", quotaType)
}
if !plan.Zone.IsNull() {
return fmt.Errorf("\"zone\" is not needed for %s type", quotaType)
}
default:
return fmt.Errorf("unsupported type: %s", quotaType)
}
return nil
}
3 changes: 0 additions & 3 deletions powerscale/helper/resource_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ func CopyFieldsToNonNestedModel(ctx context.Context, source, destination interfa
return err
}
destinationField.Set(reflect.ValueOf(types.ListNull(mapType)))
if err != nil {
return err
}
case reflect.Struct:
mapType, err := getStructAttrTypeFromType(ctx, structType.Field(i).Type)
if err != nil {
Expand Down
Loading

0 comments on commit 7b77b5c

Please sign in to comment.